fix: stable WebSocket object identity across open/message/close hooks#1737
fix: stable WebSocket object identity across open/message/close hooks#1737MarcelOlsen wants to merge 2 commits intoelysiajs:mainfrom
Conversation
WalkthroughThe Bun adapter's WebSocket handling now creates and stores a single ElysiaWS wrapper per native socket on Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BunAdapter as Bun WS
participant ElysiaWS as Wrapper
Client->>BunAdapter: connect (open)
BunAdapter->>ElysiaWS: create if missing -> store on ws.data._elysiaWS
BunAdapter->>ElysiaWS: call open(handler) with same wrapper
Client->>BunAdapter: send message
BunAdapter->>ElysiaWS: set body, call message(handler) with same wrapper
Client->>BunAdapter: close
BunAdapter->>ElysiaWS: call close(handler) with same wrapper
BunAdapter-->>ElysiaWS: optional cleanup (remove from ws.data)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/adapter/bun/index.ts`:
- Around line 560-566: The TS2345 arises because storing new ElysiaWS(ws,
context as any) in elysiaWS loses the inline any-cast used previously when
calling options.open; to fix, keep the local elysiaWS but cast it back to any
when invoking options.open (e.g., pass options.open?.(elysiaWS as any)), or
alternatively cast at declaration (const elysiaWS = new ElysiaWS(ws, context as
any) as any); update the call site in the handleResponse invocation so
options.open receives the same any-typed value; relevant symbols: ElysiaWS,
elysiaWS, ws, options.open, handleResponse.
🧹 Nitpick comments (1)
src/adapter/bun/index.ts (1)
560-561: The_elysiaWSproperty onws.datais untyped.
ws.data._elysiaWSis set dynamically without any type declaration. This works becausewsis typed asServerWebSocket<any>, but it would be cleaner to add_elysiaWSto the data shape if a type exists for it. Not blocking, but worth noting for future maintainability.
commit: |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
Fixes WebSocket (ElysiaWS) object identity not being stable across open, message, drain, and close lifecycle hooks.
Previously, every handler created a new ElysiaWS(ws, context) wrapping the same raw
Bun ServerWebSocket, meaning ws in open !== ws in message !== ws in close. This broke identity-based patterns like storing sockets in a Set/Map or attaching per-socket state via properties.
Changes
src/adapter/bun/index.ts: Cache the ElysiaWS wrapper on ws.data._elysiaWS in the open handler and reuse it in message, drain, and close. Each handler falls back to creating a new instance (??=) as a defensive measure.test/ws/connection.test.ts: Added test asserting strict reference equality (===) of the ws object across open, message, and close.Test plan
bun test test/ws/)This resolves #1716
Summary by CodeRabbit