Skip to content

fix(web): fail renders on late default serializer errors - #4650

Closed
everton-dgn wants to merge 1 commit into
solidjs:nextfrom
everton-dgn:fix/render-stream-serialization-failure
Closed

everton-dgn wants to merge 1 commit into
solidjs:nextfrom
everton-dgn:fix/render-stream-serialization-failure

Conversation

@everton-dgn

Copy link
Copy Markdown

Summary

Refs #4647 (Solid 2, next). Draft for the render-failure option proposed in the issue.

With the default serializer, an async value can fail after write returns, yet the render reports completion. Track that failure and finish through the existing failure path, without onCompleteAll. Synchronous failures and custom serializers retain their behavior. Buffered stream failures reported inside write remain outside this change.

This discards the document before the shell, including otherwise valid siblings. After the shell, the response ends with the HTML already sent; the client key remains pending. Rejecting that key instead would require a serializer change. The three updated expectations make this tradeoff explicit.

How did you test this change?

  • Six assertions fail on the base; all 39 focused tests pass with the fix.
  • The full web suite, web type tests and Prettier passed.
  • The root build and all 39 global test/typecheck tasks passed with --concurrency=1.

@changeset-bot

changeset-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6707995

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

This PR includes changesets to release 15 packages
Name Type
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
test-integration Patch
board-example Patch
canvas-example Patch
server-dbmon-example Patch
todos-server-example Patch
@solidjs/compiler Patch
@solidjs/signals Patch
solid-js Patch
@solidjs/universal 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

codspeed Bot commented Nov 3, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 7.76%

3 warnings

⚠️ 1 benchmark is dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ 1 benchmark with a significant change ran on different hardware

Its base and head results were measured on machines with a different CPU, core count or memory, so the change might not come from the code. Run base and head on the same runner type.

Learn more about Macro Runners

See affected benchmarks

⚠️ 1 benchmark with a significant change ran in a different software environment

Its base and head results were measured with a different OS or toolchain, such as compiler, runtime or library versions, so the change might come from this difference rather than from the code. Open the benchmark to see what changed.

See affected benchmarks

❌ 1 regressed benchmark
✅ 199 untouched benchmarks
⏩ 5 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ memo + sync render effect only (reference) 30.7 ms 33.2 ms -7.76%

Tip

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


Comparing everton-dgn:fix/render-stream-serialization-failure (6707995) with next (ab29345)

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

ryansolid pushed a commit that referenced this pull request Nov 3, 2026
…reject the key after

Ryan's ruling on #4659 (2026-11-03): "failing ssr on failed serialization
feels reasonable otherwise we could be midstream." everton-dgn's #4650
(cherry-picked in the previous commit) fails the render on a late default
serializer failure. Limit that to failures before the first flush (an
awaited render, or a stream whose shell has not gone out yet, checked
again where the shell is about to be written), and keep the client-side
rejection of the key for failures after it, where the render completes.

Merges the two stream-async-serialize-failure-4647 specs (pre-flush cases
from #4650, midstream cases from #4659, plus a boundary value that fails
while the shell still waits on a blocker). Re-pins the #4242 writer case:
its failure lands before the shell, so the render fails. Updates the
`handling: "serialize"` contract in 12-ssr-http.md, the renderToStream
onError JSDoc, the serializer comment and both changesets.

Co-authored-by: Éverton Toffanetto <evertondgn@hotmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE
ryansolid added a commit that referenced this pull request Nov 3, 2026
…#4647) (#4659)

* fix(web): reject a hydration key whose async value fails to serialize (#4647)

An async value that resolved to something seroval cannot serialize was
reported as `render/serialize` and the render completed, but the payload
declared the key's promise and never settled it, so the client's read
stayed pending forever. Per Ryan's ruling (2026-11-03), the hydration
serializer now rejects that promise with an error that says it is a
serialization failure and names the key. The hydrating read throws into
the nearest Errored, whose fallback replaces the server's content until
reset() or an input change reruns it as a client fetch.

Server-only: the change is in the hydration serializer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE

* fix(web): fail renders on late default serializer errors

* fix(web): split #4647 at the first flush: fail the render before it, reject the key after

Ryan's ruling on #4659 (2026-11-03): "failing ssr on failed serialization
feels reasonable otherwise we could be midstream." everton-dgn's #4650
(cherry-picked in the previous commit) fails the render on a late default
serializer failure. Limit that to failures before the first flush (an
awaited render, or a stream whose shell has not gone out yet, checked
again where the shell is about to be written), and keep the client-side
rejection of the key for failures after it, where the render completes.

Merges the two stream-async-serialize-failure-4647 specs (pre-flush cases
from #4650, midstream cases from #4659, plus a boundary value that fails
while the shell still waits on a blocker). Re-pins the #4242 writer case:
its failure lands before the shell, so the render fails. Updates the
`handling: "serialize"` contract in 12-ssr-http.md, the renderToStream
onError JSDoc, the serializer comment and both changesets.

Co-authored-by: Éverton Toffanetto <evertondgn@hotmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE

* fix(web): reject everything a midstream-failed write sent, not just its key (#4647)

- Move the rejection into an exported `rejectUnserializedKey`, passed only
  by `renderToStream` as the hydration serializer's `late` (so
  `renderToString` does not carry it), and drop the dead `started` flag.
- On a write that fails after the first flush, reject every promise and
  throw into every stream that write already sent (a nested promise, an
  async iterable's later items), not only a top-level thenable key.
- Drop the failed parse's indexes from the shared refs, so a second key on
  the same value parses it again and rejects instead of resolving
  `undefined` from a slot the client never filled.
- Hydration spec replays real recorded stream output
  (errored-unserializable-4647.gen.spec.tsx) instead of a hand-written
  payload.
- Docs: the serialize road's wire (a fixed message naming the key, all
  builds); pins cite the dated maintainer ruling (2026-11-03).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Éverton Toffanetto <evertondgn@hotmail.com>

Copy link
Copy Markdown
Member

Thanks, Éverton. Ryan ruled for this split: an unserializable value fails the render before the first flush and rejects its key on the client after it. Your commit landed with your authorship in #4659 (5ae39fc), so I am closing this draft as folded in.


Generated by Claude Code

@ryansolid ryansolid closed this Nov 3, 2026
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