fix(node/web): do not swallow getReader errors

[magne4000]

Fixes #198

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling in socket request body operations to ensure reader-acquisition errors are properly caught and emitted through error events instead of being propagated unexpectedly.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/srvx@199

commit: 29e0d01

[coderabbitai]

📝 Walkthrough

Walkthrough

A new private bodyReader() helper method was added to WebRequestSocket to properly handle errors when acquiring a stream reader. The _read() method now delegates reader acquisition through this helper, ensuring that errors from locked streams or reader initialization are properly routed to the error event instead of being silently swallowed.

Changes

Cohort / File(s)	Summary
WebRequestSocket Error Handling src/adapters/_node/web/socket.ts	Added bodyReader() helper method to safely acquire stream readers and emit errors. Updated _read() to use the helper, improving error propagation when reader acquisition fails (e.g., locked streams).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A reader once locked, now errors shine bright,
No more swallowed faults in the Node socket's night,
A helper so tiny, yet mighty and true,
Catches what breaks, and routes errors through!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly describes the main change: preventing getReader errors from being swallowed in WebRequestSocket, which is the core fix in the PR.
Linked Issues check	✅ Passed	The PR implements the fix for issue #198 by adding error handling for getReader failures through a bodyReader() helper that emits errors instead of suppressing them.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the getReader error suppression issue; no unrelated modifications or scope creep detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/adapters/_node/web/socket.ts`:
- Around line 102-108: In bodyReader(), don't call this.emit("error", error)
directly; call this.destroy(error) so the Duplex's _destroy(err, cb) override
handles teardown consistently—update the bodyReader method in the class
containing bodyReader() to invoke this.destroy(error) in the catch block (the
class already implements _destroy), ensuring stream state/teardown is handled
via the standard Node.js pattern rather than raw emit.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 47f67aca-1816-4276-83e0-8c18d74a9ae2

📥 Commits

Reviewing files that changed from the base of the PR and between a27fcbb and 29e0d01.

📒 Files selected for processing (1)

• src/adapters/_node/web/socket.ts

[pi0]

thnx!