Skip to content

Commit 27b5944

Browse files
committed
Implement Response.error()
Signed-off-by: James M Snell <jsnell@cloudflare.com>
1 parent 8c8ea21 commit 27b5944

28 files changed

+91
-12
lines changed

src/workerd/api/cache.c++

+3
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,9 @@ jsg::Promise<void> Cache::put(jsg::Lock& js,
154154
jsg::Ref<Response> jsResponse,
155155
CompatibilityFlags::Reader flags) {
156156

157+
JSG_REQUIRE(
158+
jsResponse->getType() != "error"_kj, TypeError, "Cache is unble to store an error response");
159+
157160
// Fake kj::HttpService::Response implementation that allows us to reuse jsResponse->send() to
158161
// serialize the response (headers + body) in the format needed to serve as the payload of
159162
// our cache PUT request.

src/workerd/api/eventsource.c++

+5
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,11 @@ void EventSource::run(jsg::Lock& js,
448448
kj::Maybe<jsg::Ref<Fetcher>> fetcher) {
449449
notifyOpen(js);
450450

451+
KJ_IF_SOME(resp, response) {
452+
JSG_REQUIRE(resp->getType() != "error"_kj, TypeError,
453+
"Error responses are unsupported with EventSource");
454+
}
455+
451456
auto onSuccess =
452457
JSG_VISITABLE_LAMBDA((self = JSG_THIS, readable = readable.addRef(), withReconnection,
453458
response = addRef(response), fetcher = addRef(fetcher)),

src/workerd/api/global-scope.c++

+3
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,9 @@ kj::Promise<DeferredProxy<void>> ServiceWorkerGlobalScope::request(kj::HttpMetho
267267
canceled = kj::addRef(*canceled), &headers, span = kj::mv(span)](
268268
jsg::Lock& js, jsg::Ref<Response> innerResponse) mutable
269269
-> IoOwn<kj::Promise<DeferredProxy<void>>> {
270+
JSG_REQUIRE(innerResponse->getType() != "error"_kj, TypeError,
271+
"Return value from serve handler must not be an error response (like Response.error())");
272+
270273
auto& context = IoContext::current();
271274
// Drop our fetch_handler span now that the promise has resolved.
272275
span = kj::none;

src/workerd/api/html-rewriter.c++

+4
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,10 @@ jsg::Ref<HTMLRewriter> HTMLRewriter::onDocument(DocumentContentHandlers&& handle
12161216
}
12171217

12181218
jsg::Ref<Response> HTMLRewriter::transform(jsg::Lock& js, jsg::Ref<Response> response) {
1219+
1220+
JSG_REQUIRE(response->getType() != "error"_kj, TypeError,
1221+
"HTMLRewriter cannot transform an error response");
1222+
12191223
auto maybeInput = response->getBody();
12201224

12211225
if (maybeInput == kj::none) {

src/workerd/api/http-standard-test.js

+31-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { strictEqual } from 'node:assert';
1+
import { strictEqual, rejects, throws } from 'node:assert';
22

33
export default {
44
async fetch(req) {
@@ -11,8 +11,17 @@ export default {
1111
});
1212
} else if (req.url.endsWith('/2')) {
1313
return new Response('ok');
14+
} else if (req.url.endsWith('/error')) {
15+
const resp = Response.error();
16+
strictEqual(resp.type, 'error');
17+
strictEqual(resp.status, 0);
18+
return resp;
1419
}
1520
},
21+
22+
foo() {
23+
return Response.error();
24+
},
1625
};
1726

1827
export const test = {
@@ -33,3 +42,24 @@ export const typeTest = {
3342
strictEqual(resp.type, 'default');
3443
},
3544
};
45+
46+
export const error = {
47+
async test(_, env) {
48+
await rejects(env.SERVICE.fetch('http://example/error'), {
49+
message:
50+
'Return value from serve handler must not be an error ' +
51+
'response (like Response.error())',
52+
});
53+
54+
const errorResp = Response.error();
55+
throws(() => new Response('', errorResp), {
56+
message:
57+
'Responses may only be constructed with status codes in ' +
58+
'the range 200 to 599, inclusive.',
59+
});
60+
61+
const errorRespRpc = await env.SERVICE.foo(null);
62+
strictEqual(errorRespRpc.type, 'error');
63+
strictEqual(errorRespRpc.ok, false);
64+
},
65+
};

src/workerd/api/http-standard-test.wd-test

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ const unitTests :Workerd.Config = (
1010
bindings = [
1111
( name = "SERVICE", service = "http-standard-test" )
1212
],
13-
compatibilityDate = "2023-08-01",
14-
compatibilityFlags = ["nodejs_compat", "fetch_standard_url"],
13+
compatibilityDate = "2025-02-01",
14+
compatibilityFlags = ["nodejs_compat"],
1515
)
1616
),
1717
],

src/workerd/api/http.c++

+18-1
Original file line numberDiff line numberDiff line change
@@ -1334,6 +1334,10 @@ Response::Response(jsg::Lock& js,
13341334
// Defined later in this file.
13351335
static kj::StringPtr defaultStatusText(uint statusCode);
13361336

1337+
jsg::Ref<Response> Response::error(jsg::Lock& js) {
1338+
return jsg::alloc<Response>(js, 0, kj::String(), jsg::alloc<Headers>(), CfProperty(), kj::none);
1339+
};
1340+
13371341
jsg::Ref<Response> Response::constructor(jsg::Lock& js,
13381342
jsg::Optional<kj::Maybe<Body::Initializer>> optionalBodyInit,
13391343
jsg::Optional<Initializer> maybeInit) {
@@ -1732,6 +1736,15 @@ jsg::Ref<Response> Response::deserialize(jsg::Lock& js,
17321736
const jsg::TypeHandler<kj::Maybe<jsg::Ref<ReadableStream>>>& streamHandler) {
17331737
auto body = KJ_ASSERT_NONNULL(streamHandler.tryUnwrap(js, deserializer.readValue(js)));
17341738
auto init = KJ_ASSERT_NONNULL(initDictHandler.tryUnwrap(js, deserializer.readValue(js)));
1739+
1740+
// If the status code is zero, then it was an error response. We cannot
1741+
// use the Response::constructor.
1742+
KJ_IF_SOME(status, init.status) {
1743+
if (status == 0) {
1744+
return Response::error(js);
1745+
}
1746+
}
1747+
17351748
return Response::constructor(js, kj::mv(body), kj::mv(init));
17361749
}
17371750

@@ -2388,7 +2401,7 @@ jsg::Promise<Fetcher::GetResult> Fetcher::get(
23882401
uint status = response->getStatus();
23892402
if (status == 404 || status == 410) {
23902403
return js.resolvedPromise(GetResult(js.v8Ref(js.v8Null())));
2391-
} else if (status < 200 || status >= 300) {
2404+
} else if (response->getOk()) {
23922405
// Manually construct exception so that we can incorporate method and status into the text
23932406
// that JavaScript sees.
23942407
// TODO(someday): Would be nice to attach the response to the JavaScript error, maybe? Or
@@ -2680,6 +2693,10 @@ static kj::StringPtr defaultStatusText(uint statusCode) {
26802693
return "Client Error"_kj;
26812694
} else if (statusCode >= 500 && statusCode < 600) {
26822695
return "Server Error"_kj;
2696+
} else if (statusCode == 0) {
2697+
// Status code 0 is used exclusively with error responses
2698+
// created using Response.error()
2699+
return ""_kj;
26832700
} else {
26842701
KJ_UNREACHABLE;
26852702
}

src/workerd/api/http.h

+5-8
Original file line numberDiff line numberDiff line change
@@ -1097,13 +1097,7 @@ class Response final: public Body {
10971097
//
10981098
// A network error is a response whose status is always 0, status message is always the empty
10991099
// byte sequence, header list is always empty, body is always null, and trailer is always empty.
1100-
static jsg::Unimplemented error() { return {}; };
1101-
// TODO(conform): implementation is missing; two approaches where tested:
1102-
// - returning a HTTP 5xx response but that doesn't match the spec and we didn't
1103-
// find it useful.
1104-
// - throwing/propagating a DISCONNECTED kj::Exception to actually disconnect the
1105-
// client. However, we were concerned about possible side-effects and incorrect
1106-
// error reporting.
1100+
static jsg::Ref<Response> error(jsg::Lock& js);
11071101

11081102
jsg::Ref<Response> clone(jsg::Lock& js);
11091103

@@ -1139,7 +1133,10 @@ class Response final: public Body {
11391133
// determined that only the `'default'` and `'error'` properties should be implemented.
11401134
// We currently due not implement Response.error() so "default" is the only value we
11411135
// currently support.
1142-
kj::StringPtr getType() { return "default"_kj; }
1136+
kj::StringPtr getType() {
1137+
if (statusCode == 0) return "error"_kj;
1138+
return "default"_kj;
1139+
}
11431140

11441141
JSG_RESOURCE_TYPE(Response, CompatibilityFlags::Reader flags) {
11451142
JSG_INHERIT(Body);

types/generated-snapshot/2021-11-03/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1566,6 +1566,7 @@ declare abstract class Body {
15661566
declare var Response: {
15671567
prototype: Response;
15681568
new (body?: BodyInit | null, init?: ResponseInit): Response;
1569+
error(): Response;
15691570
redirect(url: string, status?: number): Response;
15701571
json(any: any, maybeInit?: ResponseInit | Response): Response;
15711572
};

types/generated-snapshot/2021-11-03/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1571,6 +1571,7 @@ export declare abstract class Body {
15711571
export declare var Response: {
15721572
prototype: Response;
15731573
new (body?: BodyInit | null, init?: ResponseInit): Response;
1574+
error(): Response;
15741575
redirect(url: string, status?: number): Response;
15751576
json(any: any, maybeInit?: ResponseInit | Response): Response;
15761577
};

types/generated-snapshot/2022-01-31/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1572,6 +1572,7 @@ declare abstract class Body {
15721572
declare var Response: {
15731573
prototype: Response;
15741574
new (body?: BodyInit | null, init?: ResponseInit): Response;
1575+
error(): Response;
15751576
redirect(url: string, status?: number): Response;
15761577
json(any: any, maybeInit?: ResponseInit | Response): Response;
15771578
};

types/generated-snapshot/2022-01-31/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1577,6 +1577,7 @@ export declare abstract class Body {
15771577
export declare var Response: {
15781578
prototype: Response;
15791579
new (body?: BodyInit | null, init?: ResponseInit): Response;
1580+
error(): Response;
15801581
redirect(url: string, status?: number): Response;
15811582
json(any: any, maybeInit?: ResponseInit | Response): Response;
15821583
};

types/generated-snapshot/2022-03-21/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1590,6 +1590,7 @@ declare abstract class Body {
15901590
declare var Response: {
15911591
prototype: Response;
15921592
new (body?: BodyInit | null, init?: ResponseInit): Response;
1593+
error(): Response;
15931594
redirect(url: string, status?: number): Response;
15941595
json(any: any, maybeInit?: ResponseInit | Response): Response;
15951596
};

types/generated-snapshot/2022-03-21/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,7 @@ export declare abstract class Body {
15951595
export declare var Response: {
15961596
prototype: Response;
15971597
new (body?: BodyInit | null, init?: ResponseInit): Response;
1598+
error(): Response;
15981599
redirect(url: string, status?: number): Response;
15991600
json(any: any, maybeInit?: ResponseInit | Response): Response;
16001601
};

types/generated-snapshot/2022-08-04/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1590,6 +1590,7 @@ declare abstract class Body {
15901590
declare var Response: {
15911591
prototype: Response;
15921592
new (body?: BodyInit | null, init?: ResponseInit): Response;
1593+
error(): Response;
15931594
redirect(url: string, status?: number): Response;
15941595
json(any: any, maybeInit?: ResponseInit | Response): Response;
15951596
};

types/generated-snapshot/2022-08-04/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,7 @@ export declare abstract class Body {
15951595
export declare var Response: {
15961596
prototype: Response;
15971597
new (body?: BodyInit | null, init?: ResponseInit): Response;
1598+
error(): Response;
15981599
redirect(url: string, status?: number): Response;
15991600
json(any: any, maybeInit?: ResponseInit | Response): Response;
16001601
};

types/generated-snapshot/2022-10-31/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1590,6 +1590,7 @@ declare abstract class Body {
15901590
declare var Response: {
15911591
prototype: Response;
15921592
new (body?: BodyInit | null, init?: ResponseInit): Response;
1593+
error(): Response;
15931594
redirect(url: string, status?: number): Response;
15941595
json(any: any, maybeInit?: ResponseInit | Response): Response;
15951596
};

types/generated-snapshot/2022-10-31/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,7 @@ export declare abstract class Body {
15951595
export declare var Response: {
15961596
prototype: Response;
15971597
new (body?: BodyInit | null, init?: ResponseInit): Response;
1598+
error(): Response;
15981599
redirect(url: string, status?: number): Response;
15991600
json(any: any, maybeInit?: ResponseInit | Response): Response;
16001601
};

types/generated-snapshot/2022-11-30/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,7 @@ declare abstract class Body {
15951595
declare var Response: {
15961596
prototype: Response;
15971597
new (body?: BodyInit | null, init?: ResponseInit): Response;
1598+
error(): Response;
15981599
redirect(url: string, status?: number): Response;
15991600
json(any: any, maybeInit?: ResponseInit | Response): Response;
16001601
};

types/generated-snapshot/2022-11-30/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1600,6 +1600,7 @@ export declare abstract class Body {
16001600
export declare var Response: {
16011601
prototype: Response;
16021602
new (body?: BodyInit | null, init?: ResponseInit): Response;
1603+
error(): Response;
16031604
redirect(url: string, status?: number): Response;
16041605
json(any: any, maybeInit?: ResponseInit | Response): Response;
16051606
};

types/generated-snapshot/2023-03-01/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1597,6 +1597,7 @@ declare abstract class Body {
15971597
declare var Response: {
15981598
prototype: Response;
15991599
new (body?: BodyInit | null, init?: ResponseInit): Response;
1600+
error(): Response;
16001601
redirect(url: string, status?: number): Response;
16011602
json(any: any, maybeInit?: ResponseInit | Response): Response;
16021603
};

types/generated-snapshot/2023-03-01/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1602,6 +1602,7 @@ export declare abstract class Body {
16021602
export declare var Response: {
16031603
prototype: Response;
16041604
new (body?: BodyInit | null, init?: ResponseInit): Response;
1605+
error(): Response;
16051606
redirect(url: string, status?: number): Response;
16061607
json(any: any, maybeInit?: ResponseInit | Response): Response;
16071608
};

types/generated-snapshot/2023-07-01/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1597,6 +1597,7 @@ declare abstract class Body {
15971597
declare var Response: {
15981598
prototype: Response;
15991599
new (body?: BodyInit | null, init?: ResponseInit): Response;
1600+
error(): Response;
16001601
redirect(url: string, status?: number): Response;
16011602
json(any: any, maybeInit?: ResponseInit | Response): Response;
16021603
};

types/generated-snapshot/2023-07-01/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1602,6 +1602,7 @@ export declare abstract class Body {
16021602
export declare var Response: {
16031603
prototype: Response;
16041604
new (body?: BodyInit | null, init?: ResponseInit): Response;
1605+
error(): Response;
16051606
redirect(url: string, status?: number): Response;
16061607
json(any: any, maybeInit?: ResponseInit | Response): Response;
16071608
};

types/generated-snapshot/experimental/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1607,6 +1607,7 @@ declare abstract class Body {
16071607
declare var Response: {
16081608
prototype: Response;
16091609
new (body?: BodyInit | null, init?: ResponseInit): Response;
1610+
error(): Response;
16101611
redirect(url: string, status?: number): Response;
16111612
json(any: any, maybeInit?: ResponseInit | Response): Response;
16121613
};

types/generated-snapshot/experimental/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1612,6 +1612,7 @@ export declare abstract class Body {
16121612
export declare var Response: {
16131613
prototype: Response;
16141614
new (body?: BodyInit | null, init?: ResponseInit): Response;
1615+
error(): Response;
16151616
redirect(url: string, status?: number): Response;
16161617
json(any: any, maybeInit?: ResponseInit | Response): Response;
16171618
};

types/generated-snapshot/oldest/index.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1566,6 +1566,7 @@ declare abstract class Body {
15661566
declare var Response: {
15671567
prototype: Response;
15681568
new (body?: BodyInit | null, init?: ResponseInit): Response;
1569+
error(): Response;
15691570
redirect(url: string, status?: number): Response;
15701571
json(any: any, maybeInit?: ResponseInit | Response): Response;
15711572
};

types/generated-snapshot/oldest/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1571,6 +1571,7 @@ export declare abstract class Body {
15711571
export declare var Response: {
15721572
prototype: Response;
15731573
new (body?: BodyInit | null, init?: ResponseInit): Response;
1574+
error(): Response;
15741575
redirect(url: string, status?: number): Response;
15751576
json(any: any, maybeInit?: ResponseInit | Response): Response;
15761577
};

0 commit comments

Comments
 (0)