Repository navigation
fix(signals): a memo over an error boundary's output no longer halts on a held rejection (#4640) - #4641
Conversation
…4640) The settle walk from a held rejection marked every derivation it reached with the staged error, including a memo over an error boundary's output (reached through the output, which it does not mark). The commit then published the error on that memo, past the boundary that caught it, and halted. stageMark now marks a node only when one of its deps carries the staging (D-41: a derivation that reads a staged error throws it); a memo over the output is a plain reader of it (F-3). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE
🦋 Changeset detectedLatest commit: fef273d The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size (brotli, eager entry chunk)
❌ Fails the gate
Bundled with Rolldown (what Vite ships), brotli q11, decimal KB. A scenario fails only when it is over its brotli cap and its minified size is more than 20 B over the minified recorded with the cap; over the cap within that allowance is brotli layout noise and passes with a warning. Caps and their recorded minified in |
Coverage Report for CI Build 41570402033Coverage remained the same at 78.205%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Merging this PR will regress 1 benchmark3 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
|
The CodSpeed flags here look like noise, not this change. The same two benches, Generated by Claude Code |
|
The red Size check is this fix's growth, which the maintainer accepted: +70 B minified and at most +25 B brotli, only on the isPending/latest rows. Hello world is unchanged. The caps get re-recorded in the next batch ledger PR, so this PR leaves scripts/size alone. Generated by Claude Code |
review) Dropping CONFIG_ASYNC from the exclusion mask keeps a plain memo over an async memo over a staged source marked: its verdict stays pending and latest() throws until the commit (F-9, D-22, D-23, D-41; #4456). The dep scan returns on the first staged dep. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE
…a rejection (#4640 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE
Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE Co-authored-by: Claude <noreply@anthropic.com>
What was wrong
An error boundary built outside (
untrack) over an async memodetail, with a memot = createMemo(() => shown())over the boundary's output read by a render effect. After the first answer,setSel(1)starts a held change and the second ask rejects. The rejection is staged ondetail(D-41). The settle walk fromdetail(settlePendingSource) visits every transitive dependent and callsstageMarkon each one it settles:children→ boundary tree (skipped: frame) → boundary output (skipped:CONFIG_REDERIVE) →t.tis a plain memo, so it was markedCONFIG_STAGED_ERRORwithdetail's error, even though it reads only the output, which the boundary had resolved to its fallback. At the commit,commitPendingNode(t)published the error ontand from there to the render effect. No boundary sits above the effect, so the scheduler halted (REACTIVITY_HALTED, uncaughtboom) and the screen stayed onA. A render effect reading the output directly is skipped bystageMark(effects are excluded), so that shape showedERR.Under a
<Loading>(withouton), the bug happens whether or not there is a memo between. The Loading'schildrencomputed is itself a plain memo over the error boundary's output, so it was marked the same way.The rules that decide it
tmust behave as its render effect reading the output directly would. That shape showsERR.<Errored>sends that boundary to its error fallback, which is then the display … An error that escapes every boundary halts." The error came from content under the boundary and the boundary caught it, so nothing escaped and nothing may halt.<Errored>flipping to its fallback is a boundary going back to its fallback." The fallback is the display.tdoes not read a staged error. It reads the output, and the output's pass decides between content and fallback. Markingtbroke this clause. The fix applies it literally: a node is marked only if it reads a staging.<Loading>here has shown content and has noon, so it is disarmed and forwards (F-5). An error is not a pending, so the Loading does not catch it (F-12). The error boundary's fallback is the content the Loading shows. D-14 and D-15 concernonre-arms and fallback-caught flights. Neither applies here, and the change does not touch either path (boundaries.tsis unchanged).Checking each clause against the fix
detailandchildrenare still staged and commit with the frame. The output re-derives at that commit and showsERRin the same frame as everything else (pinned:["A","ERR"], with no intermediate frame).[A](or[fb, A]) through the held change.<Errored>when a signal it reads is written just before its promise rejects (regression from fb6006eb, #4156) #4189). All their pins pass.The change
packages/signals/src/core/verdict.tsstageMark: the existing exclusions still apply, and now a node is also skipped unless one of its_depscarriesCONFIG_STAGED_ERROR. An async memo is no longer excluded (review of 0ad8249). Without that, a plain memo over an async memo over a staged source lost its pending verdict:latest()returned the old value between the rejection and the commit, and then the value changed at the commit (F-9, D-22, D-23, D-41; the #4456 shape). The walk still passes through the nodes it does not mark (a boundary output), so it still retires pending sources downstream. The change is only in the verdict engine's optional path, and hello world is unchanged. CI measured +70 B minified on thelatest/isPendingrows at 0ad8249. The maintainer accepted that, and the review's version is smaller.(Prettier also reflowed one existing over-long condition in
keepReadersin the same file. That is whitespace only.)Tests
packages/signals/tests/errored-memo-over-output-held-reject-4640.test.ts. Four rows: render effect reads the output {directly, through a memo} × {in the open, under a<Loading>withouton}. Each row pins the full frame list: open["A","ERR"], loading["fb","A","ERR"]. With the fix reverted, 3 of the 4 rows fail with the uncaughtboom(memo/open, direct/loading, memo/loading). Direct/open passes either way.<Errored>when a signal it reads is written just before its promise rejects (regression from fb6006eb, #4156) #4189, projections, optimistic, keyed, lane): 323 files, 4965 passed, 14 expected-fail, 0 failures.reject-memo.mjs), run against the rebuilt dist: no uncaught in any of the 8 modes. The memo and direct shapes match.Fixes #4640.
Generated by Claude Code