fix(web): createSSRResponse settles when the render ends before the shell - #3725
Merged
Merged
Conversation
…hell `createSSRResponse` resolved only from its sink's first `write`, and its `end()` returned early while no stream controller existed. A render that failed before the shell (the `pipe()` failure completion ends the sink with nothing written) left the promise pending forever, and an abort through `signal` never reached the sink at all. A successful empty render reached the same early return. `pipe()` now hands a pre-shell failure or abort to a module-private method only `createSSRResponse`'s sink carries; it commits the stub and resolves a bodyless 500, or the redirect when a `Location` is already on the stub. A bare `end()` with nothing written runs the first-write path, so an empty document resolves an empty 200. Every other `pipe()` sink sees exactly what it did before. The promise still never rejects. Fixes #3719 Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
🦋 Changeset detectedLatest commit: 2656284 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
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 |
Size (brotli, eager entry chunk)
Bundled with Rolldown (what Vite ships), brotli q11, decimal KB. Caps in |
Coverage Report for CI Build 36700026376Coverage remained the same at 75.979%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Merging this PR will not alter performance
Comparing 
Footnotes
|
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
2 participants
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.
Fixes #3719
Summary
createSSRResponse(renderToStream(...), event)never settled when the render ended before its shell flushed, so hosts that answer every page through it (including@solidjs/vite-pluginin its default stream mode) hung the request. It now always settles:onErrorhearshandling: "failed") resolves with a bodyless 500. If aLocationis already on the response stub (set by middleware, for example), it resolves with the redirect instead, the same pre-flush rule as the shell path. The stub is committed.signalbefore the shell resolves the same way: a bodyless 500 (or the redirect), stub committed.onErrorhears nothing, because a disconnect is not a render failure.() => null,() => "") also hung: its shell reaches the sink as no write at all. It now resolves with an empty 200, the same answer ascreateSSRResponse("").The promise still never rejects.
Cause
createSSRResponseresolved only from its sink's firstwrite. Itsend()began withif (closed || !controller) return;(packages/web/src/server.tsL6815 atcce43eb44), andcontrollerexists only after that first write. Since #3569, a pre-shell failure ends the sink with nothing written, soend()returned and the promise was never settled. An abort throughsignalnever reaches the sink at all:abandon("signal")deliberately leaves it alone.Fix
A bare
end()cannot tell a failed render from a successful empty one; both arrive with nothing written. So the issue's suggested "no write means 500" would have answered empty successes with a 500. Instead:pipe()checks the sink for a module-private method (a localSymbol(), not exported) that onlycreateSSRResponse's own sink carries. A pre-shell failure calls it instead ofend(), and so does a pre-shell abort, including a signal already aborted beforepipe()is called. It commits the stub and resolves the 500 or the redirect, and it is guarded against settling twice.end()with nothing written now runs the normal first-write path, which is what gives the empty 200 (transformChunkstill sees the empty first chunk, as the string path does).pipe()sink sees exactly what it did before:end()on failure, silence on abort.No timers or listeners are added. The abort listener is still removed by the render's dispose, and
onErrorstill fires exactly once per failure.Semantics and evidence
Resolve with a 500 on a pre-shell failure; never reject:
createSSRResponse's JSDoc and the SSR/HTTP guide describe the stream promise only as resolving; the function has no rejection path anywhere.pipe()ends the sink,awaitresolves"",pipeTo()/readableclose. The thenable's contract says it "never rejects" becauseonErroralready heard the failure, and leaves the consumer's head writable "for whatever error response it builds". For stream results,createSSRResponseis that consumer.handling: "failed"has no value to send to the client, and it is reported exactly once. A rejection would carry the error into host code for a second report.Responsefailures at the handler edge into a bodyless 500. Resolving that way here gives the same answer without a second report, and cannot add 'draggable' attr to img #383 notes it cannot catch this hang because nothing is thrown.Abort before the shell resolves the same bodyless 500 (maintainer ruling). This matches
createSSRResponse's resolve-only contract andpipeTo's own consumer-disconnect path, which resolves rather than rejects. The alternative considered was rejecting withsignal.reason, as the server-function runtime's in-process calls do. It was not chosen: it would add the function's first rejection path, and host containment such as #383 would report it as a request failure anyway.Public API Changes
No exports, types, options or signatures change. Documented behavior changes (JSDoc and
documentation/solid-2.0/12-ssr-http.mdupdated):createSSRResponsewith a stream result: a render that fails, or is aborted through itssignal, before the shell flushes now resolves with a bodyless 500 (or the redirect when aLocationis already on the stub) and commits the stub. Before, the promise stayed pending forever.createSSRResponsewith a stream result that succeeds with an empty document now resolves with an empty 200 (defaultcontent-type). Before, it stayed pending forever.Tests
The new tests sit beside the #3569 (b) tests in
packages/web/test/server/ssr-async-rejection-3569.spec.tsxand reuse theirfailingPreShell()fixture, lifted to module scope. Each one races a deadline, so a hang fails with a name.next<Loading>(the #3569 (b) fixture): 500, empty body, stub committed, onefailedfailedfailedLocation+ cookie set before the render: 302,Locationand cookie keptonErrorsilent, abort listener removedcontent-typeA sync throw on the first render pass is unchanged: it still throws synchronously out of
renderToStream, beforecreateSSRResponseis called.Suites run locally:
@solidjs/webserver@solidjs/webclient@solidjs/webhydratesolid-js@solidjs/web,solid-js)Size
0 B in every size scenario, including the server-entry floors from #3722: none of them imports
renderToStreamorcreateSSRResponse. The unminifieddist/server.jsgrows by 811 B raw, 182 B brotli (the dist keeps comments).Related
kind: "request"failure report) is not implemented here. This fix needs nothing from it: the render still reports throughonErrorexactly as before, andcreateSSRResponsereports nothing itself.Known gap (follow-up)
An abort through
signalafter the shell still leaves the resolvedResponse's body open.abandon("signal")never touches the sink, so the stream is never closed or errored. Withsignal: request.signalthe host usually cancels the body itself. Whensignalis a render deadline (AbortSignal.timeout(...)), the client keeps a partial body open. That case is out of scope here and deserves its own issue.