feat: add fromNodeUpgradeHandler util + socket.io example#185
Conversation
Wraps a Node.js `(req, socket, head)` upgrade handler (e.g. from `ws`, `socket.io`) as a crossws hooks object, delegating the socket to the underlying library while keeping crossws's upgrade-time routing. Adds a `handled` sentinel to `Hooks.upgrade`'s return so the node adapter bails out of its own `wss.handleUpgrade` when the hook has taken over the socket.
📝 WalkthroughWalkthroughAdds a Node.js upgrade adapter Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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 `@docs/2.adapters/node.md`:
- Line 68: The docs note for fromNodeUpgradeHandler is incomplete: update the
sentence to state that it requires both request.runtime.node.req and the upgrade
tuple runtime.node.upgrade.{socket, head} from the Node.js runtime (since
fromNodeUpgradeHandler reads request.runtime.node.{req, upgrade} and extracts
req plus upgrade.socket and upgrade.head); mention it only works on Node.js and
must be used via the crossws node server plugin so those runtime fields are
present.
🪄 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: 112acbd4-77ed-4ecf-8383-4b96b6f95977
📒 Files selected for processing (3)
docs/2.adapters/node.mdsrc/adapters/node.tssrc/node-handler.ts
✅ Files skipped from review due to trivial changes (1)
- src/node-handler.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/adapters/node.ts
fromNodeUpgradeHandler utilfromNodeUpgradeHandler util + socket.io example
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
resolves #138
fromNodeUpgradeHandler(handler)— wraps a Node.js(req, socket, head)upgrade handler (e.g. fromws,socket.io,express-ws) as a crossws hooks object, so it can be mounted viacrossws/server/nodewhile delegating the socket to the underlying library.req/socket/headout ofrequest.runtime.node.{req, upgrade}(exposed by srvx'sNodeRequestfrom src/server/node.ts:22) — no polyfill needed, we just hand the triple to the user's handler.handled?: booleansentinel toHooks.upgrade's return type. When the hook has taken ownership of the socket, the node adapter bails out beforewss.handleUpgradeso the same socket isn't upgraded twice. Other runtimes ignore the flag.crossws/adapters/nodesubpath (not the main entry), and documented in docs/2.adapters/node.md.The wrapped handler takes ownership of the socket, so crossws's other lifecycle hooks (
open/message/close/error) are not invoked for connections routed through it — the wrapped library manages the WebSocket lifecycle as usual.Example
Test plan
pnpm vitest run test/node-handler.test.ts— new regression tests pass.handledbailout in the node adapter and confirmed both tests fail hard (via anunhandledRejectionguard) withserver.handleUpgrade() was called more than once with the same socket.pnpm vitest run— full suite (117 passed, 6 skipped).pnpm typecheck.pnpm lint.Summary by CodeRabbit
New Features
Behavior Changes
Documentation
Examples
Tests
Chores