Skip to content

Commit

Permalink
fix(ext/http): improve error message when underlying resource of requ…
Browse files Browse the repository at this point in the history
…est body unavailable (denoland#27463)

The error message is currently `Bad Resource ID`. This commit changes it to
`Cannot read request body as underlying resource unavailable`

closes denoland#27133
  • Loading branch information
kt3k authored Jan 6, 2025
1 parent 7cabd02 commit 9ad0d4c
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 4 deletions.
20 changes: 19 additions & 1 deletion ext/http/00_serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,25 @@ class InnerRequest {
return null;
}
this.#streamRid = op_http_read_request_body(this.#external);
this.#body = new InnerBody(readableStreamForRid(this.#streamRid, false));
this.#body = new InnerBody(
readableStreamForRid(
this.#streamRid,
false,
undefined,
(controller, error) => {
if (ObjectPrototypeIsPrototypeOf(BadResourcePrototype, error)) {
// TODO(kt3k): We would like to pass `error` as `cause` when BadResource supports it.
controller.error(
new error.constructor(
`Cannot read request body as underlying resource unavailable`,
),
);
} else {
controller.error(error);
}
},
),
);
return this.#body;
}

Expand Down
8 changes: 6 additions & 2 deletions ext/web/06_streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ const _original = Symbol("[[original]]");
* @param {boolean=} autoClose If the resource should be auto-closed when the stream closes. Defaults to true.
* @returns {ReadableStream<Uint8Array>}
*/
function readableStreamForRid(rid, autoClose = true, Super) {
function readableStreamForRid(rid, autoClose = true, Super, onError) {
const stream = new (Super ?? ReadableStream)(_brand);
stream[_resourceBacking] = { rid, autoClose };

Expand Down Expand Up @@ -947,7 +947,11 @@ function readableStreamForRid(rid, autoClose = true, Super) {
controller.byobRequest.respond(bytesRead);
}
} catch (e) {
controller.error(e);
if (onError) {
onError(controller, e);
} else {
controller.error(e);
}
tryClose();
}
},
Expand Down
45 changes: 44 additions & 1 deletion tests/unit/serve_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

// deno-lint-ignore-file no-console

import { assertMatch, assertRejects } from "@std/assert";
import { assertIsError, assertMatch, assertRejects } from "@std/assert";
import { Buffer, BufReader, BufWriter, type Reader } from "@std/io";
import { TextProtoReader } from "../testdata/run/textproto.ts";
import {
Expand Down Expand Up @@ -4378,3 +4378,46 @@ Deno.test(
await server.finished;
},
);

Deno.test({
name:
"req.body.getReader().read() throws the error with reasonable error message",
}, async () => {
const { promise, resolve, reject } = Promise.withResolvers<Error>();
const server = Deno.serve({ onListen, port: 0 }, async (req) => {
const reader = req.body!.getReader();

try {
while (true) {
const { done } = await reader.read();
if (done) break;
}
} catch (e) {
// deno-lint-ignore no-explicit-any
resolve(e as any);
}

reject(new Error("Should not reach here"));
server.shutdown();
return new Response();
});

async function onListen({ port }: { port: number }) {
const body = "a".repeat(1000);
const request = `POST / HTTP/1.1\r\n` +
`Host: 127.0.0.1:${port}\r\n` +
`Content-Length: 1000\r\n` +
"\r\n" + body;

const connection = await Deno.connect({ hostname: "127.0.0.1", port });
await connection.write(new TextEncoder().encode(request));
connection.close();
}
await server.finished;
const e = await promise;
assertIsError(
e,
Deno.errors.BadResource,
"Cannot read request body as underlying resource unavailable",
);
});

0 comments on commit 9ad0d4c

Please sign in to comment.