Skip to content

Commit 08b88fb

Browse files
brenelzclaude
andauthored
fix(signals): don't route an already-caught error past its Errored through a Loading (#2856)
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>
1 parent ef4d53e commit 08b88fb

3 files changed

Lines changed: 112 additions & 6 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/signals": patch
3+
---
4+
5+
Fixed an error-routing escape in `Errored > Loading > Errored > content` composition: a sync error thrown by 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 (e.g. navigating to a route that renders already-errored) and when it threw reactively after a healthy commit. The inner `Errored` consumed the ERROR dimension from the notification mask when it caught, 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 only fires while the ERROR dimension is still live in the mask. Errors surfaced through DOM insertion effects were unaffected. Restores the beta.15 (and Solid 1.x) contract that `Loading > Errored > content` reliably catches at the inner boundary.

packages/solid-signals/src/boundaries.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -304,12 +304,18 @@ export class CollectionQueue extends Queue {
304304
// Notify-through: a real error inside a `Loading` is not this boundary's to
305305
// handle — remap it to ERROR and forward toward the `Errored` that catches it.
306306
// This must run before the initialized passthrough below so the error still
307-
// reaches its catcher after this boundary has committed. The mirror case
308-
// (pending inside an `Errored`) needs no explicit rule: the PENDING dimension
309-
// survives `type &= ~collectionType` below and reaches the outer `Loading`
310-
// natively, while a pending already caught by an inner `Loading` arrives here
311-
// as a bare ERROR-dimension remainder and is correctly swallowed.
312-
if (this._collectionType & STATUS_PENDING && flags & STATUS_ERROR) {
307+
// reaches its catcher after this boundary has committed. Guarded on the ERROR
308+
// dimension still being live in `type`: an `Errored` nested below this
309+
// `Loading` consumes ERROR from the mask when it catches, and remapping off
310+
// the node's raw `flags` alone would resurrect that consumed error and route
311+
// it past the boundary that already handled it (the Loading > Errored >
312+
// content composition escape).
313+
// The mirror case (pending inside an `Errored`) needs no explicit rule: the
314+
// PENDING dimension survives `type &= ~collectionType` below and reaches the
315+
// outer `Loading` natively, while a pending already caught by an inner
316+
// `Loading` arrives here as a bare ERROR-dimension remainder and is correctly
317+
// swallowed.
318+
if (this._collectionType & STATUS_PENDING && type & STATUS_ERROR && flags & STATUS_ERROR) {
313319
return super.notify(node, STATUS_ERROR, flags, error);
314320
}
315321

packages/solid-signals/tests/createErrorBoundary.test.ts

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,101 @@ it("does not loop when an async memo is read inside Loading > Errored (#2809)",
828828
expect(unwrap(bad)).toBe("caught: posts failed");
829829
});
830830

831+
/**
832+
* #2809 fallout — the `Errored > Loading > Errored > content` composition
833+
* escape: a sync error from the content must be caught by the INNER `Errored`,
834+
* the boundary between the `Loading` and the throwing content. When the inner
835+
* boundary catches, it consumes the ERROR dimension from the notification mask
836+
* and forwards only the PENDING remainder up the queue chain — but the
837+
* `Loading` queue's notify-through remap keyed off the node's raw status flags
838+
* instead of the mask, resurrecting the already-caught error and routing it to
839+
* the OUTER boundary, whose fallback then replaced the whole subtree (and with
840+
* no outer boundary, reactivity halted). This broke the React-style
841+
* `Suspense > ErrorBoundary > content` nesting that TanStack Router mirrors.
842+
* The remap now fires only while the ERROR dimension is still live in the mask.
843+
*/
844+
function mountNestedComposition(content: () => string) {
845+
let rendered: unknown;
846+
const dispose = createRoot(dispose => {
847+
const outer = createErrorBoundary(
848+
() =>
849+
createLoadingBoundary(
850+
() => createErrorBoundary(content, () => "inner caught")(),
851+
() => "loading"
852+
)(),
853+
() => "outer caught"
854+
);
855+
createRenderEffect(
856+
() => (rendered = outer()),
857+
() => {}
858+
);
859+
return dispose;
860+
});
861+
return { rendered: () => rendered, dispose };
862+
}
863+
864+
it("catches at the inner Errored when content throws in the mounting flush (Errored > Loading > Errored)", () => {
865+
const { rendered, dispose } = mountNestedComposition(() => {
866+
throw new Error("boom on mount");
867+
});
868+
flush();
869+
expect(rendered()).toBe("inner caught");
870+
dispose();
871+
});
872+
873+
it("catches at the inner Errored when content throws reactively after a healthy commit (Errored > Loading > Errored)", () => {
874+
const [$boom, setBoom] = createSignal(false);
875+
const { rendered, dispose } = mountNestedComposition(() => {
876+
if ($boom()) throw new Error("boom");
877+
return "ok";
878+
});
879+
flush();
880+
expect(rendered()).toBe("ok");
881+
882+
setBoom(true);
883+
flush();
884+
expect(rendered()).toBe("inner caught");
885+
dispose();
886+
});
887+
888+
it("catches an async rejection at the inner Errored (Errored > Loading > Errored)", async () => {
889+
const rejected = Promise.reject(new Error("boom async"));
890+
rejected.catch(() => {});
891+
892+
let rendered: unknown;
893+
const dispose = createRoot(dispose => {
894+
// The memo lives outside the boundary computes (component-body shape); a
895+
// memo created inside the inner boundary's fn would be recreated on every
896+
// re-run and refetch forever.
897+
const data = createMemo(() => rejected);
898+
const outer = createErrorBoundary(
899+
() =>
900+
createLoadingBoundary(
901+
() =>
902+
createErrorBoundary(
903+
() => `value: ${data()}`,
904+
() => "inner caught"
905+
)(),
906+
() => "loading"
907+
)(),
908+
() => "outer caught"
909+
);
910+
createRenderEffect(
911+
() => (rendered = outer()),
912+
() => {}
913+
);
914+
return dispose;
915+
});
916+
flush();
917+
expect(rendered).toBe("loading");
918+
919+
await Promise.resolve();
920+
await Promise.resolve();
921+
flush();
922+
expect(rendered).toBe("inner caught");
923+
dispose();
924+
});
925+
831926
/**
832927
* #2809 hydration interaction: with snapshot capture active (as during
833928
* `hydrate()`), boundary computeds must not become snapshot sources. The tree no

0 commit comments

Comments
 (0)