Skip to content

fix(signals): don't route an already-caught error past its Errored through a Loading - #2856

Merged
ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/loading-errored-composition-escape
Jul 9, 2026
Merged

fix(signals): don't route an already-caught error past its Errored through a Loading#2856
ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/loading-errored-composition-escape

Conversation

@brenelz

@brenelz brenelz commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

In the Errored > Loading > Errored > content composition, a sync error thrown by the content was routed past the inner Errored to the boundary above the Loading — the outer fallback replaced the whole subtree, and with no outer boundary the error escaped entirely and (new in beta.16) permanently halted reactivity. This broke the React-style Suspense > ErrorBoundary > content nesting that TanStack Router mirrors; it worked on 2.0.0-beta.15 and regressed in beta.16 with the #2809 boundary notify-through work.

It reproduced both when the content threw during the same flush the boundaries mounted (e.g. navigating to a route that renders already-errored) and when it threw reactively after a healthy commit.

Root cause

When the inner Errored catches, CollectionQueue#notify consumes the ERROR dimension from the notification mask (type) and forwards only the PENDING remainder up the queue chain. But the Loading queue's notify-through remap keyed off the node's raw status flags instead of the mask — it saw "this node has an error", remapped the PENDING-only remainder back into an ERROR notification, and forwarded the already-caught error past its handler.

Fix

The remap now additionally requires the ERROR dimension to still be live in the mask (type & STATUS_ERROR), i.e. it only fires while the error is still unhandled — which is exactly the Errored > Loading > erroring content case the rule was written for (#2821).

Testing

  • Three new regression tests in createErrorBoundary.test.ts pinning the composition: throw in the mounting flush, reactive throw after a healthy commit, and an async rejection — all must catch at the inner boundary.
  • Full solid-signals, solid, and solid-web (client/SSR/hydration) suites pass, including the 2.0.0-beta.15 | Error thrown by async read in <Loading> is not captured by a wrapping <Errored> #2821 errored-async-nesting pins that motivated the notify-through rule.
  • Verified against the published packages that @solidjs/signals 2.0.0-beta.15 catches at the inner boundary and 2.0.0-beta.16 routes to the outer boundary in the same scenarios.

🤖 Generated with Claude Code

…rough a Loading

In Errored > Loading > Errored > content, a sync error from the content
was routed past the inner Errored to the boundary above the Loading —
both when the content threw during the same flush the boundaries mounted
and when it threw reactively after a healthy commit. With no outer
boundary, the error escaped entirely and halted reactivity. This broke
the React-style Suspense > ErrorBoundary > content nesting that TanStack
Router mirrors.

The inner Errored consumed the ERROR dimension from the notification
mask when it caught and forwarded only the PENDING remainder up the
queue chain, but the Loading queue's notify-through remap keyed off the
node's raw status flags and resurrected the already-caught error past
its handler. The remap now fires only while the ERROR dimension is still
live in the mask.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b3c1c16

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@solidjs/signals Patch
test-integration Patch
solid-js Patch
babel-preset-solid Patch
@solidjs/web Patch
@solidjs/html Patch
@solidjs/h Patch
@solidjs/universal Patch
@solidjs/element Patch

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

@codspeed-hq

codspeed-hq Bot commented Jul 9, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 55.9%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 119 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
merge 73.6 µs 166.8 µs -55.9%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing brenelz:fix/loading-errored-composition-escape (b3c1c16) with next (ef4d53e)

Open in CodSpeed

@ryansolid
ryansolid merged commit 08b88fb into solidjs:next Jul 9, 2026
2 of 3 checks passed
ryansolid added a commit that referenced this pull request Jul 9, 2026
After #2856 narrowed the remap to fire only while the ERROR dimension
was still live in the notification mask, the rule collapsed into the
generic consume-and-forward path: each boundary consumes only its own
status dimension from the mask and forwards the remainder, so an error
inside a Loading reaches its Errored natively. The only residual
behavior — suppressing pending collection on a node that is
simultaneously pending and errored — is unreachable: status propagation
(notifyStatus) assigns a single dimension per node, and the effect-phase
error path notifies with literal ERROR-only flags.

Pins the two paths the remap used to intercept (sync error in the
mounting flush and reactive error after a healthy commit, both
Errored > Loading > content). Full signals, solid, and solid-web
(client/SSR/hydration) suites pass.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants