From 9ad0d4c3db85ce9f8007dc088d164bcc7f544708 Mon Sep 17 00:00:00 2001
From: Yoshiya Hinosawa <stibium121@gmail.com>
Date: Mon, 6 Jan 2025 16:12:21 +0900
Subject: [PATCH] fix(ext/http): improve error message when underlying resource
 of request body unavailable (#27463)

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

closes #27133
---
 ext/http/00_serve.ts     | 20 +++++++++++++++++-
 ext/web/06_streams.js    |  8 +++++--
 tests/unit/serve_test.ts | 45 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/ext/http/00_serve.ts b/ext/http/00_serve.ts
index 6e5f25b473..0ef06d1902 100644
--- a/ext/http/00_serve.ts
+++ b/ext/http/00_serve.ts
@@ -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;
   }
 
diff --git a/ext/web/06_streams.js b/ext/web/06_streams.js
index 0b9ae538e2..950d46e829 100644
--- a/ext/web/06_streams.js
+++ b/ext/web/06_streams.js
@@ -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();
       }
     },
diff --git a/tests/unit/serve_test.ts b/tests/unit/serve_test.ts
index 88aecefbcb..09616b0151 100644
--- a/tests/unit/serve_test.ts
+++ b/tests/unit/serve_test.ts
@@ -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",
+  );
+});