Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BadResource Error with Request.text() in Deno #27132

Open
PutziSan opened this issue Nov 28, 2024 · 1 comment · May be fixed by #27429
Open

BadResource Error with Request.text() in Deno #27132

PutziSan opened this issue Nov 28, 2024 · 1 comment · May be fixed by #27429
Assignees
@kt3k
Labels
ext/http related to ext/http triage required 👀 Deno team needs to make a decision if this change is desired

Comments

@PutziSan
Copy link

PutziSan commented Nov 28, 2024 •
edited
Loading

Version: deno 2.1.1 (stable, release, x86_64-unknown-linux-gnu)

The following Deno server code produces seemingly random BadResource: Bad resource ID errors:

Deno.serve(async (req) => new Response(await req.text()));

Observed Behavior

The error probably occurs when the TCP connection is closed before the request body is fully read. Example error:

BadResource: Bad resource ID
    at readableStreamCollectIntoUint8Array (ext:deno_web/06_streams.js:1060:23)
    at InnerBody.consume (ext:deno_fetch/22_body.js:163:14)
    at consumeBody (ext:deno_fetch/22_body.js:255:34)
    at Request.text (ext:deno_fetch/22_body.js:351:16)
    at file:///<...>/server.ts:1:50
    at ext:deno_http/00_serve.ts:382:26
    at ext:deno_http/00_serve.ts:573:33
    at eventLoopTick (ext:core/01_core.js:175:7) {
  name: "BadResource"
}

A Wireshark capture of a failed request shows the client closing the connection immediately after sending a POST request.

495880	2024-11-26 09:35:57,705529846	127.0.0.1	127.0.0.1	HTTP/JSON	207	POST / HTTP/1.1 , JavaScript Object Notation (application/json)
496787	2024-11-26 09:35:57,714119877	127.0.0.1	127.0.0.1	TCP	66	56952 → 8080 [FIN, ACK] Seq=14383 Ack=10808 Win=65536 Len=0 TSval=658758687 TSecr=658758679
497150	2024-11-26 09:35:57,716091194	127.0.0.1	127.0.0.1	TCP	66	8080 → 56952 [FIN, ACK] Seq=10808 Ack=14384 Win=65536 Len=0 TSval=658758689 TSecr=658758679
497151	2024-11-26 09:35:57,716093907	127.0.0.1	127.0.0.1	TCP	66	56952 → 8080 [ACK] Seq=14384 Ack=10809 Win=65536 Len=0 TSval=658758689 TSecr=658758689

Expected Behavior

The server should not throw an error or throw a more descriptive error.

Context

I encountered this error while load testing my Deno server using wrk. The error occurred only on the end of a load test run, but not on every run.

Steps to Reproduce

A repository with unstable reproduction steps is available: deno-server-bug-repro.
@PutziSan PutziSan mentioned this issue Nov 28, 2024
Open
@bartlomieju bartlomieju added ext/http related to ext/http triage required 👀 Deno team needs to make a decision if this change is desired labels Dec 6, 2024
@kt3k
Copy link
Member

kt3k commented Dec 18, 2024

I was able to reproduce the error using the repro repo.

    at Request.text (ext:deno_fetch/22_body.js:351:16)
    at file:///<...>/server.ts:1:50

This part of stack trace tells that the error was thrown from req.text() call. This error can be handled in the user code like:

-Deno.serve(async (req) => new Response(await req.text()));
+Deno.serve(async (req) => {
+  try {
+    return new Response(await req.text())
+  } catch (e) {
+    if (e instanceof Deno.errors.BadResource) {
+      return new Response("Bad Request", { status: 400 });
+    }
+    throw e
+  }
+});

The server should not throw an error or throw a more descriptive error.

I disagree with swallowing or ignoring this error, but I agree this error can be more descriptive.

@kt3k kt3k self-assigned this Dec 18, 2024
@kt3k kt3k linked a pull request Dec 20, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kt3k kt3k

Labels
ext/http related to ext/http triage required 👀 Deno team needs to make a decision if this change is desired
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(ext/fetch): better error message when body resource is unavailable kt3k/deno
3 participants
@kt3k @PutziSan @bartlomieju