Repository: denoland/deno
Pull request: fix(ext/http): improve error message when underlying resource of request body unavailable

This PR improves the error message when reading request body in `Deno.serve` and the underlying resource is unavailable.

The error message is currently `Bad Resource ID`. This PR change it to `Cannot read request body as underlying resource unavailable`

closes #27133

Changed files:
filename: ext/http/00_serve.ts
patch: @@ -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;
   }
 

filename: ext/web/06_streams.js
patch: @@ -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 };
 
@@ -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();
       }
     },

filename: tests/unit/serve_test.ts
patch: @@ -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 {
@@ -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",
+  );
+});