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': {},