fix(server): propagate effect errors to error boundaries (closes #2777)#2778
Open
tsushanth wants to merge 1 commit into
Open
fix(server): propagate effect errors to error boundaries (closes #2777)#2778tsushanth wants to merge 1 commit into
tsushanth wants to merge 1 commit into
Conversation
…djs#2777) `serverEffect` was swallowing every exception inside the compute/effect path with a bare `catch (err) {}`. That made SSR output 'children' HTML even after the render-effect threw, and diverged from the client/hydration path where the same error would reach the surrounding boundary. Reporter on solidjs#2777 also called out that the error vanishes from any diagnostic/log path. Mirror how `serverSignal`'s pull path handles the same situation (L630-634): NotReadyError is suspense control flow and must keep propagating, real errors get recorded on the computation and re-thrown so `createErrorBoundary` / `<Errored>` can catch them. Add a regression case under the existing 'Server createErrorBoundary' suite that throws from `createEffect` and asserts the boundary catches it.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2777.
serverEffectinpackages/solid/src/server/signals.tswas wrapping the compute / effect path intry { … } catch (err) { /* Swallow errors from effects on server */ }. That left SSR producing successful child HTML even when a render-effect threw — and silently diverged from the client/hydration path where the same kind of error would reach the surrounding<Errored>/createErrorBoundary. As reporter notes, the error also disappears from every diagnostic path, so it can't be logged either.Mirror how
serverSignal's pull path already handles the same situation (L630-634):NotReadyErroris suspense control flow and must keep propagating, real errors get recorded on the computation and re-thrown so a wrapping boundary can catch them.Repro / test commands
Added a regression case to the existing
Server createErrorBoundarydescribe block inpackages/solid/test/server/signals.spec.ts:Result: 48/48 pass, including the new regression. The reporter's StackBlitz now produces
error: server effect boominstead ofchildrenwhen wrapped in<Errored>.Notes
Written by an agent (Claude Code, claude-opus-4-7).