From 126ffdaca642f72158198a7bcb11c32606070b81 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Thu, 30 Apr 2026 21:21:50 +0530 Subject: [PATCH] feat(webview): add timeout state and retry UI for stalled embedded webview loads (#1043) --- app/src-tauri/src/cdp/session.rs | 16 +-- app/src-tauri/src/webview_accounts/mod.rs | 102 +++++++++++++----- app/src/components/accounts/WebviewHost.tsx | 43 +++++++- ...webviewAccountService.loadListener.test.ts | 43 +++++++- app/src/services/webviewAccountService.ts | 43 +++++++- app/src/types/accounts.ts | 3 +- 6 files changed, 214 insertions(+), 36 deletions(-) diff --git a/app/src-tauri/src/cdp/session.rs b/app/src-tauri/src/cdp/session.rs index 920b79d7e..b207ef993 100644 --- a/app/src-tauri/src/cdp/session.rs +++ b/app/src-tauri/src/cdp/session.rs @@ -30,8 +30,8 @@ use crate::webview_accounts::emit_load_finished; const ATTACH_BACKOFF: Duration = Duration::from_secs(2); /// Watchdog budget before we synthesise a `webview-account:load` event with -/// `state: "timeout"` so the frontend never holds its loading spinner open on -/// a flaky network. Matches the timeout documented in issue #867. +/// `state: "timeout"` so the frontend can switch from an empty loading state +/// to explicit retry/help UI on flaky networks. Matches issue #867. const LOAD_TIMEOUT: Duration = Duration::from_secs(15); /// Returns the unique marker substring that the account's initial @@ -120,11 +120,13 @@ pub fn spawn_session( // CDP `Page.loadEventFired` signal arrives (flaky network, provider // blocking, CDP socket hiccup). // - // `emit_load_finished` dedups via `WebviewAccountsState.loaded_accounts` - // so a late watchdog is a no-op once either signal has fired. The - // returned `JoinHandle` is stored in `WebviewAccountsState.load_watchdogs` - // and aborted on close/purge so a watchdog spawned for a vanished - // account can't fire a stale timeout against a freshly-reused id. + // `emit_load_finished` only treats terminal load signals (`finished`) as + // dedup markers. If the watchdog fires first, frontend sees `timeout` + // and can show retry UI; a later real `finished` signal can still reveal. + // Late watchdogs after a terminal load are no-ops. The returned + // `JoinHandle` is stored in `WebviewAccountsState.load_watchdogs` and + // aborted on close/purge so a watchdog spawned for a vanished account + // can't fire a stale timeout against a freshly-reused id. let watchdog = { let app = app.clone(); let account_id = account_id.clone(); diff --git a/app/src-tauri/src/webview_accounts/mod.rs b/app/src-tauri/src/webview_accounts/mod.rs index 85f0a4e33..f8c081f00 100644 --- a/app/src-tauri/src/webview_accounts/mod.rs +++ b/app/src-tauri/src/webview_accounts/mod.rs @@ -1080,12 +1080,17 @@ fn redact_url_for_log(raw: &str) -> String { } /// Grow the first-cold-open webview back to its full requested bounds and -/// notify the frontend — exactly once per account open. Called from the -/// three independent signals (native `WebviewBuilder::on_page_load`, CDP -/// `Page.loadEventFired`, 15 s watchdog) — the first one wins and the rest -/// short-circuit via `WebviewAccountsState.loaded_accounts`. Resetting -/// happens in `webview_account_close` / `webview_account_purge` so a reopen -/// fires again. +/// notify the frontend once the page is actually loaded. Called from three +/// signals (native `WebviewBuilder::on_page_load`, CDP `Page.loadEventFired`, +/// and the 15 s watchdog). +/// +/// Timeout is a non-terminal state: we emit `webview-account:load{state: +/// "timeout"}` so the frontend can show retry/help UI, but we deliberately do +/// NOT reveal or mark the account as loaded yet. If a later `finished` signal +/// arrives, that call still reveals and emits `state:"finished"`. +/// +/// Resetting the terminal loaded marker happens in `webview_account_close` / +/// `webview_account_purge` so a reopen fires again. /// /// Doing the `set_size` server-side (instead of waiting for the frontend to /// invoke `webview_account_reveal`) avoids an extra IPC round-trip and the @@ -1110,12 +1115,50 @@ pub(crate) fn emit_load_finished( return; }; - let is_first = app_state + if state == "timeout" { + // If we've already observed a terminal load, ignore late watchdogs. + let already_loaded = app_state + .loaded_accounts + .lock() + .unwrap() + .contains(account_id); + if already_loaded { + log::debug!( + "[webview-accounts][{}] timeout deduped after terminal load url={}", + account_id, + url + ); + return; + } + + log::info!( + "[webview-accounts][{}] load timeout event url={}", + account_id, + redact_url_for_log(url) + ); + if let Err(err) = app.emit( + "webview-account:load", + serde_json::json!({ + "account_id": account_id, + "state": state, + "url": url, + }), + ) { + log::warn!( + "[webview-accounts][{}] emit webview-account:load(timeout) failed: {}", + account_id, + err + ); + } + return; + } + + let is_first_terminal = app_state .loaded_accounts .lock() .unwrap() .insert(account_id.to_string()); - if !is_first { + if !is_first_terminal { log::debug!( "[webview-accounts][{}] load event deduped state={} url={}", account_id, @@ -1578,25 +1621,19 @@ pub async fn webview_account_open( builder = builder.devtools(true); } - // Wire the native page-load signal so the frontend can hide its spinner as - // soon as CEF's LoadHandler reports the main frame finished. Dedup against - // the CDP `Page.loadEventFired` subscription and the 15 s watchdog through - // `emit_load_finished` so we only fire the first winning signal per open. + // Wire the native page-load signal and forward only *usable* load + // completions to `emit_load_finished`: + // - skip placeholder `about:blank#openhuman-acct-*` commits (otherwise + // we reveal a blank viewport before real content arrives), + // - treat Chromium network error pages (`chrome-error://…`) as timeout + // signals so frontend shows retry/help UI instead of the dino page. // - // Skip `data:` URLs: an early placeholder shape on some platforms - // fires `Finished` synchronously and we don't want to dedup-claim - // the load slot on it. - // - // We deliberately do NOT skip `about:blank#…` here: in practice the - // CDP `Page.loadEventFired` subscription isn't reaching us reliably - // (Gmail finishes loading but the event doesn't fire through - // pump_events — separate triage). The `about:blank` native load - // fires within ~50ms of spawn and is the only fast-path reveal we - // get. The downside is the user sees the placeholder briefly until - // the real provider page paints — better than waiting 15 s for the - // watchdog timeout. + // Real provider commits still emit `finished`. Dedup against CDP + // `Page.loadEventFired` + watchdog happens in `emit_load_finished`. let page_load_app = app.clone(); let page_load_account_id = args.account_id.clone(); + let page_load_placeholder_fragment = format!("#{}", cdp::placeholder_marker(&args.account_id)); + let page_load_real_url = real_url_str.clone(); builder = builder.on_page_load(move |_webview, payload| { if !matches!(payload.event(), tauri::webview::PageLoadEvent::Finished) { return; @@ -1605,6 +1642,23 @@ pub async fn webview_account_open( if url.scheme() == "data" { return; } + if !skip_cdp_for_debug && url.as_str().ends_with(&page_load_placeholder_fragment) { + log::debug!( + "[webview-accounts][{}] skipping placeholder native-finished url={}", + page_load_account_id, + redact_url_for_log(url.as_str()) + ); + return; + } + if url.scheme() == "chrome-error" { + emit_load_finished( + &page_load_app, + &page_load_account_id, + "timeout", + &page_load_real_url, + ); + return; + } emit_load_finished( &page_load_app, &page_load_account_id, diff --git a/app/src/components/accounts/WebviewHost.tsx b/app/src/components/accounts/WebviewHost.tsx index 12ecf9ea1..a761a8770 100644 --- a/app/src/components/accounts/WebviewHost.tsx +++ b/app/src/components/accounts/WebviewHost.tsx @@ -4,6 +4,7 @@ import { useEffect, useRef } from 'react'; import { hideWebviewAccount, openWebviewAccount, + retryWebviewAccountLoad, setWebviewAccountBounds, } from '../../services/webviewAccountService'; import { useAppSelector } from '../../store/hooks'; @@ -18,6 +19,18 @@ interface WebviewHostProps { const LOADING_STATUSES: ReadonlySet = new Set(['pending', 'loading']); +const PROVIDER_COPY: Record = { + whatsapp: 'WhatsApp', + telegram: 'Telegram', + linkedin: 'LinkedIn', + gmail: 'Gmail', + slack: 'Slack', + discord: 'Discord', + 'google-meet': 'Google Meet', + zoom: 'Zoom', + browserscan: 'BrowserScan', +}; + /** * Reserves a rectangular slot in the React layout that the native child * webview is glued to. We measure the placeholder's bounding rect and @@ -44,6 +57,8 @@ const WebviewHost = ({ accountId, provider }: WebviewHostProps) => { // the `setAccountStatus('pending')` dispatch in `openWebviewAccount` is // visually indistinguishable from no overlay, so this is safe. const isLoading = status !== undefined && LOADING_STATUSES.has(status); + const isTimeout = status === 'timeout'; + const providerName = PROVIDER_COPY[provider] ?? 'app'; // Spawn / show + keep bounds synced on every layout change. // IMPORTANT: both refs are reset on cleanup so switching accountIds @@ -133,7 +148,33 @@ const WebviewHost = ({ accountId, provider }: WebviewHostProps) => { aria-live="polite" aria-label="Loading account">
- Loading… + {`Loading ${providerName}...`} +
+ ) : null} + + {isTimeout ? ( +
+
+

{`${providerName} is taking longer than expected.`}

+

+ The embedded app may still be starting up. Retry to reload it without signing in + again. +

+
+
) : null} diff --git a/app/src/services/__tests__/webviewAccountService.loadListener.test.ts b/app/src/services/__tests__/webviewAccountService.loadListener.test.ts index a70eeb2cb..bf2d0967e 100644 --- a/app/src/services/__tests__/webviewAccountService.loadListener.test.ts +++ b/app/src/services/__tests__/webviewAccountService.loadListener.test.ts @@ -6,6 +6,7 @@ import { addAccount, resetAccountsState } from '../../store/accountsSlice'; import { closeWebviewAccount, openWebviewAccount, + retryWebviewAccountLoad, setWebviewAccountBounds, startWebviewAccountService, stopWebviewAccountService, @@ -136,19 +137,46 @@ describe('webviewAccountService load listener', () => { }); }); - it('still reveals on timeout fallback', async () => { + it('marks account timeout and skips reveal on timeout fallback', async () => { const bounds = { x: 0, y: 0, width: 800, height: 600 }; await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds }); vi.mocked(invoke).mockClear(); await fireLoadEvent({ state: 'timeout', url: '' }); + expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('webview_account_reveal', expect.anything()); + expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('timeout'); + }); + + it('can recover from timeout when a later finished signal arrives', async () => { + const bounds = { x: 11, y: 22, width: 810, height: 610 }; + await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds }); + vi.mocked(invoke).mockClear(); + + await fireLoadEvent({ state: 'timeout', url: '' }); + expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('timeout'); + expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('webview_account_reveal', expect.anything()); + + await fireLoadEvent({ state: 'finished', url: 'https://web.telegram.org/' }); expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_reveal', { args: { account_id: ACCOUNT_ID, bounds }, }); expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('open'); }); + it('treats chromium offline error-page URLs as timeout and hides webview', async () => { + const bounds = { x: 7, y: 9, width: 800, height: 620 }; + await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds }); + vi.mocked(invoke).mockClear(); + + await fireLoadEvent({ state: 'finished', url: 'chrome-error://chromewebdata/' }); + + expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_hide', { + args: { account_id: ACCOUNT_ID }, + }); + expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('timeout'); + }); + it('treats `reused` event as finished (warm re-open path)', async () => { const bounds = { x: 0, y: 0, width: 800, height: 600 }; await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds }); @@ -170,4 +198,17 @@ describe('webviewAccountService load listener', () => { expect(vi.mocked(invoke)).not.toHaveBeenCalled(); }); + + it('retry re-opens with cached bounds and provider', async () => { + const bounds = { x: 0, y: 0, width: 920, height: 700 }; + await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds }); + vi.mocked(invoke).mockClear(); + + await retryWebviewAccountLoad(ACCOUNT_ID, 'telegram'); + + expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_open', { + args: { account_id: ACCOUNT_ID, provider: 'telegram', bounds }, + }); + expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('loading'); + }); }); diff --git a/app/src/services/webviewAccountService.ts b/app/src/services/webviewAccountService.ts index 26335963b..5f8e2d20a 100644 --- a/app/src/services/webviewAccountService.ts +++ b/app/src/services/webviewAccountService.ts @@ -64,7 +64,7 @@ interface NotificationClickPayload { interface WebviewAccountLoadPayload { account_id: string; // `'finished'` — native `on_page_load` or CDP `Page.loadEventFired` fired - // `'timeout'` — 15 s watchdog elapsed; reveal anyway so spinner isn't stuck + // `'timeout'` — 15 s watchdog elapsed; keep hidden and show retry UI // `'reused'` — warm re-open of already-loaded account; reveal synchronously state: 'finished' | 'timeout' | 'reused' | string; url: string; @@ -105,6 +105,12 @@ const lastBoundsByAccount = new Map(); // to the on-screen rect prematurely would defeat the loading overlay. const loadingAccounts = new Set(); +function looksLikeChromiumErrorUrl(rawUrl: string | undefined | null): boolean { + if (!rawUrl) return false; + const u = rawUrl.toLowerCase(); + return u.startsWith('chrome-error://') || u.includes('chromewebdata'); +} + export function startWebviewAccountService(): void { if (started) return; if (!isTauri()) { @@ -178,6 +184,21 @@ function handleWebviewAccountLoad(payload: WebviewAccountLoadPayload) { log('load event account=%s state=%s url=%s', accountId, payload.state, payload.url); loadingAccounts.delete(accountId); + const timeoutLike = + payload.state === 'timeout' || + (payload.state === 'finished' && looksLikeChromiumErrorUrl(payload.url)); + + if (timeoutLike) { + log('load timeout account=%s reason=%s url=%s', accountId, payload.state, payload.url); + // Force-hide the child webview so the timeout overlay is visible even if + // the provider loaded a Chromium internal error page (`chromewebdata`). + void invoke('webview_account_hide', { args: { account_id: accountId } }).catch(err => { + errLog('webview_account_hide failed during timeout account=%s: %o', accountId, err); + }); + store.dispatch(setAccountStatus({ accountId, status: 'timeout' })); + return; + } + // Rust already resized the webview to `requested_bounds` as part of // `emit_load_finished`, so the native side is already correct. We still // issue `webview_account_reveal` here as a belt-and-braces idempotent @@ -191,6 +212,7 @@ function handleWebviewAccountLoad(payload: WebviewAccountLoadPayload) { // error we still flip to `'open'` so the spinner never hangs indefinitely — // the webview will have been positioned server-side by `emit_load_finished`. const bounds = lastBoundsByAccount.get(accountId); + log('load finished account=%s state=%s reveal=%s', accountId, payload.state, Boolean(bounds)); if (bounds) { invoke('webview_account_reveal', { args: { account_id: accountId, bounds } }) .catch(err => { @@ -807,7 +829,7 @@ interface OpenAccountArgs { export async function openWebviewAccount(args: OpenAccountArgs): Promise { if (!isTauri()) throw new Error('webview accounts require the desktop app'); - log('open account=%s provider=%s', args.accountId, args.provider); + log('load start account=%s provider=%s', args.accountId, args.provider); store.dispatch(setAccountStatus({ accountId: args.accountId, status: 'pending' })); lastBoundsByAccount.set(args.accountId, args.bounds); loadingAccounts.add(args.accountId); @@ -833,6 +855,23 @@ export async function openWebviewAccount(args: OpenAccountArgs): Promise { } } +/** + * Retry a stalled initial load for an embedded webview account while preserving + * the existing profile/session cookies on disk. + */ +export async function retryWebviewAccountLoad( + accountId: string, + provider: AccountProvider +): Promise { + const bounds = lastBoundsByAccount.get(accountId); + if (!bounds) { + errLog('retry skipped: missing bounds account=%s provider=%s', accountId, provider); + return; + } + log('retry load account=%s provider=%s', accountId, provider); + await openWebviewAccount({ accountId, provider, bounds }); +} + export async function setWebviewAccountBounds( accountId: string, bounds: WebviewAccountBounds diff --git a/app/src/types/accounts.ts b/app/src/types/accounts.ts index 8c9de025e..b744cc272 100644 --- a/app/src/types/accounts.ts +++ b/app/src/types/accounts.ts @@ -15,10 +15,11 @@ export type AccountProvider = // 'pending' — openWebviewAccount invoked, Rust-side add_child not yet confirmed // 'loading' — CEF child webview spawned off-screen, waiting for first page-loaded // signal; WebviewHost shows its spinner +// 'timeout' — initial load watchdog elapsed; keep overlay visible and let user retry // 'open' — page loaded, webview_account_reveal completed, webview on-screen // 'closed' — webview destroyed // 'error' — open/reveal failed (lastError populated) -export type AccountStatus = 'pending' | 'loading' | 'open' | 'error' | 'closed'; +export type AccountStatus = 'pending' | 'loading' | 'timeout' | 'open' | 'error' | 'closed'; export interface Account { id: string;