mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
feat(webview): add timeout state and retry UI for stalled embedded webview loads (#1043)
This commit is contained in:
@@ -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<R: Runtime>(
|
||||
// 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();
|
||||
|
||||
@@ -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<R: Runtime>(
|
||||
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<R: Runtime>(
|
||||
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<R: Runtime>(
|
||||
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,
|
||||
|
||||
@@ -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<AccountStatus> = new Set(['pending', 'loading']);
|
||||
|
||||
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',
|
||||
};
|
||||
|
||||
/**
|
||||
* 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">
|
||||
<div className="h-8 w-8 animate-spin rounded-full border-2 border-stone-300 border-t-stone-600" />
|
||||
<span className="text-xs font-medium tracking-wide">Loading…</span>
|
||||
<span className="text-xs font-medium tracking-wide">{`Loading ${providerName}...`}</span>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{isTimeout ? (
|
||||
<div
|
||||
data-testid={`webview-timeout-${accountId}`}
|
||||
className="absolute inset-0 z-10 flex flex-col items-center justify-center gap-4 bg-stone-50/95 px-6 text-center"
|
||||
role="status"
|
||||
aria-live="polite"
|
||||
aria-label="Webview load timeout">
|
||||
<div className="max-w-sm space-y-1">
|
||||
<p className="text-sm font-semibold text-stone-800">{`${providerName} is taking longer than expected.`}</p>
|
||||
<p className="text-xs text-stone-600">
|
||||
The embedded app may still be starting up. Retry to reload it without signing in
|
||||
again.
|
||||
</p>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => {
|
||||
log('retry clicked account=%s provider=%s', accountId, provider);
|
||||
void retryWebviewAccountLoad(accountId, provider);
|
||||
}}
|
||||
className="rounded-md bg-primary-600 px-3 py-1.5 text-xs font-semibold text-white transition-colors hover:bg-primary-700">
|
||||
Retry loading
|
||||
</button>
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, WebviewAccountBounds>();
|
||||
// to the on-screen rect prematurely would defeat the loading overlay.
|
||||
const loadingAccounts = new Set<string>();
|
||||
|
||||
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<void> {
|
||||
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<void> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<void> {
|
||||
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
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user