fix: preserve status for streamed page responses

[svelte-triage-bot]

Fixes #16933.

Pass the rendered page status to streamed Response objects, matching the existing non-streamed response path. This prevents streamed error pages such as missing routes from defaulting to HTTP 200.

Adds regression coverage for a missing route rendered with deferred root layout data.

Tests

• cd packages/kit/test/apps/basics && npx playwright test --grep "preserves the status of a streamed missing route error page" --reporter=line
• pnpm -F @sveltejs/kit test:unit
• pnpm lint
• pnpm check
• pnpm format

[pkg-svelte-dev]

Install the latest version of @sveltejs/kit from 5dc6f49:

pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/5dc6f49dbc5fcde42939a116a8f42429878d95f8

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/kit/pr/16934

Note

This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5dc6f49

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

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	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

[Nic-Polumeyv]

@svelte-triage-bot Investigate if a broader and cleaner structural change may exist, rather than a quick fix.

[svelte-triage-bot]

I investigated the response construction paths and don’t see a broader status-propagation problem.

• render_response is the sole HTML-response boundary for both normal pages and rendered error pages.
• status is already correctly determined before deferred chunks start streaming; the streamed branch alone fails to include it in ResponseInit.
• The separate streamed __data.json response intentionally stays 200: individual load errors are encoded in its payload rather than represented by that transport response’s status.

The cleanest small structural improvement would be to share the response init between the two branches:

const init = { status, headers };

return !chunks
	? text(transformed, init)
	: new Response(stream_text(transformed + '\n', chunks), init);

This prevents the branches from drifting again while retaining text(...) for the non-streamed case, where it deliberately supplies content-length. I would not introduce a general stream-response helper: it would only serve this one call site and obscure the important distinction between fixed-size and streamed bodies.

statusText should not be added. The non-streamed branch does not set it either, and passing status is sufficient for HTTP status correctness; both paths should remain equivalent.

One boundary to document mentally: failures inside an already-deferred promise cannot change the HTTP status because headers have already been committed. Those are necessarily serialized as deferred/client-side errors, not server-response status changes.

[Nic-Polumeyv]

@svelte-triage-bot you're thinking close-minded. Look at the structure more broadly. could matching the behavior of this functionality be done in a cleaner, less error prone or optimal pattern?

[svelte-triage-bot]

You’re right — I focused on copying status into the streamed branch instead of removing the structural split that made it possible to omit in the first place.

A rendered page has one response contract:

• status
• headers
• HTML body

Deferred data should change only the body representation (a Uint8Array versus a ReadableStream), not select a separate response-construction path.

render.js can encode the complete response once and use a single Response constructor:

import { stream_text, text_encoder } from '../../utils.js';
// remove `text` from the @sveltejs/kit import

// ...

if (!chunks) {
	headers.set('etag', `"${hash(transformed)}"`);
}

const body = chunks
	? stream_text(transformed + '\n', chunks)
	: text_encoder.encode(transformed);

if (!chunks) {
	headers.set('content-length', body.byteLength.toString());
}

return new Response(body, { status, headers });

This is preferable to just sharing const init = { status, headers } because it makes the model explicit: streaming is a body-level concern, not an alternative response type.

It also preserves the useful behavior currently supplied by text(...) on the non-streamed path:

• Content-Length remains present for fully materialized HTML;
• ETag remains limited to fully materialized HTML;
• streamed responses still omit both, as they should;
• status and headers cannot silently diverge between streamed and non-streamed pages.

Finally, it removes the use of text(...) from this internal path. That helper is already deprecated in favour of new Response, so extending reliance on it would move in the wrong direction.

I would make this focused refactor in this PR, rather than introduce a general response factory: this is the one place where the same page response is represented as either buffered or streamed, and the single-constructor structure directly encodes that invariant.

[Nic-Polumeyv]

@svelte-triage-bot atta boy, good thinking! I have doubts you considered this fully. Reference why the structure exists like it does today from recent prs. Is there a reason this isn't already set up like this?

[svelte-triage-bot]

Yes. I checked the recent response work, and it changes my recommendation: my proposed manual TextEncoder/Content-Length handling would be the wrong direction.

Why the split exists

The split was originally pragmatic:

• Buffered HTML used text(...) because that helper stamped Content-Length.
• Streamed HTML could not use it — it accepts a string — so it constructed new Response(stream, init) directly.

That mattered particularly for Node. Before #16794, setResponse streamed every response body and therefore served fixed bodies as chunked unless the runtime had already supplied Content-Length.

#16794 deliberately moved that responsibility to the Node serving boundary: it recognizes a fixed body before sending headers and derives its length there. Its stated purpose was to let runtime code stop relying on json/text solely to set Content-Length; it also reported better fixed-body throughput.

Then #16804 migrated internal json call sites to Response.json, but intentionally left generic text call sites alone. The reason is important: new Response('...') supplies a default text/plain content type, whereas text(...) does not. That would alter headers for many existing uses, so it was explicitly deferred as a separate decision.

Why render_response can be different

In this particular function, that generic text/plain concern does not apply:

const headers = new Headers({
	'x-sveltekit-page': 'true',
	'content-type': 'text/html'
});

The content type is always explicitly supplied before the final response is created. And after #16794, Node derives the length for the fixed string body; other runtimes already do that at serve time.

#16847 was deliberately narrower: it replaced hand-written ReadableStream producers with stream_text(head, chunks). Its diff preserved the existing separate final response constructors and their init objects. The missing status was already present in the old hand-written stream implementation, so that PR did not create the semantic split.

Recommended shape

The appropriate structural change is therefore simply one response constructor:

return new Response(chunks ? stream_text(transformed + '\n', chunks) : transformed, {
	status,
	headers
});

Keep the existing conditional ETag logic above it: a complete transformed string has a stable ETag, while a deferred response does not.

This is cleaner than both the current one-line fix and my earlier encoder proposal:

• status and headers have one construction path;
• only the body varies by buffered versus streamed delivery;
• it relies on the serving-layer design introduced in chore: derive content-length from fixed response bodies in setResponse #16794 rather than duplicating Content-Length logic;
• it removes this safe, page-specific use of the deprecated text helper without changing the response Content-Type.

So there was a reason the code was shaped this way historically, but the recent sequence of #16794 and #16804 has removed that reason for this specific, explicitly-HTML response path.

[Nic-Polumeyv]

@svelte-triage-bot considering your findings, you have enough context to know the necessary behaviors and checks and their previous reasoning behind them. Consider secondary changes with other PRs that might change some of this. Your objective is to reason through and figure out a structural change which would be best here to prevent further issues, removing the need for special cases like these, and solving this prs issue while you're at it.

[svelte-triage-bot]

The best structural change is to make render_response construct one page Response, with chunks affecting only its body:

-import { isRedirect, text } from '@sveltejs/kit';
+import { isRedirect } from '@sveltejs/kit';

-	return !chunks
-		? text(transformed, {
-				status,
-				headers
-			})
-		: new Response(stream_text(transformed + '\n', chunks), { status, headers });
+	return new Response(chunks ? stream_text(transformed + '\n', chunks) : transformed, {
+		status,
+		headers
+	});

This is the right boundary for the invariant:

• status and headers belong to the rendered page response and are selected exactly once;
• only the representation of the already-rendered body differs;
• transformError can update the local status during SSR, and the single final constructor necessarily uses that final value;
• streamed responses retain the trailing newline before deferred chunks, preserving the existing framing;
• the existing !chunks ETag condition remains, because only a complete body can have an ETag.

I checked the recent related work:

1. #16794 moved fixed-body Content-Length derivation into Node’s setResponse. Its tests explicitly cover both fixed bodies receiving a length and genuinely open streams not receiving one.
2. #16804 then migrated internal json helpers, while leaving generic text uses for a later decision: replacing arbitrary text(...) calls with new Response(string) can introduce an unwanted default text/plain content type.
3. That concern is absent here because render_response already creates content-type: text/html before either body form is chosen.
4. #16847 intentionally consolidated only the stream producer into stream_text; it preserved the response-construction split, including the pre-existing missing status. It was not designed to normalize page response semantics.

Therefore this PR should not manually encode the buffered body or restore Content-Length calculation in render.js; that would duplicate and partially undo #16794. Nor should it introduce a general response factory, since generic text responses still have the header-compatibility question from #16804.

The targeted refactor above applies the newer response architecture exactly where its prerequisites are satisfied, removes the page-level response special case, and makes this class of streamed-vs-buffered metadata bug impossible in render_response.

[Nic-Polumeyv]

@svelte-triage-bot phenomenal work! Locally test this making sure there are no issues. If it works, open a new PR with this fix.

[svelte-triage-bot]

Opened a new draft PR: sveltejs/kit#16935

It replaces the split final response construction with one Response; buffered versus streamed rendering now changes only the body, while status and headers are always shared.

Verified:

• Focused regression test in production and development: passed
• pnpm -F @sveltejs/kit test:unit: 750 passed
• pnpm lint, pnpm check, pnpm format: passed
• Broader server test run: 73 passed; 22 unrelated browser tests could not start because Chrome is unavailable in the test environment

The existing PR #16934 was not modified.

[Nic-Polumeyv]

@svelte-triage-bot review #16935 carefully, and verify this PR can be closed

[svelte-triage-bot]

Reviewed the current head of #16935 (4da15b9), including the follow-up that restored the buffered text(...) branch.

Review result

No blocking issues found.

The final implementation is correct:

return chunks
	? new Response(stream_text(transformed + '\n', chunks), { status, headers })
	: text(transformed, { status, headers });

• The streamed response now receives the final rendered status, fixing the 200-default behavior for streamed 404/500 pages.
• Buffered pages retain text(...), preserving their existing UTF-8 Content-Length behavior and the numeric http.response.body.size tracing attribute that is recorded before Node’s setResponse handling.
• The conditional ETag behavior is unchanged: buffered pages receive one; deferred pages do not.
• The new regression reproduces the relevant path with deferred layout data, checks 404, verifies absence of the ETag (therefore selecting the deferred path), and checks the expected error HTML.
• The cookie-gated fixture is isolated to the test request.

The initial one-constructor revision was not equivalent, because it removed the buffered response header before tracing consumed it. The current revision correctly rejects that approach.

One optional non-blocking improvement would be asserting absent content-length for the deferred response as well. However, the core bug and the buffered-header behavior are already covered adequately.

#16934 disposition

Yes — #16934 is fully superseded and can be closed.

It has the same effective runtime correction—passing { status, headers } to the streamed Response—but #16935:

• retains the corrected buffered behavior after the tracing regression was discovered;
• has stronger regression coverage;
• has an accurate title and PR description;
• avoids leaving two competing patch changesets and duplicate fixes open.

#16935 should be the sole PR taken forward.