fix(node): soft error when response body is already locked

[huseeiin]

If the stream gets closed at some point (like if the browser is closed mid-stream) it can throw unhandled errors, which can crash NodeJS. ignoring these errors is completely fine and doesn't cause hidden consequences. its only a problem in the nodejs adapter and doesn't happen in deno/bun
i simply added a try catch to the whole streamBody function. the rest is just formatting differences

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling in streaming response processing to provide more robust failure recovery and prevent unexpected interruptions.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors error handling in the streamBody function by adding an outer try/catch for synchronous errors, introducing nested error handling inside streamHandle to route errors to streamCancel, and explicitly managing error paths while preserving existing stream reading and response writing logic.

Changes

Cohort / File(s)	Summary
Stream Error Handling Refactor src/adapters/_node/send.ts	Wraps entire streamBody implementation in try/catch to capture synchronous errors. Adds nested try block inside streamHandle to catch stream-related errors and route them to streamCancel with proper Error typing. Error paths now explicitly handled with outer catch swallowing unexpected errors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A stream caught fire, errors flew wild,
But we wrapped it tight with catch so styled,
Nested handlers dance, errors now tamed,
Cancellation flows where mistakes are claimed,
Our little fix makes the stream behave mild! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: wrapping streamBody in error handling to prevent crashes when a response body stream is already locked or closed.

_{✏️ 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@158

commit: c5d4899

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/adapters/_node/send.ts (1)

123-124: Consider narrowing the catch scope to avoid hiding unrelated bugs.

The empty catch block will swallow all synchronous errors, not just the intended locked stream errors. While this prevents crashes from mid-stream closures, it also masks potential bugs during development.

Consider filtering for the specific error types that are safe to ignore:

♻️ Suggested refinement

-    // eslint-disable-next-line no-empty
-  } catch {}
+  } catch (error) {
+    // Ignore errors from locked/closed streams (e.g., browser closed mid-stream)
+    // Re-throw unexpected errors to avoid hiding bugs
+    if (
+      error instanceof TypeError &&
+      error.message.includes("locked")
+    ) {
+      return;
+    }
+    throw error;
+  }

Alternatively, if all errors in this path are truly safe to ignore, at least add a brief comment explaining why:

-    // eslint-disable-next-line no-empty
-  } catch {}
+  } catch {
+    // Errors here are expected when client disconnects mid-stream; safe to ignore
+  }

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 83dba8e and 3be58c0.

📒 Files selected for processing (1)

• src/adapters/_node/send.ts

🧰 Additional context used

🧬 Code graph analysis (1)

src/adapters/_node/send.ts (1)

test/_fixture.ts (1)

• error (44-49)

🔇 Additional comments (3)

src/adapters/_node/send.ts (3)

73-88: LGTM!

The early destruction check and streamCancel function are well-structured. The guard prevents unnecessary work when the response is already destroyed, and the cleanup logic properly handles both the reader and response.

---

90-110: LGTM!

The nested try/catch properly routes synchronous errors to streamCancel, and the drain handling for backpressure is correctly implemented.

---

112-122: LGTM!

Event listeners are properly registered and cleaned up in finally, ensuring no memory leaks. The promise-based completion handling is correct.

[pi0]

Can you please add a test to current fixture? Also i think we could reduce scope of try {} catch {}, current change might ignore more than needed

[huseeiin]

closing until sufficient reproduction is found