feat: add PartySocket multi-region reconnect manager - #1369
Conversation
|
@Ayush-0918 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:
|
📝 WalkthroughWalkthroughAdds jittered PartySocket reconnect utilities and a ChangesPartySocket multi-region reconnection
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PartySocket
participant PartySocketReconnectManager
participant RegionEndpoint
PartySocket->>PartySocketReconnectManager: report disconnect
PartySocketReconnectManager->>PartySocketReconnectManager: increment retry count and wait with jittered backoff
PartySocketReconnectManager->>RegionEndpoint: send timed HEAD probes
RegionEndpoint-->>PartySocketReconnectManager: return latency or failure
PartySocketReconnectManager-->>PartySocket: provide best region or null
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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
🧹 Nitpick comments (3)
src/lib/partySocketReconnect.ts (1)
118-134: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider guarding against re-entrant
onDisconnectcalls.There's no in-flight flag, so if
onDisconnectwere invoked again before a prior call's delay/probe resolves (e.g., duplicate disconnect events), both calls would independently incrementretryCount, sleep, and probe concurrently, racing oncurrentRegion. Worth anisReconnectingguard if the caller can't guarantee serialized invocation.🤖 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 118 - 134, Guard PartySocketReconnect.onDisconnect against re-entrant calls by tracking whether a reconnect attempt is already in flight. Return without starting another retry while the guard is active, and reliably clear it after the delay and getBestRegion probe complete, including early-return paths, so retryCount and currentRegion are updated by only one concurrent attempt.src/__tests__/lib/partySocketReconnect.test.ts (2)
148-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest doesn't actually verify backoff delay growth despite its title.
jest.runAllTimers()fast-forwards through the pending timeout regardless of how long it actually is, so this never checks that the delay grows between the 1st/2nd/3rd disconnect (perreconnectionDelayGrowFactor). Consider asserting onsetTimeout's scheduled delay (e.g. spy onsetTimeoutand inspect the ms argument, or assert delay viajitteredReconnectDelaydirectly) to actually cover the backoff behavior this segment is meant to test.🤖 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/__tests__/lib/partySocketReconnect.test.ts` around lines 148 - 179, The test named “onDisconnect increments retryCount and delays before reconnect” only verifies completion, not backoff growth. Update it to observe each reconnect timeout’s scheduled delay—using the existing timeout scheduling or jitteredReconnectDelay path—and assert that the first three disconnects use progressively larger delays according to reconnectionDelayGrowFactor, while preserving the maxRetries assertion.
76-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInline comment about
Date.now()mocking is inaccurate, and the assertion is too weak to verify it.The comment states Jest fake timers "don't mock Date.now() by default unless configured," but modern fake timers (the default since Jest 27, and this project is on Jest 29.7.0) do fake
Dateby default. Given that,jest.advanceTimersByTime(50)before awaitingprobePromiseshould make the measured latency deterministic (50), so the test could assert that directly instead of the vacuoustypeof latency === "number"check, which would pass even for a completely broken timing calculation.♻️ Suggested tightening
- // Since we mocked fetch to resolve immediately but advanced timers, Date.now() will reflect the timer advancement if we mocked Date.now - // Actually jest fake timers don't mock Date.now() by default unless configured. - // We can just check that it returns a number. - expect(typeof latency).toBe("number"); + // Modern fake timers (default since Jest 27) fake Date too, so advancing + // timers before resolving the mocked fetch call makes latency deterministic. + expect(latency).toBe(50);Can you confirm Jest 29's modern fake timers fake
Date.now()by default in this project's config (nodoNotFake: ['Date']override)?🤖 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/__tests__/lib/partySocketReconnect.test.ts` around lines 76 - 96, Verify the Jest 29 configuration uses modern fake timers without a doNotFake Date override, then update the probeRegion test to assert the deterministic latency value of 50 after advancing timers. Remove the inaccurate Date.now() commentary and replace the weak typeof latency assertion with the exact expected value.
🤖 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/lib/partySocketReconnect.ts`:
- Around line 118-138: Update onDisconnect so retry exhaustion and temporary
absence of a healthy region produce distinguishable outcomes. Preserve the
existing maxRetries termination behavior, but replace the ambiguous bestRegion
null result with an explicit outcome indicating that another reconnect attempt
should remain possible; update the Promise return type and any callers of
onDisconnect to handle both states.
- Around line 78-99: Update probeRegion to use CORS mode and validate the fetch
response with response.ok before returning the latency, while preserving timeout
cleanup and Infinity for failures. If the endpoint cannot support CORS, document
within probeRegion that the probe intentionally measures reachability only
instead of treating HTTP errors as healthy.
---
Nitpick comments:
In `@src/__tests__/lib/partySocketReconnect.test.ts`:
- Around line 148-179: The test named “onDisconnect increments retryCount and
delays before reconnect” only verifies completion, not backoff growth. Update it
to observe each reconnect timeout’s scheduled delay—using the existing timeout
scheduling or jitteredReconnectDelay path—and assert that the first three
disconnects use progressively larger delays according to
reconnectionDelayGrowFactor, while preserving the maxRetries assertion.
- Around line 76-96: Verify the Jest 29 configuration uses modern fake timers
without a doNotFake Date override, then update the probeRegion test to assert
the deterministic latency value of 50 after advancing timers. Remove the
inaccurate Date.now() commentary and replace the weak typeof latency assertion
with the exact expected value.
In `@src/lib/partySocketReconnect.ts`:
- Around line 118-134: Guard PartySocketReconnect.onDisconnect against
re-entrant calls by tracking whether a reconnect attempt is already in flight.
Return without starting another retry while the guard is active, and reliably
clear it after the delay and getBestRegion probe complete, including
early-return paths, so retryCount and currentRegion are updated by only one
concurrent attempt.
🪄 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: 35210707-b5d6-4a97-8aa1-cebd2fb23e3d
📒 Files selected for processing (2)
src/__tests__/lib/partySocketReconnect.test.tssrc/lib/partySocketReconnect.ts
| async probeRegion(region: string): Promise<number> { | ||
| const controller = new AbortController(); | ||
| const timeout = setTimeout( | ||
| () => controller.abort(), | ||
| this.config.pingTimeoutMs, | ||
| ); | ||
| const start = Date.now(); | ||
| try { | ||
| const url = region.startsWith("http") ? region : `https://${region}`; | ||
| await fetch(url, { | ||
| method: "HEAD", | ||
| signal: controller.signal, | ||
| mode: "no-cors", | ||
| }); | ||
| return Date.now() - start; | ||
| } catch { | ||
| return Infinity; | ||
| } finally { | ||
| clearTimeout(timeout); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' src/lib/partySocketReconnect.tsRepository: SatyamPandey-07/WorkSphere
Length of output: 4111
🌐 Web query:
PartyKit edge endpoint CORS no-cors ping health check docs
💡 Result:
In PartyKit, CORS, ping/health checks, and endpoint handling are managed as follows: CORS Configuration PartyKit provides native CORS support for HTTP requests directed at your server [1][2]. - You can enable CORS by setting cors: true in your server options for permissive default headers, or pass a custom HeadersInit object (or cors configuration object with origin/headers) to specify restricted access [3][2]. - When enabled, PartyKit automatically handles preflight OPTIONS requests for matched routes and appends the appropriate CORS headers to non-WebSocket responses [1][4]. Health Checks and Ping There is no single built-in "ping" endpoint provided automatically by the PartyKit framework, but you can implement custom health check logic easily [5][6]: - HTTP Endpoints: You can define an onRequest or onFetch handler to respond to a specific path (e.g., /health) with a 200 OK status [5][7]. - Custom Logic: If your application requires deep health checks (such as verifying database connectivity), you can implement this logic within your onRequest handler and return appropriate status codes (e.g., 503 if a dependency is down) [6]. - WebSocket Pings: For persistent connections, applications often implement their own ping/pong mechanism within the onMessage handler to monitor connection health [8]. Note that some PartyKit internal features, such as those used for hibernation or internal keep-alive tasks, may utilize their own ping/pong mechanics internally [9][10]. General Endpoint Handling - Requests are handled via methods such as onRequest (for room-specific requests) or onFetch (for lightweight, global requests that do not require room state) [7]. - You can intercept or modify incoming requests before they reach the room using static onBeforeRequest or onBeforeConnect handlers, which run at the edge [7][11]. top_results: [1][3][2][5][7][6][8]
Citations:
- 1: Add CORS support to
routePartykitRequest. cloudflare/partykit#320 - 2: https://github.com/cloudflare/partykit/blob/main/packages/partyserver/src/index.ts
- 3: https://github.com/partykit/partykit/blob/main/packages/party.io/README.md
- 4: cloudflare/partykit@9bd3f56
- 5: https://docs.partykit.io/tutorials/add-partykit-to-a-nextjs-app/2-set-up-server/
- 6: https://www.hivebook.wiki/wiki/partykit-cloudflare-edge-realtime-sdk-with-durable-objects
- 7: https://docs.partykit.io/reference/partyserver-api/
- 8: https://github.com/dev-badace/party-socket-test
- 9: Add experimental_waitUntil API for long-running tasks cloudflare/partykit#296
- 10: https://blog.partykit.io/posts/partyserver-api/
- 11: https://github.com/cloudflare/partykit/blob/main/packages/partyserver/README.md
🏁 Script executed:
rg -n "probeRegion|pingTimeoutMs|nearest active edge region|PartyKit" src README.md docsRepository: SatyamPandey-07/WorkSphere
Length of output: 24530
🌐 Web query:
site:docs.partykit.io CORS PartyKit edge endpoint fetch no-cors
💡 Result:
PartyKit applications run on Cloudflare’s edge network, and the platform allows for standard HTTP and WebSocket communication with your PartyKit endpoints [1]. If you are encountering CORS-related issues when using fetch from a web browser to your PartyKit endpoint, it is important to note: 1. CORS is a browser-enforced security mechanism. PartyKit servers are regular HTTP servers, and you are responsible for handling CORS headers (such as Access-Control-Allow-Origin) in your onRequest or onFetch response objects if you need to support cross-origin requests from web clients [2][3]. 2. The no-cors mode in the browser fetch API intentionally restricts the request to be an opaque request. It is generally not suitable for communicating with an API endpoint like PartyKit where you expect to receive data (such as JSON) or need to set custom headers (like Authorization), as no-cors mode limits your ability to inspect the response or modify request headers [4][3]. To resolve CORS issues, you should configure your PartyKit server to include the necessary CORS headers in its response: return new Response(JSON.stringify({ success: true }), { status: 200, headers: { "Content-Type": "application/json", "Access-Control-Allow-Origin": "*", // Or specify your origin "Access-Control-Allow-Methods": "GET, POST, OPTIONS", "Access-Control-Allow-Headers": "Content-Type, Authorization" } }); Additionally, ensure your server properly handles the OPTIONS request (preflight), which browsers send automatically before making cross-origin requests [3]. If you are using no-cors mode specifically to bypass CORS preflight checks, please be aware that this will not allow your browser to read the response body from your PartyKit server. Instead, you should implement proper CORS support on the server side as shown above.
Citations:
- 1: https://docs.partykit.io/how-partykit-works/
- 2: https://docs.partykit.io/reference/partyserver-api/
- 3: https://docs.partykit.io/tutorials/add-partykit-to-a-nextjs-app/2-set-up-server/
- 4: https://docs.partykit.io/guides/authentication/
🏁 Script executed:
rg -n "cors|Access-Control-Allow-Origin|OPTIONS|onFetch|onRequest" party src/partykit.json partykit.jsonRepository: SatyamPandey-07/WorkSphere
Length of output: 226
🏁 Script executed:
git ls-files | rg '(^|/)(partykit\.json|server\.(ts|js)|multiRegionServer\.(ts|js)|party/|src/lib/partySocketReconnect\.ts|src/__tests__/lib/partySocketReconnect\.test\.ts|PARTYKIT_ARCHITECTURE\.md)$'Repository: SatyamPandey-07/WorkSphere
Length of output: 330
🏁 Script executed:
wc -l partykit.json party/server.ts party/multiRegionServer.ts src/__tests__/lib/partySocketReconnect.test.ts && \
printf '\n--- partykit.json ---\n' && cat -n partykit.json && \
printf '\n--- party/server.ts (1-220) ---\n' && sed -n '1,220p' party/server.ts && \
printf '\n--- party/multiRegionServer.ts (1-260) ---\n' && sed -n '1,260p' party/multiRegionServer.ts && \
printf '\n--- src/__tests__/lib/partySocketReconnect.test.ts (1-260) ---\n' && sed -n '1,260p' src/__tests__/lib/partySocketReconnect.test.tsRepository: SatyamPandey-07/WorkSphere
Length of output: 20757
🏁 Script executed:
wc -l partykit.json party/server.ts party/multiRegionServer.ts src/__tests__/lib/partySocketReconnect.test.ts && \
printf '\n--- partykit.json ---\n' && cat -n partykit.json && \
printf '\n--- party/server.ts (1-220) ---\n' && sed -n '1,220p' party/server.ts && \
printf '\n--- party/multiRegionServer.ts (1-260) ---\n' && sed -n '1,260p' party/multiRegionServer.ts && \
printf '\n--- src/__tests__/lib/partySocketReconnect.test.ts (1-260p) ---\n' && sed -n '1,260p' src/__tests__/lib/partySocketReconnect.test.tsRepository: SatyamPandey-07/WorkSphere
Length of output: 20758
🏁 Script executed:
wc -l partykit.json party/server.ts party/multiRegionServer.ts src/__tests__/lib/partySocketReconnect.test.ts && \
printf '\n--- partykit.json ---\n' && cat -n partykit.json && \
printf '\n--- party/server.ts (1-220) ---\n' && sed -n '1,220p' party/server.ts && \
printf '\n--- party/multiRegionServer.ts (1-260p) ---\n' && sed -n '1,260p' party/multiRegionServer.ts && \
printf '\n--- src/__tests__/lib/partySocketReconnect.test.ts (1-260p) ---\n' && sed -n '1,260p' src/__tests__/lib/partySocketReconnect.test.tsRepository: SatyamPandey-07/WorkSphere
Length of output: 20759
🏁 Script executed:
cat -n partykit.json && printf '\n--- party/server.ts ---\n' && sed -n '1,220p' party/server.ts && printf '\n--- party/multiRegionServer.ts ---\n' && sed -n '1,260p' party/multiRegionServer.ts && printf '\n--- src/__tests__/lib/partySocketReconnect.test.ts ---\n' && sed -n '1,260p' src/__tests__/lib/partySocketReconnect.test.tsRepository: SatyamPandey-07/WorkSphere
Length of output: 20570
probeRegion only measures reachability here, not HTTP health.
With mode: "no-cors", the response is opaque, so 4xx/5xx responses still look successful and can be treated as healthy candidates. If the PartyKit endpoint exposes CORS on this path, switch to mode: "cors" and check response.ok; otherwise document that this probe is reachability-only.
🤖 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 78 - 99, Update probeRegion to
use CORS mode and validate the fetch response with response.ok before returning
the latency, while preserving timeout cleanup and Infinity for failures. If the
endpoint cannot support CORS, document within probeRegion that the probe
intentionally measures reachability only instead of treating HTTP errors as
healthy.
| async onDisconnect(): Promise<string | null> { | ||
| this.retryCount++; | ||
| if (this.retryCount > this.config.maxRetries) { | ||
| return null; | ||
| } | ||
|
|
||
| const delay = jitteredReconnectDelay(this.retryCount, this.config); | ||
| if (delay > 0) { | ||
| await new Promise((resolve) => setTimeout(resolve, delay)); | ||
| } | ||
|
|
||
| const bestRegion = await this.getBestRegion(); | ||
| if (bestRegion) { | ||
| this.currentRegion = bestRegion; | ||
| } | ||
| return bestRegion; | ||
| } | ||
|
|
||
| onConnect(): void { | ||
| this.retryCount = 0; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
onDisconnect conflates two different failure states into the same null return.
return null at line 121 (retries exhausted — stop trying permanently) is indistinguishable from the null returned via bestRegion at line 133 (no healthy region right now — should still retry on the next disconnect). A caller wiring this into actual reconnection logic has no way to tell "give up" from "wait and try again," which matters for correctly driving a reconnect state machine.
♻️ Suggested fix: make the two outcomes distinguishable
- async onDisconnect(): Promise<string | null> {
+ async onDisconnect(): Promise<{ region: string | null; exhausted: boolean }> {
this.retryCount++;
if (this.retryCount > this.config.maxRetries) {
- return null;
+ return { region: null, exhausted: true };
}
const delay = jitteredReconnectDelay(this.retryCount, this.config);
if (delay > 0) {
await new Promise((resolve) => setTimeout(resolve, delay));
}
const bestRegion = await this.getBestRegion();
if (bestRegion) {
this.currentRegion = bestRegion;
}
- return bestRegion;
+ return { region: bestRegion, exhausted: false };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async onDisconnect(): Promise<string | null> { | |
| this.retryCount++; | |
| if (this.retryCount > this.config.maxRetries) { | |
| return null; | |
| } | |
| const delay = jitteredReconnectDelay(this.retryCount, this.config); | |
| if (delay > 0) { | |
| await new Promise((resolve) => setTimeout(resolve, delay)); | |
| } | |
| const bestRegion = await this.getBestRegion(); | |
| if (bestRegion) { | |
| this.currentRegion = bestRegion; | |
| } | |
| return bestRegion; | |
| } | |
| onConnect(): void { | |
| this.retryCount = 0; | |
| } | |
| async onDisconnect(): Promise<{ region: string | null; exhausted: boolean }> { | |
| this.retryCount++; | |
| if (this.retryCount > this.config.maxRetries) { | |
| return { region: null, exhausted: true }; | |
| } | |
| const delay = jitteredReconnectDelay(this.retryCount, this.config); | |
| if (delay > 0) { | |
| await new Promise((resolve) => setTimeout(resolve, delay)); | |
| } | |
| const bestRegion = await this.getBestRegion(); | |
| if (bestRegion) { | |
| this.currentRegion = bestRegion; | |
| } | |
| return { region: bestRegion, exhausted: false }; | |
| } | |
| onConnect(): void { | |
| this.retryCount = 0; | |
| } |
🤖 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 118 - 138, Update onDisconnect
so retry exhaustion and temporary absence of a healthy region produce
distinguishable outcomes. Preserve the existing maxRetries termination behavior,
but replace the ambiguous bestRegion null result with an explicit outcome
indicating that another reconnect attempt should remain possible; update the
Promise return type and any callers of onDisconnect to handle both states.
2 participants
Description
This PR implements a reusable
PartySocketReconnectManagerto improve the reliability of real-time PartySocket connections across multiple PartyKit edge regions.Changes
PartySocketReconnectManagerto manage reconnection lifecycle.Related Issue
Checklist
Screenshots / Screen Recordings (if applicable)
Not applicable (backend/infrastructure feature).
Breaking Changes
Summary by CodeRabbit