From 69b62a83d241405d8d9be5e418bbb45faa51a3fd Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Tue, 8 Sep 2026 14:23:49 +0800 Subject: [PATCH] fix(gui): contain a failed lazy chunk instead of taking down the app Suspense covers only the pending half of a dynamic import. When the chunk fails to load the promise rejects and the error propagates to the nearest boundary -- and neither lazy() site had one: the diff editor in PrPanel and a comment''s inline code context in CommentItem. DiffView carries an internal boundary around DiffPane, but that cannot see its own chunk failing to arrive. So a Monaco snippet that could not be fetched cost the user the entire application. Each lazy subtree now owns a LazyBoundary (Suspense plus a boundary) and the failure stays inside the pane that could not load. A stale chunk is singled out. The window holds a hashed module URL the app no longer has -- it was rebuilt or updated while the window stayed open -- so re-rendering re-requests the same dead URL and fails identically; only a reload recovers. Offering "retry" there is offering a button that cannot work, so isChunkLoadError detects the case and both the pane fallback and the root crash screen lead with reload and say why. Other errors keep retry. Observed via the root crash screen added earlier, which reported the failing module by name -- the diagnosis this containment work was meant to make possible. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + CHANGELOG.zh-CN.md | 1 + .../src/components/common/AppCrashScreen.tsx | 29 ++++--- .../src/components/common/LazyBoundary.tsx | 75 +++++++++++++++++++ .../renderer/src/components/common/index.ts | 1 + .../src/components/features/pr/PrPanel.tsx | 11 ++- .../features/pr/tabs/comments/CommentItem.tsx | 10 ++- .../src/renderer/src/i18n/locales/de-DE.json | 7 ++ .../src/renderer/src/i18n/locales/en-US.json | 7 ++ .../src/renderer/src/i18n/locales/ja-JP.json | 7 ++ .../src/renderer/src/i18n/locales/zh-CN.json | 7 ++ .../src/renderer/src/styles/common/crash.scss | 19 +++++ docs/arch/03-gui/01-ui-interaction.md | 5 +- 13 files changed, 161 insertions(+), 19 deletions(-) create mode 100644 apps/desktop/src/renderer/src/components/common/LazyBoundary.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 248103c2..afa860cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and the versioning follows [Semantic Versioning](https://semver.org/). ### 🔧 Fixed +- A part of the interface that loads on demand — the diff editor, a comment's inline code context — no longer takes the whole app down with it when it fails to load; the failure now stays inside that pane. If it failed because the app was updated or rebuilt while the window was open, it says so and offers to reload, which is the only thing that actually helps in that case. - A failed review now shows the provider's actual error instead of only "all fallback models failed" — the real cause (an unavailable model, an expired login, an exhausted quota) was previously swallowed and never reached the run card. - A local CLI provider that exits successfully but returns an empty reply is now reported as a failure naming that cause, rather than as an unexplained LLM failure. - A merged PR now leaves the list on its own shortly after you merge it, instead of lingering until the next periodic sync — the remote takes a few seconds to actually mark it merged, and the app now waits for that rather than refreshing too early and finding nothing changed. diff --git a/CHANGELOG.zh-CN.md b/CHANGELOG.zh-CN.md index 55e47be0..a0091c78 100644 --- a/CHANGELOG.zh-CN.md +++ b/CHANGELOG.zh-CN.md @@ -15,6 +15,7 @@ ### 🔧 修复 +- 按需加载的界面部分——diff 编辑器、评论中的内联代码上下文——加载失败时不再拖垮整个应用，失败被限制在该区域内。若失败原因是窗口开着时应用被更新或重新构建，会明确说明并提供重新加载，那也是这种情况下唯一有效的操作。 - 评审失败时现在会展示供应商返回的真实错误，而不再只有一句「所有备选模型均调用失败」——真正的原因（模型不可用、登录过期、额度耗尽）此前被吞掉，从未出现在运行卡片上。 - 本地 CLI 供应商正常退出却返回空回复时，现在会作为失败上报并指明该原因，而不再表现为一次无从解释的 LLM 调用失败。 - 合并 PR 后，该 PR 会在稍后自动从列表中消失，而不再滞留到下一次周期同步——远端需要几秒才真正标记为已合并，应用现在会等待这一刻，而不是过早刷新、结果什么都没变。 diff --git a/apps/desktop/src/renderer/src/components/common/AppCrashScreen.tsx b/apps/desktop/src/renderer/src/components/common/AppCrashScreen.tsx index 087edb2d..401ce60f 100644 --- a/apps/desktop/src/renderer/src/components/common/AppCrashScreen.tsx +++ b/apps/desktop/src/renderer/src/components/common/AppCrashScreen.tsx @@ -1,30 +1,41 @@ import { useTranslation } from 'react-i18next'; +import { isChunkLoadError } from './LazyBoundary'; /** * Full-window fallback for a crash in the app's root subtree. * * Without a boundary at the root, any error thrown while rendering unmounts the whole tree and leaves an empty `#root` * — which reads as a permanently black window, with no way back short of restarting the app. This screen is what the - * user gets instead: what broke, and two ways out. + * user gets instead: what broke, and a way out. * - * `onRetry` re-renders the subtree, which is enough when the crash came from transient state (a stale record read - * during an in-flight update); reloading rebuilds the renderer from scratch and is the way out when it did not. + * The way out depends on the failure. `onRetry` re-renders the subtree, which is enough when the crash came from + * transient state (a stale record read during an in-flight update). A **stale chunk** is the exception: the page holds + * a hashed module URL that no longer exists (the app was rebuilt or updated while this window stayed open), so + * re-rendering re-requests the same dead URL and fails identically — only a reload recovers. Offering "retry" there + * would be offering a button that cannot work, so that case leads with reload and explains why. */ export function AppCrashScreen({ err, onRetry }: { err: Error; onRetry: () => void }) { const { t } = useTranslation(); + const stale = isChunkLoadError(err); return (
-

{t('crash.title')}

-

{t('crash.hint')}

+

{stale ? t('crash.staleTitle') : t('crash.title')}

+

{stale ? t('crash.staleHint') : t('crash.hint')}

{err.message || String(err)}
- - + {!stale && ( + + )}
diff --git a/apps/desktop/src/renderer/src/components/common/LazyBoundary.tsx b/apps/desktop/src/renderer/src/components/common/LazyBoundary.tsx new file mode 100644 index 00000000..29d6c104 --- /dev/null +++ b/apps/desktop/src/renderer/src/components/common/LazyBoundary.tsx @@ -0,0 +1,75 @@ +import { Suspense, type ReactNode } from 'react'; +import { useTranslation } from 'react-i18next'; +import { ErrorBoundary } from './ErrorBoundary'; + +/** + * Whether an error is a dynamic-import failure — the chunk behind a `lazy()` could not be fetched. + * + * Worth distinguishing because the remedy is the opposite of the usual one: the page is holding a hashed chunk URL that + * no longer exists on disk (the app was rebuilt or updated while this window stayed open), so **retrying re-requests the + * same dead URL and fails again**. Only a reload — which re-reads the entry and picks up current hashes — recovers. + * + * The message differs per engine, hence matching several forms rather than one. + */ +export function isChunkLoadError(err: Error): boolean { + const msg = err.message.toLowerCase(); + return ( + msg.includes('failed to fetch dynamically imported module') || // Chromium (Electron) + msg.includes('error loading dynamically imported module') || // Firefox + msg.includes('importing a module script failed') || // Safari + msg.includes('unable to preload') // Vite's preload helper + ); +} + +/** + * Suspense + an error boundary around a `lazy()` subtree. + * + * `Suspense` alone only covers the *pending* half of a lazy import: if the chunk fails to load, the promise rejects and + * the error propagates to the nearest boundary. With no boundary in between it reaches the root one and takes the whole + * app down — a Monaco snippet failing to load should not cost the user the comment thread around it. So each lazy + * subtree gets its own boundary, and the failure stays inside the pane that could not load. + */ +export function LazyBoundary({ + label, + loading, + children, +}: { + /** Names the failing region in logs (see ErrorBoundary). */ + label: string; + /** Rendered while the chunk is in flight. */ + loading: ReactNode; + children: ReactNode; +}) { + const { t } = useTranslation(); + return ( + ( +
+

{t('lazyLoad.failed')}

+

+ {isChunkLoadError(err) ? t('lazyLoad.staleHint') : t('lazyLoad.genericHint')} +

+
+ {/* Retry is offered only when it can actually work: for a stale chunk it would re-request the same dead + URL, so that case leads with reload instead. */} + {!isChunkLoadError(err) && ( + + )} + +
+
+ )} + > + {children} + + ); +} diff --git a/apps/desktop/src/renderer/src/components/common/index.ts b/apps/desktop/src/renderer/src/components/common/index.ts index 781175ee..f9ab8137 100644 --- a/apps/desktop/src/renderer/src/components/common/index.ts +++ b/apps/desktop/src/renderer/src/components/common/index.ts @@ -7,6 +7,7 @@ export * from './Avatar'; export * from './BitbucketImage'; export * from './ConfirmModal'; export * from './ErrorBoundary'; +export * from './LazyBoundary'; export * from './LlmProviderIcon'; export * from './Loading'; export * from './MermaidDiagram'; diff --git a/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx b/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx index 04155f61..343acb81 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx @@ -1,4 +1,4 @@ -import { lazy, Suspense, useEffect, useMemo, useRef, useState, type ReactNode } from 'react'; +import { lazy, useEffect, useMemo, useRef, useState, type ReactNode } from 'react'; import { useTranslation } from 'react-i18next'; import type { LocalPrStatus, @@ -10,7 +10,7 @@ import type { } from '@meebox/shared'; import { invoke } from '../../../api'; import { useDraftsForPr } from '../../../stores/drafts-store'; -import { PaneLoading } from '../../common'; +import { LazyBoundary, PaneLoading } from '../../common'; import { ActivityPanel } from './tabs/activity/ActivityPanel'; import { CommitsPanel } from './tabs/CommitsPanel'; // Monaco editor (~10MB) lazy-loaded: the DiffView chunk is fetched only when actually switching to the Diff tab, @@ -211,7 +211,10 @@ export function PrPanel({ {/* keep-alive: each tab mounts only on first visit, then stays alive with only CSS show/hide (see KeepAliveTab). Switching away and back is instant, no refetch, embedded Monaco / scroll position / expanded state all preserved, eliminating switch jitter. */} - }> + } + > setPendingCommitView(null)} onViewCommitScopeChange={onViewCommitScopeChange} /> - + {t('commentsPanel.loadingCodeContext')}} + {t('commentsPanel.loadingCodeContext')}} > - + ) : null; // Edit mode: textarea replaces the markdown body in place; non-edit mode: render markdown diff --git a/apps/desktop/src/renderer/src/i18n/locales/de-DE.json b/apps/desktop/src/renderer/src/i18n/locales/de-DE.json index e4da190c..8a4be21d 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/de-DE.json +++ b/apps/desktop/src/renderer/src/i18n/locales/de-DE.json @@ -329,6 +329,8 @@ "hint": "Ein Neuladen behebt das in der Regel. Tritt es wiederholt auf, benennen die Details unten und das Anwendungsprotokoll (meebox.log) die Ursache.", "reload": "Neu laden", "retry": "Erneut versuchen", + "staleHint": "Ein Teil der Oberfläche konnte nicht mehr geladen werden — das passiert, wenn die Anwendung bei geöffnetem Fenster aktualisiert oder neu gebaut wird. Ein Neuladen übernimmt die aktuelle Version.", + "staleTitle": "Die Anwendungsdateien haben sich geändert, seit dieses Fenster geöffnet wurde", "title": "Beim Rendern der Oberfläche ist ein Fehler aufgetreten" }, "diffSearchPanel": { @@ -502,6 +504,11 @@ "expandTitle": "Code um die verankerte Zeile erweitern", "loading": "Code-Kontext wird geladen…" }, + "lazyLoad": { + "failed": "Dieser Teil der Oberfläche konnte nicht geladen werden.", + "genericHint": "Ein erneuter Versuch hilft meistens; schlägt es weiterhin fehl, laden Sie das Fenster neu.", + "staleHint": "Die Anwendungsdateien haben sich seit dem Öffnen dieses Fensters geändert — meist durch ein Update oder einen Neubau. Ein Neuladen übernimmt die aktuelle Version." + }, "llmProfileForm": { "apiKeyOptionalPlaceholder": "Dieser Provider benötigt keinen Schlüssel (leer lassen)", "cliCommandFallback": "Kommandozeilen-Tool", diff --git a/apps/desktop/src/renderer/src/i18n/locales/en-US.json b/apps/desktop/src/renderer/src/i18n/locales/en-US.json index 3f981bd5..ddff7557 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/en-US.json +++ b/apps/desktop/src/renderer/src/i18n/locales/en-US.json @@ -329,6 +329,8 @@ "hint": "Reloading usually resolves it. If it keeps happening, the details below and the application log (meebox.log) identify the cause.", "reload": "Reload", "retry": "Retry", + "staleHint": "Part of the interface could no longer be loaded, which happens when the app is updated or rebuilt while a window stays open. Reloading picks up the current version.", + "staleTitle": "The app files changed since this window opened", "title": "Something went wrong rendering the interface" }, "diffSearchPanel": { @@ -502,6 +504,11 @@ "expandTitle": "Expand code around the anchored line", "loading": "Loading code context…" }, + "lazyLoad": { + "failed": "This part of the interface could not be loaded.", + "genericHint": "Retrying often works; if it keeps failing, reload the window.", + "staleHint": "The app files changed since this window opened — usually an update or a rebuild. Reloading picks up the current version." + }, "llmProfileForm": { "apiKeyOptionalPlaceholder": "This provider needs no key (leave empty)", "cliCommandFallback": "command-line tool", diff --git a/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json b/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json index 71f160b1..1b440396 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json +++ b/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json @@ -323,6 +323,8 @@ "hint": "再読み込みで通常は復旧します。繰り返し発生する場合は、以下の詳細とアプリケーションログ（meebox.log）で原因を特定できます。", "reload": "再読み込み", "retry": "再試行", + "staleHint": "画面の一部を読み込めなくなりました。ウィンドウを開いたままアプリが更新または再ビルドされた場合に発生します。再読み込みすると最新版が読み込まれます。", + "staleTitle": "ウィンドウを開いた後にアプリのファイルが変更されました", "title": "画面の描画中にエラーが発生しました" }, "diffSearchPanel": { @@ -491,6 +493,11 @@ "expandTitle": "アンカー行の前後のコードを展開", "loading": "コードコンテキストを読み込み中…" }, + "lazyLoad": { + "failed": "この部分の画面を読み込めませんでした。", + "genericHint": "再試行で復旧することが多く、繰り返し失敗する場合はウィンドウを再読み込みしてください。", + "staleHint": "ウィンドウを開いた後にアプリのファイルが変更されました（通常は更新または再ビルド）。再読み込みすると最新版が読み込まれます。" + }, "llmProfileForm": { "apiKeyOptionalPlaceholder": "このプロバイダーはキー不要です（空のままに）", "cliCommandFallback": "コマンドラインツール", diff --git a/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json b/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json index 48503162..daf19ceb 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json +++ b/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json @@ -323,6 +323,8 @@ "hint": "重新加载通常即可恢复。若反复出现，下方详情与应用日志（meebox.log）可定位原因。", "reload": "重新加载", "retry": "重试", + "staleHint": "部分界面已无法加载——通常是窗口开着时应用被更新或重新构建所致。重新加载即可载入当前版本。", + "staleTitle": "应用文件在窗口打开后发生了变化", "title": "界面渲染出错" }, "diffSearchPanel": { @@ -491,6 +493,11 @@ "expandTitle": "展开锚定行前后代码", "loading": "加载代码上下文…" }, + "lazyLoad": { + "failed": "这部分界面加载失败。", + "genericHint": "重试通常可以恢复；若持续失败，请重新加载窗口。", + "staleHint": "应用文件在窗口打开后发生了变化——通常是更新或重新构建所致。重新加载即可载入当前版本。" + }, "llmProfileForm": { "apiKeyOptionalPlaceholder": "该 provider 无需密钥（留空）", "cliCommandFallback": "命令行", diff --git a/apps/desktop/src/renderer/src/styles/common/crash.scss b/apps/desktop/src/renderer/src/styles/common/crash.scss index 837dfde4..5546af5f 100644 --- a/apps/desktop/src/renderer/src/styles/common/crash.scss +++ b/apps/desktop/src/renderer/src/styles/common/crash.scss @@ -56,3 +56,22 @@ display: flex; gap: $space-2; } + +// Failure of one lazily-loaded pane (LazyBoundary). Sized to sit inside whatever region failed — a comment's code +// context, the diff pane — rather than taking over the window like .app-crash does. +.lazy-boundary-error { + padding: $space-8; + color: $text-muted; + font-size: $fs-xs; + line-height: 1.5; + + p { + margin: 0 0 $space-2; + } +} + +.lazy-boundary-actions { + display: flex; + gap: $space-2; + margin-top: $space-3; +} diff --git a/docs/arch/03-gui/01-ui-interaction.md b/docs/arch/03-gui/01-ui-interaction.md index ffb36453..e89b6ab7 100644 --- a/docs/arch/03-gui/01-ui-interaction.md +++ b/docs/arch/03-gui/01-ui-interaction.md @@ -90,10 +90,11 @@ Platform differences are decided via `AppInfo.platform` (delivered by the main p - **Second-level modal backdrop click only closes its own layer**: for nested modals (connection/LLM/proxy editing, confirm dialogs) the backdrop click calls `stopPropagation`, so it does not bubble up to close the outer settings modal (including createPortal confirm dialogs — React synthetic events still bubble along the component tree). - **Action-level toast vs. full-screen error**: a failed remote action (review decision/merge/publish) raises a toast, distinct from the full-screen error of a fatal bootstrap failure. -- **Never a blank window (three layers of failure containment)**: a renderer failure must always land on a readable screen, never on the window's bare background — which is indistinguishable from a hung app and has no way back short of a restart. Each layer covers what the one below it structurally cannot see: +- **Never a blank window (four layers of failure containment)**: a renderer failure must always land on a readable screen, never on the window's bare background — which is indistinguishable from a hung app and has no way back short of a restart. Each layer covers what the one below it structurally cannot see: 1. **Render-phase errors** → the root `ErrorBoundary` in `main.tsx` (fallback `AppCrashScreen`, offering retry / reload). Without one, any error thrown while rendering unmounts the whole tree and empties `#root`. The boundary also relays the stack through `log:write`, since the renderer console is never written to file and a caught render error is not an uncaught window error either — so a crash would otherwise leave nothing on disk to diagnose. 2. **Failures before React mounts** → `boot-guard.ts`, imported **first** in `main.tsx` so it is armed before any other module can throw. A module that fails while initializing (i18n / theme / the app bundle) takes down the entry before `render()` runs, leaving no React and therefore no boundary. The guard paints a plain-DOM recovery screen — no React, no i18n, no stylesheet, since each of those is a candidate cause — on an uncaught error or when `#root` is still empty after a timeout. - 3. **Renderer process death** → `WindowManager.installCrashRecovery` in main (`render-process-gone` / `did-fail-load`), which logs and reloads the window with a bounded retry budget. When the process itself dies (OOM, GPU fault, a native crash), no in-page JavaScript survives to report it; only main is left watching. `unresponsive` is logged but deliberately not recovered — it usually resolves on its own, and reloading would discard the user's in-flight state. + 3. **A lazily-loaded subtree failing to load** → `LazyBoundary` (Suspense + a boundary) around each `lazy()` site. `Suspense` alone covers only the *pending* half of a dynamic import; a rejected chunk propagates to the nearest boundary, and with none in between it reaches the root one — so a Monaco snippet that fails to fetch costs the user the entire app. Each lazy subtree therefore owns a boundary and the failure stays inside the pane that could not load. A **stale chunk** (the window holds a hashed module URL the rebuilt/updated app no longer has) is called out separately by `isChunkLoadError`, because there retrying re-requests the same dead URL and only a reload recovers — the fallback and the root crash screen both lead with reload in that case rather than offering a button that cannot work. + 4. **Renderer process death** → `WindowManager.installCrashRecovery` in main (`render-process-gone` / `did-fail-load`), which logs and reloads the window with a bounded retry budget. When the process itself dies (OOM, GPU fault, a native crash), no in-page JavaScript survives to report it; only main is left watching. `unresponsive` is logged but deliberately not recovered — it usually resolves on its own, and reloading would discard the user's in-flight state. - **Auto-refresh on window focus**: when the window regains focus, proactively fetch PR meta once (to follow the "switch to the platform, make edits, then switch back" scenario). - **Layout preferences persisted**: sidebar/chat width and collapse state, diff view mode, etc. are stored in localStorage.