Skip to content

fix(web): freeze response head before disposing an awaited renderToStream - #3292

Merged
ryansolid merged 1 commit into
nextfrom
fix-async-render-status
Sep 6, 2026
Merged

fix(web): freeze response head before disposing an awaited renderToStream#3292
ryansolid merged 1 commit into
nextfrom
fix-async-render-status

Conversation

@ryansolid

Copy link
Copy Markdown
Member

Bug

await renderToStream(...) (the thenable that replaced renderToStringAsync) resolved after disposing the render owner, while event.response was still uncommitted. Every scope-tied httpStatus / httpHeader cleanup therefore retracted its declaration before the consumer ever saw the HTML: a page calling httpStatus(404, "Not Found") came back as a 200 from createSSRResponse(html, event), and headers declared with httpHeader vanished.

The pipe / pipeTo / readable paths never hit this because createSSRResponse commits the stub on the shell's first write, i.e. before the final dispose() runs (firstFlushed).

Integrations (e.g. @solidjs/vite-plugin's async render mode) currently work around it by committing the stub from onCompleteAll, which happens to run just before the thenable's complete().

Fix

Treat render completion as the awaited path's head-freeze point, mirroring what the shell flush does for the piped forms: the thenable commits the request's response stub via commitResponseStub immediately before dispose(). The request event is captured at renderToStream entry (a silent lookup of the request scope's store, peekRequestEvent), not in then(), because the thenable may legitimately be awaited outside the scope it was started in — await provideRequestEvent(event, () => renderToStream(...)) is the storage module's own documented shape.

  • createSSRResponse / commitEventResponse already pass an already-committed stub through untouched, so consumers need no change.
  • Retraction semantics of httpStatus / httpHeader are untouched: a scope disposed mid-render (an errored, recovered boundary) still retracts, because the head is still open at that point.
  • The failRender resolution path (a real error in a retry pass) deliberately keeps the head open as before: the render died, and the consumer keeps a writable head for whatever error response it builds around the partial HTML.
  • Server-only code path; the client bundles are byte-identical (size scenarios measure the same as on next).

Doc comments updated: the then() contract (type + implementation), createSSRResponse (string results from an awaited render arrive committed), commitResponseStub, and the httpStatus / httpHeader jsdoc (server + client stubs) now state when committed flips on each path.

Tests

packages/web/test/server/http-components.spec.tsx, new describe "awaited renderToStream: response head freezes at completion" (4 of 5 fail on next — 200 instead of 404/410/500):

  • httpStatus/httpHeader declarations survive into createSSRResponse
  • declarations made under a boundary that settles asynchronously survive too
  • a scope disposed mid-render still retracts (recovered-boundary semantics preserved)
  • a synchronously erroring child under Errored keeps parity with the pipe path
  • the pipe path is unchanged: the head freezes at shell flush

Gates

  • pnpm --filter @solidjs/web test (client, server, hydrate configs): all green
  • pnpm --filter @solidjs/web test-types, build: green
  • scripts/size: identical numbers to the current next head (the app: render + one signal / app: CSR scenarios are already over cap on next by 33 B / 30 B since feat(web): support responsive image preloads #3183 — unrelated to this PR)

Changeset: @solidjs/web patch.

@changeset-bot

changeset-bot Bot commented Sep 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: edf891b

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

This PR includes changesets to release 11 packages
Name Type
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
test-integration Patch
@solidjs/compiler Patch
@solidjs/diagnostics 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

@coveralls

coveralls commented Sep 6, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 34017406325

Coverage remained the same at 71.814%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 998
Covered Lines: 764
Line Coverage: 76.55%
Relevant Branches: 783
Covered Branches: 515
Branch Coverage: 65.77%
Branches in Coverage %: Yes
Coverage Strength: 14.95 hits per line

💛 - Coveralls

@codspeed-hq

codspeed-hq Bot commented Sep 6, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 136 untouched benchmarks


Comparing fix-async-render-status (edf891b) with next (a58fcbf)

Open in CodSpeed

ryansolid added a commit to solidjs/solid-vite-plugin that referenced this pull request Sep 6, 2026
rc.7 (solidjs/solid#3292) freezes the response head when an awaited
renderToStream completes, before the render is disposed, so the generated
SSR entry no longer needs to commit the stub from onCompleteAll and authored
entries need no hook. Peer floors move to ^2.0.0-rc.7; README caveat and the
unreleased render-mode changeset drop the workaround wording.

Against rc.6 the render-mode suite fails exactly the six async head
assertions (status/header/Location, dev+prod); the rc.7 ride commit
(catalog + lockfile + minimumReleaseAgeExclude) turns them green.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid
ryansolid force-pushed the fix-async-render-status branch from 75029ea to 701aac7 Compare September 6, 2026 06:43
…ream

`await renderToStream(...)` resolved AFTER disposing the render owner,
while `event.response` was still uncommitted, so every scope-tied
`httpStatus`/`httpHeader` cleanup retracted its declaration before the
consumer ever saw the HTML: a page's `httpStatus(404)` came back 200 from
`createSSRResponse(html, event)` and declared headers vanished. The pipe
paths never hit this because `createSSRResponse` commits the stub on the
shell's first write, before the final dispose.

Treat completion as the awaited path's head-freeze point: the thenable
commits the request's response stub (captured at render start — the
thenable may be awaited outside the request scope it was started in)
right before `dispose()`. `createSSRResponse` / `commitEventResponse`
already pass an already-committed stub through untouched. Retraction
semantics are unchanged — a scope disposed mid-render still retracts —
and the failRender resolution path keeps the head open as before.

Integrations no longer need the `onCompleteAll` commit workaround.
@ryansolid
ryansolid force-pushed the fix-async-render-status branch from 701aac7 to edf891b Compare September 6, 2026 06:47
@ryansolid
ryansolid merged commit ecdfc3a into next Sep 6, 2026
7 checks passed
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