Repository navigation
feat(webview): add timeout state and retry UI for stalled embedded webview loads - #1043
Conversation
…and enhance timeout handling - Added a retry button for users when a webview account fails to load within the expected time. - Updated loading status to 'timeout' and displayed a user-friendly message with retry options. - Enhanced the handling of Chromium error pages to trigger the timeout UI instead of showing a blank page. - Updated tests to cover new timeout behavior and retry functionality.
📝 WalkthroughWalkthroughThese changes implement a timeout and retry flow for slow-loading embedded webviews. They introduce a new Changes
Sequence DiagramsequenceDiagram
actor User
participant WebviewHost as Frontend:<br/>WebviewHost
participant Service as Service:<br/>webviewAccountService
participant Backend as Backend:<br/>webview_accounts
participant CDP as CDP:<br/>Session
participant Store as Store:<br/>Account Status
User->>WebviewHost: Open account
activate WebviewHost
WebviewHost->>Service: openWebviewAccount()
deactivate WebviewHost
activate Service
Service->>Service: Set status: 'loading'
Service->>Backend: webview_account_open()
deactivate Service
activate Backend
Backend->>CDP: Monitor page load
Note over CDP: Timer starts (LOAD_TIMEOUT)
alt Load completes in time
CDP->>Backend: Page load finished
Backend->>Service: emit_load_finished(finished)
activate Service
Service->>Store: Update status: 'open'
Service->>WebviewHost: Reveal webview
deactivate Service
else Load exceeds timeout
CDP->>Backend: Timeout watchdog fires
Backend->>Service: emit_load_finished(timeout)
activate Service
Service->>Store: Update status: 'timeout'
Service->>Backend: webview_account_hide()
deactivate Service
deactivate Backend
activate WebviewHost
WebviewHost->>WebviewHost: Render timeout overlay
WebviewHost->>User: Show "Loading...<br/>Retry" UI
deactivate WebviewHost
User->>WebviewHost: Click Retry
activate WebviewHost
WebviewHost->>Service: retryWebviewAccountLoad()
deactivate WebviewHost
activate Service
Service->>Backend: webview_account_open()<br/>(with cached bounds)
deactivate Service
activate Backend
Backend->>CDP: Resume/restart load
CDP->>Backend: Page load finished
Backend->>Service: emit_load_finished(finished)
activate Service
Service->>Store: Update status: 'open'
Service->>WebviewHost: Reveal webview
deactivate Service
deactivate Backend
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/components/accounts/WebviewHost.tsx (1)
22-32: ⚡ Quick winAvoid duplicating provider labels in
PROVIDER_COPY.Line 22 introduces a second source of truth for provider display names. This can drift from
app/src/types/accounts.ts(PROVIDERSlabels).♻️ Proposed refactor
import { useAppSelector } from '../../store/hooks'; -import type { AccountProvider, AccountStatus } from '../../types/accounts'; +import { PROVIDERS } from '../../types/accounts'; +import type { AccountProvider, AccountStatus } from '../../types/accounts'; @@ -const PROVIDER_COPY: Record<AccountProvider, string> = { - whatsapp: 'WhatsApp', - telegram: 'Telegram', - linkedin: 'LinkedIn', - gmail: 'Gmail', - slack: 'Slack', - discord: 'Discord', - 'google-meet': 'Google Meet', - zoom: 'Zoom', - browserscan: 'BrowserScan', -}; +const PROVIDER_COPY: Record<AccountProvider, string> = Object.fromEntries( + PROVIDERS.map(p => [p.id, p.label]) +) as Record<AccountProvider, string>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/accounts/WebviewHost.tsx` around lines 22 - 32, PROVIDER_COPY duplicates display labels already defined in PROVIDERS; remove the hardcoded PROVIDER_COPY and instead import the canonical PROVIDERS mapping from app/src/types/accounts.ts (or re-use its label values) so the component uses the single source of truth; update WebviewHost.tsx to reference PROVIDERS[provider].label (or similar property) where PROVIDER_COPY was used and ensure type compatibility with AccountProvider.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/components/accounts/WebviewHost.tsx`:
- Around line 22-32: PROVIDER_COPY duplicates display labels already defined in
PROVIDERS; remove the hardcoded PROVIDER_COPY and instead import the canonical
PROVIDERS mapping from app/src/types/accounts.ts (or re-use its label values) so
the component uses the single source of truth; update WebviewHost.tsx to
reference PROVIDERS[provider].label (or similar property) where PROVIDER_COPY
was used and ensure type compatibility with AccountProvider.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0572f5f8-6ded-413a-abad-1057a7f4b2e3
📒 Files selected for processing (6)
app/src-tauri/src/cdp/session.rsapp/src-tauri/src/webview_accounts/mod.rsapp/src/components/accounts/WebviewHost.tsxapp/src/services/__tests__/webviewAccountService.loadListener.test.tsapp/src/services/webviewAccountService.tsapp/src/types/accounts.ts
Summary
Introduces a
timeoutaccount status when the 15s load watchdog expires:On timeout or Chromium error-page detection:
Supports non-terminal timeout design:
Page.loadEventFired/on_page_loadcan still recover sessionHandles Chromium error pages (
chrome-error://chromewebdata) consistently:Adds
retryWebviewAccountLoad:Problem
Webview failures (network issues, CEF hiccups, Chromium error pages):
Required restarting app or reopening account:
Solution
Rust (
webview_accounts/mod.rs,cdp/session.rs)emit_load_finished:"timeout"as non-terminalForward
chrome-error://URLs:"timeout"instead of"finished"TypeScript (
webviewAccountService.ts)Add
looksLikeChromiumErrorUrl:On timeout:
webview_account_hidestatus: 'timeout'UI (
WebviewHost.tsx)Add timeout UI branch:
Use
PROVIDER_COPY:Account Status (
accounts.ts)'timeout''loading'and'open'Submission Checklist
Tests
Coverage Matrix
docs/TEST-COVERAGE-MATRIX.md)Feature IDs
## RelatedNetwork Dependencies
Manual Smoke Checklist
Linked Issues
Closes #NNNmissing)Impact
Platform
Data / Persistence
timeoutis ephemeral (not persisted)Security
UX
Related
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes