From f9d4822204ce2d9236ae50bf82877a7d0cc3ac8e Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 21 Sep 2026 19:32:28 -0400 Subject: [PATCH] fix(mobile): catch the diff-comments loader rejection at the effect `use-mobile-session-diff-comments.ts` ran `void loadDiffComments()` with no catch, so a *rejected* `worktree.show` raised an unhandled rejection on every session mount: a document-level error, not a page fault, and a red herring in crash reports and device proofs. The catch goes at the effect rather than inside the loader, whose promise the recording adapter awaits. `config/scripts/mobile-web-app-session-render.test.mjs` pinned the page's error list to exactly that one rejection; it is now the empty list, which is what makes the browser proof notice the fix. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../mobile-web-app-session-render.test.mjs | 53 +++++++------------ .../use-mobile-session-diff-comments.ts | 11 ++-- 2 files changed, 22 insertions(+), 42 deletions(-) diff --git a/config/scripts/mobile-web-app-session-render.test.mjs b/config/scripts/mobile-web-app-session-render.test.mjs index b4989a5a199..014797eaaf7 100644 --- a/config/scripts/mobile-web-app-session-render.test.mjs +++ b/config/scripts/mobile-web-app-session-render.test.mjs @@ -24,23 +24,21 @@ import { * cannot say any of it, because they mock react-native away — it is Flow source vitest will not * parse. * - * Two defects this file found, both invisible natively and both a console line rather than a crash. - * One is fixed in the commit beside it and one is reported rather than fixed: + * Two defects this file found, both invisible natively and both a console line rather than a crash, + * and both now fixed: * - * - **Fixed.** `use-mobile-session-markdown-actions.ts` registered `BackHandler` with no platform - * guard, and the effect re-registers whenever the dirty-draft list changes. React Native Web - * answers with "BackHandler is not supported on web and should not be used." and an inert - * subscription: two lines on the console at mount, and a hardware-back guard never armed anyway. - * - **Reported.** `use-mobile-session-diff-comments.ts` runs `void loadDiffComments()` in an effect - * with no catch. The loader returns on a *refused* `worktree.show` and nothing catches a - * *rejected* one, so a host that will not answer raises an unhandled rejection on every session - * mount. `.catch` is the fix and it is one line, but the corpus certifies the rejection — - * `matrix-session.diff-notes-worktree.show-1` lists it as an effect of the loaded checkpoint — so - * fixing it is a golden re-record and a review event rather than something this lane lands. + * - `use-mobile-session-markdown-actions.ts` registered `BackHandler` with no platform guard, and + * the effect re-registers whenever the dirty-draft list changes. React Native Web answers with + * "BackHandler is not supported on web and should not be used." and an inert subscription: two + * lines on the console at mount, and a hardware-back guard never armed anyway. + * - `use-mobile-session-diff-comments.ts` ran `void loadDiffComments()` in an effect with no catch. + * The loader returns on a *refused* `worktree.show` and nothing caught a *rejected* one, so a + * host that will not answer raised an unhandled rejection on every session mount. `.catch` at the + * effect is the fix, and it moved a golden: the corpus certified the rejection as an effect of the + * loaded checkpoint, so `matrix-session.diff-notes-worktree.show-1` was re-recorded without it. * - * So the error assertion below is an exact list rather than `toEqual([])` or a filter: that one - * rejection and nothing else. A second error reds it, and so does the rejection going away, which - * is what makes this file the place the fix is noticed when it lands. + * So the error assertion below is an exact empty list rather than a filter: nothing from this + * closure reaches the document, and any error at all reds it. * * **The terminal is not painted here, and this file must not look as though it is.** Putting a * terminal on screen needs the host protocol handshake, a tab snapshot, a terminal inventory and a @@ -198,15 +196,6 @@ async function openRoute(route, awaitText) { /** The session header renders it, so the chrome is on screen before this reads the tree. */ const BACK_LABEL = 'Back to worktrees' -/** - * The one error this page is expected to produce, named in full. - * - * `use-mobile-session-diff-comments.ts`'s uncaught `loadDiffComments()` against a double that - * answers no RPC. The category is the double's own, so this string is stable for this file and - * says which refusal reached the document rather than only that something did. - */ -const KNOWN_UNCAUGHT = 'RenderCheckShellDouble: the render check answers no RPC' - describeRender( 'the session route in a real browser', () => { @@ -219,10 +208,9 @@ describeRender( expect(text).toContain(key) } expect(text).not.toContain(UNMATCHED) - // Exact, because this closure's defects are exactly console lines. The unguarded - // `BackHandler` put two here and is fixed; the uncaught diff-notes rejection is the one - // entry left and is a golden re-record away from going too. - expect(opened.errors).toEqual([KNOWN_UNCAUGHT]) + // Exact, because this closure's defects are exactly console lines: the unguarded + // `BackHandler` put two here and the uncaught diff-notes rejection one, and both are fixed. + expect(opened.errors).toEqual([]) await opened.page.close() }, 120_000) @@ -268,8 +256,8 @@ describeRender( // Chromium reports a refused subresource as a console error naming the directive, so // anything this closure loaded that the policy blocked lands here. expect(opened.errors.filter((entry) => entry.includes('Content Security Policy'))).toEqual([]) - // And nothing else beyond the one rejection above, so this case reads the whole account. - expect(opened.errors).toEqual([KNOWN_UNCAUGHT]) + // And nothing else at all, so this case reads the whole account and not only the policy. + expect(opened.errors).toEqual([]) // Stronger than the line above and independent of it: not one request left the origin, so // there is nothing for the policy to have refused. A font, a beacon or a provider image // added anywhere in this closure reds this. @@ -318,9 +306,4 @@ describeRender( * **The storage refusals.** A page write needs a control to make it. The refusal's own chain is * `mobile/src/session/mobile-structured-send-page-storage-refusal.test.ts` end to end over the * real `page-async-storage`. - * - * **That the one uncaught rejection is harmless.** It is not reported as a page fault — the shell's - * `fault` notify is raised by the React boundary, and `__orcaRenderCheckFaults` is empty here — so - * the generation is not dropped and the screen keeps working. What it costs is a document-level - * error on every mount, which is a line in a crash report and a red herring in the device proof. */ diff --git a/mobile/src/session/use-mobile-session-diff-comments.ts b/mobile/src/session/use-mobile-session-diff-comments.ts index e271c7a5bf3..5c1c770f2f1 100644 --- a/mobile/src/session/use-mobile-session-diff-comments.ts +++ b/mobile/src/session/use-mobile-session-diff-comments.ts @@ -60,13 +60,10 @@ export function useMobileSessionDiffComments(scope: MobileSessionDocumentReaders ) useEffect(() => { - // No catch, deliberately, and it is a recorded defect rather than an oversight: a *refused* - // `worktree.show` returns above, and a *rejected* one is an unhandled rejection on every mount - // — visible in the page as a document-level error, measured by - // `config/scripts/mobile-web-app-session-render.test.mjs`. Adding `.catch` here is the fix and - // it moves a golden: `matrix-session.diff-notes-worktree.show-1` certifies the rejection as an - // effect of the loaded checkpoint, so the change is a re-record and a review event, not a line. - void loadDiffComments() + // Caught here and not in the loader: a *rejected* `worktree.show` would otherwise be an + // unhandled rejection on every mount, and the loader's own promise is awaited by the recording + // adapter, which a swallowed rejection inside it would hide. + void loadDiffComments().catch(() => undefined) }, [loadDiffComments]) const addDiffCommentForFile = useCallback(