fix: bind Participant identity to the connection, not the token (#88) - #89
Conversation
The Room socket dropped every reconnect and refresh: Participant.tokenHash was @unique and recordJoin always create()d, but the client replays the stored Room Token on each (re)connect (F5, socket.io retry). The stale row blocked the next insert with P2002, so the gateway disconnected the socket and the client looped on "Reconnecting…" until it gave up. A Participant is one connection, not one token: - drop the unique index on tokenHash (kept as an audit column) - each connection inserts its own row (own id, already tracked per socket), so reconnects and two Receivers joining in the same second never collide - startup reconciliation marks rows left connected by an ungraceful shutdown as disconnected, clearing phantom receiver counts after a crash/redeploy Drops the now-dead findByTokenHash from the repository. Closes #88
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesParticipant presence lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RoomPresenceService
participant ParticipantRepository
participant ParticipantDatabase
RoomPresenceService->>ParticipantRepository: markAllDisconnected(new Date())
ParticipantRepository->>ParticipantDatabase: update connected participants
ParticipantDatabase-->>ParticipantRepository: updated row count
ParticipantRepository-->>RoomPresenceService: reconciled count
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/prisma/migrations/20260719000000_participant_tokenhash_drop_unique/migration.sql (1)
4-4: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid locking the table during index deletion.
A standard
DROP INDEXacquires anACCESS EXCLUSIVElock, which will block all read and write queries on theParticipanttable until the drop completes. For a live database, consider dropping the index concurrently to prevent downtime.Note that
DROP INDEX CONCURRENTLYcannot be executed inside a database transaction. In Prisma, you can instruct the migration engine not to wrap this specific script in a transaction by adding-- prisma DisableTransactionat the top of the file.💡 Proposed change
+-- prisma DisableTransaction -- DropIndex -- tokenHash is no longer a participant identity: each socket connection is its -- own Participant row, so the same token legitimately repeats across reconnects. -DROP INDEX "Participant_tokenHash_key"; +DROP INDEX CONCURRENTLY "Participant_tokenHash_key";🤖 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 `@apps/api/prisma/migrations/20260719000000_participant_tokenhash_drop_unique/migration.sql` at line 4, Update the migration statement dropping "Participant_tokenHash_key" to use concurrent index deletion, and add Prisma’s DisableTransaction directive at the top of the migration so it runs outside a transaction.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In
`@apps/api/prisma/migrations/20260719000000_participant_tokenhash_drop_unique/migration.sql`:
- Line 4: Update the migration statement dropping "Participant_tokenHash_key" to
use concurrent index deletion, and add Prisma’s DisableTransaction directive at
the top of the migration so it runs outside a transaction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c74a7bff-2575-41be-be4a-c05df66645b2
📒 Files selected for processing (7)
apps/api/prisma/migrations/20260719000000_participant_tokenhash_drop_unique/migration.sqlapps/api/prisma/schema.prismaapps/api/src/domain/participant/participant.repository.fake.tsapps/api/src/domain/participant/participant.repository.tsapps/api/src/infrastructure/persistence/repositories/prisma-participant.repository.tsapps/api/src/room/room-presence.service.spec.tsapps/api/src/room/room-presence.service.ts
|
Applied — switched to |
The Room socket dropped on every reconnect and refresh. The wss handshake completed (
101) and the token authenticated, but the server then immediately sent a socket.ioDISCONNECTand the client sat onReconnecting…until it exhausted its budget and readLost connection.Root cause
Participant.tokenHashwas@uniqueandRoomPresenceService.recordJoinalways calledparticipant.create(). The client stores the Room Token (roomSessionService.store) and replays the same token on every (re)connect — F5 replays the stored token and socket.io'sreconnection: truereplays it automatically.recordLeaveonly setsdisconnectedAt, never freeing the hash, so the stale row blocked the next insert with PrismaP2002. The design used the token as participant identity when it should have used the connection.This broke, in order of severity:
iat, no nonce) → same hash → the second couldn't connect.receiverCountafter a crash/redeploy leftdisconnectedAt = nullrows counted forever.Fix — a Participant is one connection, not one token
tokenHash(migration); keep the column as an audit trail of who joined.socket.idin the gateway), so reconnects and same-second joins can't collide.onApplicationBootstrap) marks rows left connected by an ungraceful shutdown as disconnected, clearing phantom counts. Assumes a single api instance — commented as such for when that changes.findByTokenHashfrom the repository.Verified
Local:
jest(235 pass, incl. new reconnect + startup-sweep specs onRoomPresenceService),eslint,tsc --noEmitall green. Migration is a plainDROP INDEX, applied by the existingprisma migrate deployon boot. Live socket reconnect against the deployed stack not yet exercised — verify after deploy by refreshing a Room and confirming the socket reconnects instead of looping.Closes #88
Summary by CodeRabbit
Bug Fixes
Tests