Repository navigation
fix(socket): prevent duplicate connections on rapid offline-online toggles (fixes #1390) - #1410
Conversation
|
@renoschubert is attempting to deploy a commit to the pandeysatyam1802-gmailcom's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 Thank you for contributing to WorkSphere! Please ensure: ✅ Tests pass 💡 Connect & Support:
|
📝 WalkthroughWalkthroughPartySocket reconnect handling now tracks connection states, prevents duplicate connections, and cancels pending reconnect timers. Chat message actions update speech-synthesis wiring, read-aloud component placement, and copy-button visibility styling. ChangesPartySocket reconnect lifecycle
Chat message actions
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/chat/ChatMessages.tsx (1)
1176-1176: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExpose the action controls when keyboard-focused.
The ancestor at Line 1013 remains
opacity-0unless hovered, so the child’sfocus-within:opacity-100cannot make it visible while tabbing. Add a focus-within visibility rule to the ancestor and retain a visible focus indicator.Proposed fix
- className="absolute top-2 right-2 flex items-center gap-1.5 opacity-0 group-hover:opacity-100 transition-opacity z-10" + className="absolute top-2 right-2 flex items-center gap-1.5 opacity-0 group-hover:opacity-100 focus-within:opacity-100 transition-opacity z-10"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/chat/ChatMessages.tsx` at line 1176, Update the action-controls ancestor near the message controls container to include a focus-within opacity rule, allowing keyboard focus to reveal the controls even when the group is not hovered. Preserve the existing hover visibility behavior and retain an appropriate visible focus indicator on the focused control.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/chat/ChatMessages.tsx`:
- Line 933: Update the speech state flow around MessageList and ReadAloudButton
so both use the same useSpeechSynthesis instance. Lift the hook to their nearest
shared parent and pass its state and speech callbacks down, ensuring playback
started by ReadAloudButton updates speakingMessageId and speakingSentenceIndex
used by MessageList.
In `@src/lib/partySocketReconnect.ts`:
- Around line 75-96: Update the _wait and _clearTimeouts overrides in
src/lib/partySocketReconnect.ts:75-96 so cancelling the pending timer also
resolves/releases the promise awaited by the reconnect flow, allowing stale
_connect work to exit and future reconnects to proceed. Add coverage in
src/__tests__/lib/partySocketReconnect.test.ts:107-137 for
cancel-then-reconnect, asserting that a fresh connection can start.
---
Outside diff comments:
In `@src/components/chat/ChatMessages.tsx`:
- Line 1176: Update the action-controls ancestor near the message controls
container to include a focus-within opacity rule, allowing keyboard focus to
reveal the controls even when the group is not hovered. Preserve the existing
hover visibility behavior and retain an appropriate visible focus indicator on
the focused control.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6245a4bb-bb33-4742-86e7-4e54e55a0bbb
📒 Files selected for processing (3)
src/__tests__/lib/partySocketReconnect.test.tssrc/components/chat/ChatMessages.tsxsrc/lib/partySocketReconnect.ts
| speakMessage, | ||
| stopSpeech, | ||
| } = useSpeechSynthesis(); | ||
| const { speakingMessageId, speakingSentenceIndex } = useSpeechSynthesis(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C 4 '\b(ReadAloudButton|useSpeechSynthesis|speakingMessageId|speakingSentenceIndex)\b' srcRepository: SatyamPandey-07/WorkSphere
Length of output: 29750
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/hooks/useSpeechSynthesis.ts --view expanded || true
printf '\n--- ChatMessages excerpt ---\n'
sed -n '920,1040p' src/components/chat/ChatMessages.tsx
printf '\n--- ReadAloudButton excerpt ---\n'
sed -n '1,180p' src/components/chat/ReadAloudButton.tsxRepository: SatyamPandey-07/WorkSphere
Length of output: 10219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,380p' src/hooks/useSpeechSynthesis.tsRepository: SatyamPandey-07/WorkSphere
Length of output: 10132
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('src/hooks/useSpeechSynthesis.ts')
text = p.read_text()
for needle in ['speakMessage', 'setSpeakingMessageId', 'setSpeakingSentenceIndex', 'cancel', 'stopSpeech', 'onboundary', 'onend']:
idx = text.find(needle)
if idx != -1:
start = text.rfind('\n', 0, idx-200)
end = text.find('\n', idx+500)
print(f'=== {needle} ===')
print(text[max(0, idx-200):min(len(text), idx+700)])
print()
PYRepository: SatyamPandey-07/WorkSphere
Length of output: 5707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'createContext|useContext|Provider|Context|module|let speaking|const speaking|export .*Context' src/hooks/useSpeechSynthesis.ts src/components/chat/ReadAloudButton.tsx src/components/chat/ChatMessages.tsxRepository: SatyamPandey-07/WorkSphere
Length of output: 1852
Share the speech state with ReadAloudButton. ReadAloudButton owns a separate useSpeechSynthesis() instance and only calls speak(text), so speakingMessageId / speakingSentenceIndex in MessageList never reflect playback started from the button. Lift the hook to a shared parent or pass the state/callbacks down.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/chat/ChatMessages.tsx` at line 933, Update the speech state
flow around MessageList and ReadAloudButton so both use the same
useSpeechSynthesis instance. Lift the hook to their nearest shared parent and
pass its state and speech callbacks down, ensuring playback started by
ReadAloudButton updates speakingMessageId and speakingSentenceIndex used by
MessageList.
| s._wait = function (this: any) { | ||
| if (pendingTimeoutId) { | ||
| clearTimeout(pendingTimeoutId); | ||
| } | ||
| return new Promise<void>((resolve) => { | ||
| pendingTimeoutId = setTimeout(() => { | ||
| pendingTimeoutId = null; | ||
| resolve(); | ||
| }, this._getNextDelay()); | ||
| }); | ||
| }; | ||
|
|
||
| const originalClearTimeouts = s._clearTimeouts; | ||
| s._clearTimeouts = function (this: any) { | ||
| if (pendingTimeoutId) { | ||
| clearTimeout(pendingTimeoutId); | ||
| pendingTimeoutId = null; | ||
| } | ||
| if (originalClearTimeouts) { | ||
| originalClearTimeouts.call(this); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the locked PartySocket version to verify its _wait/_connect-lock contract.
fd -a '^(package.json|pnpm-lock.yaml|yarn.lock|package-lock.json)$' . -0 |
xargs -0 -r rg -n -C2 'partysocket'Repository: SatyamPandey-07/WorkSphere
Length of output: 1433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/lib/partySocketReconnect.ts --view expanded
printf '\n---\n'
cat -n src/lib/partySocketReconnect.ts | sed -n '1,220p'
printf '\n---\n'
ast-grep outline src/__tests__/lib/partySocketReconnect.test.ts --view expanded
printf '\n---\n'
cat -n src/__tests__/lib/partySocketReconnect.test.ts | sed -n '1,220p'Repository: SatyamPandey-07/WorkSphere
Length of output: 17575
🌐 Web query:
PartySocket 1.3.0 _wait _clearTimeouts reconnect lock internal connect loop source map
💡 Result:
The query components _wait, _clearTimeouts, reconnect, lock, internal connect loop, and source map refer to the internal mechanics and recent updates of PartySocket version 1.3.0 [1][2]. PartySocket (part of the Cloudflare PartyKit ecosystem) manages WebSocket connections, including automatic reconnection logic [3]. Key technical context for these terms in version 1.3.0 includes: Internal Connect Lock (_connectLock) In previous versions, a known issue caused the _connectLock to remain true if maxRetries were exhausted [4][5]. When the connection lock was stuck, calling reconnect() would silently fail because the library incorrectly perceived an ongoing connection attempt [4][5]. Version 1.3.0 includes fixes ensuring this lock is correctly released, allowing manual reconnection even after automatic retry limits are hit [5]. Connect Loop and Reconnection The internal connection logic (often involving private methods like _connect) manages the retry lifecycle [4][5]. Recent updates, including version 1.3.0, have refined how this loop handles terminal states [1][6]. Specifically, changes were made to the shouldReconnectOnClose predicate, allowing developers to classify certain close events (like authentication failures) as terminal, thereby stopping the automatic reconnect loop [1][6]. Synchronous Lifecycle and Buffering Version 1.3.0 introduced changes to make close() dispatch its close event synchronously [1][7]. This prevents issues where consumers detaching listeners during a socket swap might miss terminal close events [1][7]. Additionally, the library now includes logic to handle message buffering and draining during socket replacements (via methods like drainQueuedMessages), and it emits warnings when send() is called on a permanently closed socket [1][7]. Source Maps The reference to "source map" concerns the distributed files of the partysocket package [2][3]. As seen in the package distribution, PartySocket 1.3.0 ships with corresponding .map files (e.g., ws.js.map, index.js.map) for its bundled files [2][3]. These allow developers to debug the minified or bundled library code during development by mapping it back to the original TypeScript source [2][3][8]. In summary, these terms describe the robust handling of WebSocket state, the resolution of internal lock-up bugs [5], and the improved reliability of connection lifecycle events in PartySocket 1.3.0 [1][7].
Citations:
- 1: https://npmx.dev/package-changelog/partysocket/v/1.1.18
- 2: https://npmx.dev/diff/partysocket/v/1.1.18...1.3.0
- 3: https://registry.npmjs.org/partysocket
- 4: partysocket: Unable to force
.reconnect()after failed connection cloudflare/partykit#252 - 5: Fix:
reconnect()not working aftermaxRetriesexhausted cloudflare/partykit#322 - 6: Add partysocket terminal close reconnect predicate cloudflare/partykit#409
- 7: fix(partysocket): reliable buffered messages and close events across socket replacement cloudflare/partykit#403
- 8: https://app.unpkg.com/partysocket@1.1.16/files/dist/index.cjs.map
🌐 Web query:
PartySocket _wait _clearTimeouts _connectLock source code 1.3.0
💡 Result:
In PartySocket version 1.3.0, the _wait, _clearTimeouts, and _connectLock properties are internal members of the ReconnectingWebSocket class [1]. _connectLock is a boolean flag used to manage connection attempts, ensuring that the library does not initiate multiple simultaneous connection sequences [2][3]. The properties _wait and _clearTimeouts are associated with managing the reconnection lifecycle, including handling delays and timeouts [4][5]. Specifically, a notable interaction involving these members was addressed in updates leading up to 1.3.0. A bug existed where the _connectLock would remain set to true (preventing future reconnect() calls) if the maximum number of retries (maxRetries) was reached [6][1]. This was resolved by explicitly setting this._connectLock = false within the early return block that handles the exhaustion of retries [1]: if (this._retryCount >= maxRetries) { this._debug("max retries reached", this._retryCount, ">=", maxRetries); this._connectLock = false; // Fix: Ensure the lock is released return; } These internal mechanisms are part of the broader logic in the ReconnectingWebSocket class, which handles WebSocket resilience, such as buffering messages and managing connection state [4][7].
Citations:
- 1: cloudflare/partykit@3ed3bf0
- 2: partykit/partykit@5e1f03d
- 3: https://socket.dev/npm/package/partysocket/diff/0.0.0-b5a4202
- 4: https://github.com/cloudflare/partykit/blob/main/packages/partysocket/README.md
- 5: https://docs.partykit.io/reference/partysocket-api/
- 6: partysocket: Unable to force
.reconnect()after failed connection cloudflare/partykit#252 - 7: https://github.com/cloudflare/partykit
🌐 Web query:
site:github.com/partykit/partykit ReconnectingWebSocket _wait _clearTimeouts _connectLock await
💡 Result:
The terms _wait, _clearTimeouts, and _connectLock are internal private methods and properties used within the ReconnectingWebSocket implementation (specifically the version used in the PartyKit PartySocket library) to manage connection state and flow control [1][2]. The ReconnectingWebSocket class uses these to safely manage the lifecycle of WebSocket connection attempts [1]: _connectLock: A boolean flag used to prevent concurrent connection attempts [1]. When a connection process begins, it is set to true; if another attempt is triggered while the lock is held, the function exits early to avoid multiple simultaneous connections [1]. _wait: An asynchronous method that returns a promise, which resolves after a specific delay (determined by the reconnection strategy) [1]. This is typically used in the connection flow to pause before attempting to reconnect, ensuring the system does not overwhelm the server with rapid connection requests [1]. _clearTimeouts: A method that cleans up any active timers (such as those used for reconnection delays or uptime timeouts) [1]. It is essential for preventing memory leaks and ensuring that old, scheduled reconnection attempts do not trigger after the socket has been closed or reconnected [1]. These methods work together in the _connect workflow: it checks the _connectLock, uses _wait to handle the delay, and relies on _clearTimeouts to reset state during lifecycle transitions [1]. The use of await with these methods allows the class to manage complex asynchronous state changes in a more readable and reliable manner [1].
Citations:
🌐 Web query:
ReconnectingWebSocket _connectLock await this._wait() clearTimeouts Partysocket source code
💡 Result:
The mention of ReconnectingWebSocket, _connectLock, and _wait in the context of PartySocket refers to the internal implementation details of the ReconnectingWebSocket class, which PartySocket extends [1][2][3]. PartySocket is a fork of the reconnecting-websocket library, maintained as part of the PartyKit project [4][1][5]. Regarding the specific symbols you mentioned: _connectLock: This is an internal boolean property used to prevent multiple concurrent connection attempts [6][7]. A known issue (tracked as #252 in the PartyKit repository) involved _connectLock remaining set to true when the maximum number of retries (maxRetries) was reached [6][7]. This effectively prevented subsequent manual calls to.reconnect from initiating a new connection [6][7]. This was resolved by explicitly setting this._connectLock = false in the early return path within the _connect method [6][7]. _wait: This is an internal asynchronous helper method typically used to manage delays (such as exponential backoff) before attempting a reconnection [6]. The PartySocket library includes these mechanisms to ensure robust WebSocket management, such as handling connection timeouts, buffering messages during offline periods, and allowing dynamic URL updates [4][1][8]. Top results: [6][7][1]
Citations:
- 1: https://github.com/cloudflare/partykit/blob/main/packages/partysocket/README.md
- 2: https://www.npmjs.com/package/partysocket
- 3: https://cdn.jsdelivr.net/npm/partysocket@1.1.5/index.d.ts
- 4: https://github.com/cloudflare/partykit/tree/main/packages/partysocket
- 5: partysocket: fork
reconnecting-websocketpartykit/partykit#21 - 6: cloudflare/partykit@3ed3bf0
- 7: Fix:
reconnect()not working aftermaxRetriesexhausted cloudflare/partykit#322 - 8: https://docs.partykit.io/reference/partysocket-api/
🌐 Web query:
partysocket source _connectLock _wait _clearTimeouts _connect method code
💡 Result:
In the PartySocket library (a fork of reconnecting-websocket), the methods and properties you inquired about are core components of the connection management and retry logic [1][2][3]. _connectLock This is a private boolean flag used to prevent concurrent or overlapping connection attempts [1]. When a connection process begins, _connectLock is set to true [1][4]. It must be released (set back to false) once the connection is successful, if it fails, or when a terminal condition is met, to allow future connection attempts via.reconnect or automatic retries [1][5][6]. A known issue in earlier versions involved this lock becoming permanently stuck as true if maxRetries was exhausted, which prevented manual reconnections [7][6]. This was patched by explicitly releasing the lock in the maxRetries guard [5][6]. _connect This private method initiates the WebSocket connection [1]. It first checks if _connectLock is true or if automatic reconnection is disabled (via _shouldReconnect) [1]. If either is true, it returns early [1]. Otherwise, it acquires the lock (_connectLock = true), waits for the configured delay (using _wait), resolves the URL/protocols, and attempts to create the new WebSocket instance [1]. _wait This private method returns a Promise that resolves after a delay [1]. It is used to throttle connection attempts, typically calling _getNextDelay to determine the wait duration based on the configured reconnection strategy (e.g., exponential backoff) [1]. _clearTimeouts This private method is responsible for clearing all pending timers associated with reconnection attempts or connection timeouts [1]. It ensures that no stale connection processes or delayed reconnection triggers remain active, such as when the socket is closed manually or by the user [1]. These mechanisms collectively ensure that PartySocket maintains a stable, controlled reconnection state machine, preventing race conditions during connection transitions [1][6].
Citations:
- 1: partykit/partykit@f01ad2b
- 2: https://github.com/cloudflare/partykit/tree/main/packages/partysocket
- 3: https://github.com/cloudflare/partykit/blob/main/packages/partysocket/README.md
- 4: partykit/partykit@5e1f03d
- 5: cloudflare/partykit@3ed3bf0
- 6: Fix:
reconnect()not working aftermaxRetriesexhausted cloudflare/partykit#322 - 7: partysocket: Unable to force
.reconnect()after failed connection cloudflare/partykit#252
Don’t clear _wait()’s timer without also releasing the pending reconnect.
_wait() is awaited in PartySocket’s reconnect flow, so cancelling the timeout leaves the in-flight _connect() suspended and can block later reconnect attempts instead of just aborting the stale one.
src/lib/partySocketReconnect.ts#L75-L96: use a cancellation path that resolves/releases the pending wait, or avoid awaiting an uncancelled promise here.src/__tests__/lib/partySocketReconnect.test.ts#L107-L137: cover cancel-then-reconnect and assert a fresh connection can still start.
📍 Affects 2 files
src/lib/partySocketReconnect.ts#L75-L96(this comment)src/__tests__/lib/partySocketReconnect.test.ts#L107-L137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/partySocketReconnect.ts` around lines 75 - 96, Update the _wait and
_clearTimeouts overrides in src/lib/partySocketReconnect.ts:75-96 so cancelling
the pending timer also resolves/releases the promise awaited by the reconnect
flow, allowing stale _connect work to exit and future reconnects to proceed. Add
coverage in src/__tests__/lib/partySocketReconnect.test.ts:107-137 for
cancel-then-reconnect, asserting that a fresh connection can start.
2 participants
Title
fix(socket): prevent duplicate connections on rapid offline-online toggles (fixes #1390)
Description
This PR resolves a race condition where multiple duplicate WebSocket connections were being established under flaky network connectivity when the connection toggled rapidly between online and offline.
Key Changes
Related Issue
Summary by CodeRabbit
Bug Fixes
Tests