From d0a21c4d6d402063ccc3a68d22a3ebf2cb5583ac Mon Sep 17 00:00:00 2001 From: Nicholas Paun Date: Mon, 19 Oct 2026 21:13:09 -0700 Subject: [PATCH] Follow the Fetch spec for statusText The Fetch spec gives a response an empty status message unless something sets it: ResponseInit.statusText defaults to "", and "initialize a response" copies it as-is. Only the scheme fetch steps set one, to `OK` for data: and about:blank URLs and to `OK` or `Partial Content` for blob: URLs. workerd instead derived the RFC 9110 reason phrase from the status code, so `new Response(null, { status: 404 }).statusText` was "Not Found" rather than "", and likewise for Response.json() and Response.redirect(). An unset statusText is now "". A Response initialized from another Response, through the constructor or Response.json(), copies its status message exactly. Responses returned by fetch() keep the reason phrase the server sent, even when it matches the status code's default. data: URL fetches now set "OK" explicitly, which is the value they already reported. workerd does not fetch blob: or about: URLs. The HTTP reason phrase sent for a Response returned to a client is unchanged: an unset status message still sends the status code's conventional phrase, so HTTP clients see no difference even though the JS-visible statusText is empty. Error messages that include a response's status, such as those thrown by Fetcher's put() and get() helpers, use the same conventional phrase. Compatibility: this changes behaviour for all Workers, on every compatibility date, with no compatibility flag. For example, `new Response(null, { status: 404 }).statusText` becomes "" where it was "Not Found", and Response.json() and Response.redirect() results report "" where they reported "OK" or "Found". This follows James Snell's review, which questioned gating the change. The statusText expected failures in response-init-001, response-static-redirect, response-static-json and response-clone ("Check Response's clone with default values, without body") are removed from both fetch WPT configs. The js-rpc-test and Python SDK test expectations that relied on the derived phrase are updated. Fixes #4973 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/workerd/api/http.c++ | 34 ++++++++--------- src/workerd/api/http.h | 7 +++- src/workerd/api/tests/BUILD.bazel | 8 ++++ src/workerd/api/tests/js-rpc-test.js | 2 +- .../api/tests/response-status-text-target.js | 13 +++++++ .../api/tests/response-status-text-test.js | 38 +++++++++++++++++++ .../tests/response-status-text-test.wd-test | 25 ++++++++++++ src/workerd/server/tests/python/sdk/worker.py | 3 +- src/wpt/fetch/api-test-ts.ts | 29 ++------------ src/wpt/fetch/api-test.ts | 29 ++------------ 10 files changed, 114 insertions(+), 74 deletions(-) create mode 100644 src/workerd/api/tests/response-status-text-target.js create mode 100644 src/workerd/api/tests/response-status-text-test.js create mode 100644 src/workerd/api/tests/response-status-text-test.wd-test diff --git a/src/workerd/api/http.c++ b/src/workerd/api/http.c++ index 9e3aa58cd06..0cbc863d36a 100644 --- a/src/workerd/api/http.c++ +++ b/src/workerd/api/http.c++ @@ -1078,10 +1078,7 @@ jsg::Ref Response::constructor(jsg::Lock& js, statusCode = otherResponse->statusCode; bodyEncoding = otherResponse->bodyEncoding; - kj::StringPtr otherStatusText = otherResponse->getStatusText(); - if (otherStatusText != defaultStatusText(statusCode)) { - statusText = kj::str(otherStatusText); - } + statusText = otherResponse->statusText.clone(); headers = js.alloc(js, *otherResponse->headers); cf = otherResponse->cf.deepClone(js); KJ_IF_SOME(otherWs, otherResponse->webSocket) { @@ -1208,12 +1205,9 @@ jsg::Ref Response::json_( } } KJ_CASE_ONEOF(res, jsg::Ref) { - auto otherStatusText = res->getStatusText(); auto newInit = InitializerDict{ .status = res->statusCode, - .statusText = otherStatusText == nullptr || - otherStatusText == defaultStatusText(res->statusCode) - ? jsg::Optional() : kj::str(otherStatusText), + .statusText = res->statusText.clone(), .headers = maybeSetContentType(js, Headers::constructor(js, res->headers.addRef())), .cf = res->cf.getRef(js), .encodeBody = @@ -1345,14 +1339,14 @@ kj::Promise> Response::send(jsg::Lock& js, } auto maybeLength = jsBody.tryGetLength(js, encoding); auto stream = newSystemStream( - outer.send(statusCode, getStatusText(), outHeaders, maybeLength), encoding, context, + outer.send(statusCode, getReasonPhrase(), outHeaders, maybeLength), encoding, context, flushAfterWrite); // We need to enter the AsyncContextFrame that was captured when the // Response was created before starting the loop. jsg::AsyncContextFrame::Scope scope(js, asyncContext); return jsBody.pumpTo(js, kj::mv(stream), EndStream::YES); } else { - outer.send(statusCode, getStatusText(), outHeaders, static_cast(0)); + outer.send(statusCode, getReasonPhrase(), outHeaders, static_cast(0)); return addNoopDeferredProxy(kj::READY_NOW); } } @@ -1361,6 +1355,13 @@ int Response::getStatus() { return statusCode; } kj::StringPtr Response::getStatusText() { + KJ_IF_SOME(text, statusText) { + return text; + } + // The Fetch spec leaves the status message empty unless something sets it. + return ""_kj; +} +kj::StringPtr Response::getReasonPhrase() { KJ_IF_SOME(text, statusText) { return text; } @@ -2262,10 +2263,8 @@ jsg::Ref makeHttpResponse(jsg::Lock& js, } // TODO(someday): Fill response CF blob from somewhere? - kj::Maybe maybeStatusText = statusText == defaultStatusText(statusCode) - ? kj::Maybe() - : kj::str(statusText); - return js.alloc(js, statusCode, kj::mv(maybeStatusText), kj::mv(responseHeaders), + // A network response's status message is the reason phrase the server sent. + return js.alloc(js, statusCode, kj::str(statusText), kj::mv(responseHeaders), nullptr, kj::mv(responseBody), kj::mv(urlList), kj::mv(webSocket), bodyEncoding); } @@ -2335,6 +2334,7 @@ jsg::Promise> fetchImplNoOutputLock(jsg::Lock& js, return js.resolvedPromise(Response::constructor(js, kj::mv(maybeResponseBody), Response::InitializerDict{ .status = 200, + .statusText = kj::str("OK"), .headers = kj::mv(headers), })); } @@ -2680,7 +2680,7 @@ static jsg::Promise throwOnError( // should people really use fetch() if they want to inspect error responses? JSG_REQUIRE(status >= 200 && status < 300, Error, kj::str("HTTP ", method, " request failed: ", response->getStatus(), " ", - response->getStatusText())); + response->getReasonPhrase())); }); } @@ -2735,8 +2735,8 @@ jsg::Promise Fetcher::get( // TODO(someday): Would be nice to attach the response to the JavaScript error, maybe? Or // should people really use fetch() if they want to inspect error responses? JSG_FAIL_REQUIRE(Error, - kj::str( - "HTTP GET request failed: ", response->getStatus(), " ", response->getStatusText())); + kj::str("HTTP GET request failed: ", response->getStatus(), " ", + response->getReasonPhrase())); } else { return parseResponse(js, kj::mv(response), kj::mv(type)); } diff --git a/src/workerd/api/http.h b/src/workerd/api/http.h index d3d0f996186..a6fd5eb32dc 100644 --- a/src/workerd/api/http.h +++ b/src/workerd/api/http.h @@ -1128,6 +1128,9 @@ class Response final: public Body { int getStatus(); kj::StringPtr getStatusText(); + // The reason phrase for the HTTP status line and for error messages. Unlike getStatusText(), an + // unset status message falls back to the status code's conventional phrase. + kj::StringPtr getReasonPhrase(); jsg::Ref getHeaders(jsg::Lock& js); bool getOk(); @@ -1217,8 +1220,8 @@ class Response final: public Body { private: int statusCode; - // If the statusText is empty, we will derive it from the statusCode. If there's no - // match, it will be empty. + // The status message, if one was set. When unset, getStatusText() returns "", as the Fetch spec + // requires, while getReasonPhrase() derives the status code's conventional phrase. kj::Maybe statusText; jsg::Ref headers; CfProperty cf; diff --git a/src/workerd/api/tests/BUILD.bazel b/src/workerd/api/tests/BUILD.bazel index 0c24145008d..e920a7fb2b8 100644 --- a/src/workerd/api/tests/BUILD.bazel +++ b/src/workerd/api/tests/BUILD.bazel @@ -996,6 +996,14 @@ wd_test( data = ["response-error-test.js"], ) +wd_test( + src = "response-status-text-test.wd-test", + data = [ + "response-status-text-target.js", + "response-status-text-test.js", + ], +) + wd_test( size = "large", src = "scheduler-test.wd-test", diff --git a/src/workerd/api/tests/js-rpc-test.js b/src/workerd/api/tests/js-rpc-test.js index 588fabb533f..89c3a398726 100644 --- a/src/workerd/api/tests/js-rpc-test.js +++ b/src/workerd/api/tests/js-rpc-test.js @@ -1930,7 +1930,7 @@ export let serializeHttpTypes = { let req = await env.MyService.returnResponse(); assert.strictEqual(req.status, 404); - assert.strictEqual(req.statusText, 'Not Found'); + assert.strictEqual(req.statusText, ''); assert.strictEqual(req.headers.get('Content-Type'), 'abc'); assert.deepEqual(req.cf, { foo: 123, bar: 'def' }); diff --git a/src/workerd/api/tests/response-status-text-target.js b/src/workerd/api/tests/response-status-text-target.js new file mode 100644 index 00000000000..a8e63ec7667 --- /dev/null +++ b/src/workerd/api/tests/response-status-text-target.js @@ -0,0 +1,13 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 +export default { + fetch(request) { + const params = new URL(request.url).searchParams; + const init = { status: Number(params.get('status')) }; + if (params.has('statusText')) { + init.statusText = params.get('statusText'); + } + return new Response(null, init); + }, +}; diff --git a/src/workerd/api/tests/response-status-text-test.js b/src/workerd/api/tests/response-status-text-test.js new file mode 100644 index 00000000000..e3a625798c5 --- /dev/null +++ b/src/workerd/api/tests/response-status-text-test.js @@ -0,0 +1,38 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 +import { strictEqual } from 'node:assert'; + +// A Response with no statusText has an empty status message, as the Fetch spec requires, on +// every compatibility date. These Workers run without pedantic_wpt, which WPT always enables. +export const emptyByDefault = { + test() { + strictEqual(new Response(null, { status: 404 }).statusText, ''); + strictEqual(Response.json({}).statusText, ''); + strictEqual(Response.redirect('http://example.com/').statusText, ''); + strictEqual( + new Response(null, { statusText: 'Custom' }).statusText, + 'Custom' + ); + }, +}; + +// The HTTP status line still carries the status code's conventional reason phrase when no +// statusText is set. The target service returns Responses built from the query parameters, and +// fetch() reports the reason phrase it received. WPT can't observe what a Worker sends. +export const reasonPhraseOnTheWire = { + async test(ctrl, env) { + const url = (params) => + `http://target/?${new URLSearchParams(params).toString()}`; + + strictEqual( + (await env.target.fetch(url({ status: 404 }))).statusText, + 'Not Found' + ); + strictEqual( + (await env.target.fetch(url({ status: 200, statusText: 'Custom' }))) + .statusText, + 'Custom' + ); + }, +}; diff --git a/src/workerd/api/tests/response-status-text-test.wd-test b/src/workerd/api/tests/response-status-text-test.wd-test new file mode 100644 index 00000000000..9b45bd0c402 --- /dev/null +++ b/src/workerd/api/tests/response-status-text-test.wd-test @@ -0,0 +1,25 @@ +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [ + ( name = "response-status-text-test", + worker = ( + modules = [ + (name = "worker", esModule = embed "response-status-text-test.js") + ], + compatibilityFlags = ["nodejs_compat"], + bindings = [ + (name = "target", service = "response-status-text-target"), + ], + ) + ), + ( name = "response-status-text-target", + worker = ( + modules = [ + (name = "worker", esModule = embed "response-status-text-target.js") + ], + compatibilityFlags = ["nodejs_compat"], + ) + ), + ], +); diff --git a/src/workerd/server/tests/python/sdk/worker.py b/src/workerd/server/tests/python/sdk/worker.py index 6ea49ce6a25..1bef7374d07 100644 --- a/src/workerd/server/tests/python/sdk/worker.py +++ b/src/workerd/server/tests/python/sdk/worker.py @@ -463,8 +463,7 @@ async def response_unit_tests(env): response_json = Response.json([1, 2, 3]) assert await response_json.text() == "[1, 2, 3]" assert ( - repr(response_json) - == "Response(status=200, status_text='OK', content_type='application/json')" + repr(response_json) == "Response(status=200, content_type='application/json')" ) response_json = Response.from_json([1, 2, 3]) diff --git a/src/wpt/fetch/api-test-ts.ts b/src/wpt/fetch/api-test-ts.ts index 75b737e7705..441ff31ce04 100644 --- a/src/wpt/fetch/api-test-ts.ts +++ b/src/wpt/fetch/api-test-ts.ts @@ -811,7 +811,6 @@ export default { 'response/response-clone.any.js': { comment: 'TODO Investigate this', expectedFailures: [ - "Check Response's clone with default values, without body", 'Check response clone use structureClone for teed ReadableStreams (Int8Arraychunk)', 'Check response clone use structureClone for teed ReadableStreams (Int16Arraychunk)', 'Check response clone use structureClone for teed ReadableStreams (Int32Arraychunk)', @@ -844,34 +843,12 @@ export default { 'response/response-error.any.js': {}, 'response/response-from-stream.any.js': {}, 'response/response-headers-guard.any.js': {}, - 'response/response-init-001.any.js': { - comment: 'statusText should be inited to OK', - expectedFailures: ['Check default value for statusText attribute'], - }, + 'response/response-init-001.any.js': {}, 'response/response-init-002.any.js': {}, 'response/response-init-contenttype.any.js': {}, 'response/response-static-error.any.js': {}, - 'response/response-static-json.any.js': { - comment: 'statusText does not match the status code', - expectedFailures: [ - 'Check response returned by static json() with init undefined', - 'Check response returned by static json() with init {"status":400}', - 'Check response returned by static json() with init {"headers":{}}', - 'Check response returned by static json() with init {"headers":{"content-type":"foo/bar"}}', - 'Check response returned by static json() with init {"headers":{"x-foo":"bar"}}', - ], - }, - 'response/response-static-redirect.any.js': { - comment: 'statusText does not match the status code', - expectedFailures: [ - 'Check default redirect response', - 'Check response returned by static method redirect(), status = 301', - 'Check response returned by static method redirect(), status = 302', - 'Check response returned by static method redirect(), status = 303', - 'Check response returned by static method redirect(), status = 307', - 'Check response returned by static method redirect(), status = 308', - ], - }, + 'response/response-static-json.any.js': {}, + 'response/response-static-redirect.any.js': {}, 'response/response-stream-bad-chunk.any.js': {}, 'response/response-stream-disturbed-1.any.js': {}, 'response/response-stream-disturbed-2.any.js': {}, diff --git a/src/wpt/fetch/api-test.ts b/src/wpt/fetch/api-test.ts index c21861f4175..e4c7b97cfc8 100644 --- a/src/wpt/fetch/api-test.ts +++ b/src/wpt/fetch/api-test.ts @@ -808,7 +808,6 @@ export default { 'response/response-clone.any.js': { comment: 'TODO Investigate this', expectedFailures: [ - "Check Response's clone with default values, without body", 'Check response clone use structureClone for teed ReadableStreams (Int8Arraychunk)', 'Check response clone use structureClone for teed ReadableStreams (Int16Arraychunk)', 'Check response clone use structureClone for teed ReadableStreams (Int32Arraychunk)', @@ -841,34 +840,12 @@ export default { 'response/response-error.any.js': {}, 'response/response-from-stream.any.js': {}, 'response/response-headers-guard.any.js': {}, - 'response/response-init-001.any.js': { - comment: 'statusText should be inited to OK', - expectedFailures: ['Check default value for statusText attribute'], - }, + 'response/response-init-001.any.js': {}, 'response/response-init-002.any.js': {}, 'response/response-init-contenttype.any.js': {}, 'response/response-static-error.any.js': {}, - 'response/response-static-json.any.js': { - comment: 'statusText does not match the status code', - expectedFailures: [ - 'Check response returned by static json() with init undefined', - 'Check response returned by static json() with init {"status":400}', - 'Check response returned by static json() with init {"headers":{}}', - 'Check response returned by static json() with init {"headers":{"content-type":"foo/bar"}}', - 'Check response returned by static json() with init {"headers":{"x-foo":"bar"}}', - ], - }, - 'response/response-static-redirect.any.js': { - comment: 'statusText does not match the status code', - expectedFailures: [ - 'Check default redirect response', - 'Check response returned by static method redirect(), status = 301', - 'Check response returned by static method redirect(), status = 302', - 'Check response returned by static method redirect(), status = 303', - 'Check response returned by static method redirect(), status = 307', - 'Check response returned by static method redirect(), status = 308', - ], - }, + 'response/response-static-json.any.js': {}, + 'response/response-static-redirect.any.js': {}, 'response/response-stream-bad-chunk.any.js': {}, 'response/response-stream-disturbed-1.any.js': {}, 'response/response-stream-disturbed-2.any.js': {},