fix(realtime): forward an explicit send() timeout of 0 - #2647
fix(realtime): forward an explicit send() timeout of 0#2647MohammedAlkindi wants to merge 1 commit into
Conversation
send()'s websocket branch resolved the timeout with ||, so a caller passing timeout: 0 got the channel default instead. httpSend() and the REST fallback in the same method already use ??.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughSummary by CodeRabbit
Walkthrough
Merge Risk: ⚪ Minimal · up to The WebSocket send path now preserves an explicitly supplied timeout of 0, matching the existing HTTP paths. The change is localized and no actionable merge-blocking risk remains after normal checks and review. 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 |
🔍 Description
What changed?
send()resolves an explicitopts.timeoutwith??instead of||, matching the other two paths in the same method.Why was this change needed?
send()resolves the timeout in three places. Two use??:0is falsy, so the websocket branch replaced a caller'stimeout: 0with the channel default while the other two passed it through. The same call therefore behaved differently depending on which branch it took.git blameshows the||predates the Phoenix adapter refactor and no commit or discussion treatstimeout: 0as deliberately ignored, so this reads as a miss rather than a decision.🧪 Testing
npx vitest runinpackages/core/realtime-js: 474 passed, 2 failed, 1 skipped. Both failures are inRealtimeClient.worker.test.tsand fail identically withopts.timeout || this.timeoutrestored from master, so they are pre-existing.tsc --noEmitis clean. Reverting the change makes the added test fail on the pushed timeout argument.📋 Checklist