-
Notifications
You must be signed in to change notification settings - Fork 51
WebSocket Timeout Fix #671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 9f938a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a WebSocket timeout issue that could cause uncaught exceptions by restructuring the connection flow to handle aborts more gracefully. The fix moves the stream creation earlier in the process and adds proper cleanup for WebSocket connections that are aborted before completion.
Key changes:
- Move
DataStream
creation before WebSocket connection attempt to enable early abort handling - Add listener to close WebSocket connections when stream is closed before RSocket connection completes
- Reorganize abort signal handling to occur before connection attempts
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/common/src/client/sync/stream/AbstractRemote.ts | Restructures WebSocket connection flow to prevent undefined stream access and enable connection abort |
.changeset/bright-yaks-pump.md | Documents the bug fix for WebSocket connection timeout exceptions |
Comments suppressed due to low confidence (1)
packages/common/src/client/sync/stream/AbstractRemote.ts:337
- The variable name 'disposeSocketConnectionTimeout' is misleading as it doesn't dispose a timeout but rather disposes a listener. Consider renaming to 'disposeStreamListener' or 'disposeConnectionListener' for clarity.
let disposeSocketConnectionTimeout = () => {};
Co-authored-by: Copilot <[email protected]>
Overview
This is a slightly alternative approach to #668. The fix here declares the
stream
early which should avoid the(TypeError: Cannot read property 'close' of undefined')
issue highlighted in the linked PR. Additionally the changes here allow the WebSocket initial connection request to be aborted without waiting for it to reject/complete.Testing
I've added a button to change the connection status to the React Supabase Todolist demo. I've also testing a slow connection attempt by running the PowerSync service locally with a paused attached debugger.