Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions src/runtime/webcore/fetch/FetchTasklet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1622,6 +1622,17 @@ impl FetchTasklet {
if this.signal_store.body_receive_mode() == BodyReceiveMode::Ignore {
return;
}
// reader.cancel() on a still-arriving body aborts the fetch so the
// connection closes and the server observes the cancellation. A fully
// received body is left to drain/cleanup so the socket stays reusable.
// `result` is replaced wholesale by the HTTP thread under `mutex`, so
// read `has_more` under the lock like the other JS-thread callbacks.
this.mutex.lock();
let has_more = this.result.has_more;
this.mutex.unlock();
Comment thread
robobun marked this conversation as resolved.
Outdated
if has_more {
this.abort_task();
}
this.ignore_remaining_response_body(false);
}

Expand Down Expand Up @@ -1766,14 +1777,22 @@ impl FetchTasklet {
}
// enabling streaming will make the http thread to drain into the main thread (aka stop buffering)
// without a stream ref, response body or response instance alive it will just ignore the result
// An aborted fetch (reader.cancel() on an in-flight body, or an
// AbortSignal) is already shutting the connection down; don't re-arm the
// receive or resume draining, which would read the rest of an unbounded
// body and hold the socket open (drain_events resumes before shutdowns).
let aborted = self.signal_store.aborted.load(Ordering::Relaxed);
if self
.signal_store
.set_receive_mode_terminal(BodyReceiveMode::Ignore)
&& !aborted
{
self.schedule_receive_resume();
}
if let Some(http_) = self.http.as_mut() {
http_.enable_response_body_streaming();
if !aborted {
http_.enable_response_body_streaming();
}
Comment thread
robobun marked this conversation as resolved.
}
// we should not keep the process alive if we are ignoring the body
let _ = self.javascript_vm;
Expand Down Expand Up @@ -2254,7 +2273,12 @@ impl FetchTasklet {
}

pub(crate) fn abort_task(&mut self) {
self.signal_store.aborted.store(true, Ordering::Relaxed);
// Idempotent: reader.cancel() and an AbortSignal can both reach here for
// the same fetch. Only the first abort enqueues a shutdown; a second
// would append a redundant ShutdownMessage for an already-closing socket.
if self.signal_store.aborted.swap(true, Ordering::Relaxed) {
return;
}
self.tracker.did_cancel(&self.global_this);

if let Some(http_) = self.http.as_mut() {
Expand Down
16 changes: 8 additions & 8 deletions test/js/web/fetch/fetch-backpressure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,21 +300,21 @@
});

describe.concurrent("fetch() receive backpressure — streaming consumer shapes", () => {
test("reader.cancel() resumes the socket so the abandoned body drains", async () => {
test("reader.cancel() aborts the in-flight request instead of draining the abandoned body", async () => {
await using server = await serve("h1");
const r1 = await fetch(server.url, { keepalive: true });
const reader = r1.body!.getReader();
await reader.read();
await Bun.sleep(50);
await reader.cancel();
// The cancelled body must drain before the connection is poolable;
// poll server.sent() instead of asserting a connection count.
while (server.sent() < TOTAL) await Bun.sleep(5);
// Cancelling an in-flight body aborts the request (matching Node/Deno and
// browsers, see #33227): the connection closes, so the server never sends
// the whole body. A subsequent request still succeeds on a fresh connection.
const buf = await (await fetch(server.url, { keepalive: true })).arrayBuffer();
expect({ byteLength: buf.byteLength, sent: server.sent() }).toEqual({
byteLength: TOTAL,
sent: 2 * TOTAL,
});
expect(buf.byteLength).toBe(TOTAL);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Drain-for-keepalive would push the first body to completion too, reaching
// 2 * TOTAL; the abort leaves the first response only partially sent.
expect(server.sent()).toBeLessThan(2 * TOTAL);

Check warning on line 317 in test/js/web/fetch/fetch-backpressure.test.ts

View check run for this annotation

Claude / Claude Code Review

toBeLessThan(2*TOTAL) may flake on large-kernel-buffer CI hosts

This assertion depends on the 16 MiB body being large enough that the server's `push()` loop blocks before finishing — but the sibling test just above (line 232) documents that some CI hosts have `tcp_rmem[2]+tcp_wmem[2]` approaching 256 MiB and uses a 1 GiB body for exactly that reason. On those hosts the server writes all 16 MiB into kernel buffers during the 50 ms stall, req 1 contributes `TOTAL`, req 2 adds `TOTAL`, and `toBeLessThan(2 * TOTAL)` fails with equality. Since the abort-vs-drain
Comment thread
robobun marked this conversation as resolved.
Outdated
});

test("res.body.tee() both branches drain", async () => {
Expand Down
45 changes: 45 additions & 0 deletions test/regression/issue/33227.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { sleep } from "bun";
import { expect, test } from "bun:test";

// https://github.com/oven-sh/bun/issues/33227
test("reader.cancel() aborts the fetch and triggers the server's stream cancel", async () => {
const { promise: sawAbort, resolve: onAbort } = Promise.withResolvers<void>();
const { promise: sawCancel, resolve: onCancel } = Promise.withResolvers<void>();
const encoder = new TextEncoder();

using server = Bun.serve({
port: 0,
fetch(request) {
request.signal.addEventListener("abort", () => onAbort());
let count = 0;
return new Response(
new ReadableStream({
// Stream forever (paced) so the body is always in flight: the unfixed
// drain path keeps the connection open and actively reading, so the
// server never sees a close unless the cancel actually aborts.
async pull(controller) {
controller.enqueue(encoder.encode(`data: ${count++}\n\n`));
await sleep(10);
},
cancel() {
onCancel();
},
}),
{ headers: { "Content-Type": "text/event-stream" } },
);
},
});

const res = await fetch(`http://localhost:${server.port}`, { method: "POST" });
const reader = res.body!.getReader();

const first = await reader.read();
expect(first.done).toBe(false);
expect(first.value!.byteLength).toBeGreaterThan(0);

await reader.cancel();

// Cancelling the reader must abort the fetch and close the connection, so the
// server observes request.signal's abort and runs the body stream's cancel().
await Promise.all([sawAbort, sawCancel]);
});
Loading