fix(node): call request abort controller only on close

[pi0]

resolves #160

---

req.body initiates Node Readable.toWeb. When the read is canceled, it emits an error event on the original request for "Operation canceled," which can be wrongly interpreted as if the original client is disconnected and triggers the req abort controller.

This PR removes the additional error event listener from the node event and relies on the res close event to trigger the abort controller.

This makes behavior consistent with Bun and Deno.

Summary by CodeRabbit

• Bug Fixes

  • Improved request abort handling by refining error event management in the request lifecycle.

• Tests

  • Added test coverage for request body cancellation scenarios to ensure reliability during client disconnects.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR fixes a bug where req.signal.aborted was incorrectly becoming true when canceling body reads in the Node.js adapter. It removes the "error" event listener from the abort controller, adds a test route to reproduce the issue, and includes a test case to verify the fix.

Changes

Cohort / File(s)	Summary
Abort controller fix src/adapters/_node/request.ts	Removed "error" event listener registration from req in the _abortController getter, decoupling req error events from triggering abort logic.
Test coverage test/_fixture.ts, test/_tests.ts, test/node.test.ts	Added "/body-cancel" route handler to test fixture that consumes request body, cancels the read, and reports abort signal state. Added corresponding test case "cancel reading body" to verify both abortedAfterCancel and abortedAfterTimeout are false. Updated server teardown to call close(true).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related issues

• Node adapter aborts requests after the socket closes, triggering Undici “Response body … disturbed” crash #142 — Directly addresses abort-controller lifecycle concerns regarding req error event handling and unwanted abort triggers within the Node adapter's lazy-instantiated abort controller.

Possibly related PRs

• fix(node): don't fire abort signal on normal request completion #153 — Also modifies abort-handling logic in src/adapters/_node/request.ts, shifting abort triggers from req event listeners to res.on("close") handling.

Poem

🐰 A signal wrongly raised in haste,
When bodies canceled—errors faced,
I've plucked the listener causing strain,
Now read.cancel() won't bring the pain! 🎯✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(node): call request abort controller only on close' accurately describes the main change: removing the error listener and relying on response close events.
Linked Issues check	✅ Passed	The PR fully addresses issue #160 by removing the problematic 'error' event listener on req that was causing req.signal.aborted to become true when body reads were canceled.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue #160: removing the error listener, adding test cases, and adjusting test teardown to ensure deterministic behavior.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/h3js/srvx@161

commit: 4541660

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@test/_fixture.ts`:
- Around line 207-218: The test route handling "/body-cancel" currently
hardcodes abortedAfterCancel to false; change it to read the real abort signal
state by assigning abortedAfterCancel = req.signal.aborted (or evaluating
req.signal.aborted immediately after await reader.cancel()) so the handler
returns the actual signal state; update the variable in the block that declares
abortedAfterCancel and ensure the subsequent abortedAfterTimeout still reads
req.signal.aborted after the timeout.

🧹 Nitpick comments (1)

test/_tests.ts (1)

161-179: Make the request body stream finite to avoid flaky hangs.
The stream never closes, which can keep the request body open and cause nondeterministic behavior in fetch/undici. Consider closing after the first chunk.

♻️ Proposed change

-      body: new ReadableStream({
-        async pull(controller) {
-          await new Promise((resolve) => setTimeout(resolve, 100));
-          controller.enqueue(new TextEncoder().encode("hello"));
-        },
-      }),
+      body: new ReadableStream({
+        async start(controller) {
+          await new Promise((resolve) => setTimeout(resolve, 100));
+          controller.enqueue(new TextEncoder().encode("hello"));
+          controller.close();
+        },
+      }),