Conversation
✅ Deploy Preview for hoppdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR implements call de-duplication using Redis to prevent simultaneous duplicate call initiation attempts during a ringing window. The backend acquires a symmetric SETNX key on call initiation, rejects subsequent attempts with an "already-calling" reason, and releases the slot on acceptance/rejection. The frontend throttles call actions and displays specific user feedback for de-duped rejections. ChangesCall De-duplication Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. 🔧 golangci-lint (2.12.1)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/internal/handlers/websocketHandlers.go (1)
360-377:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRelease the dedupe slot on caller cancellation too.
The new cleanup only happens in
acceptCallandrejectCall, but the caller-side cancel flow sendscall_end. After a user aborts a ringing call,call:pending:*stays alive until TTL expiry, so an immediate retry to the same participant is incorrectly blocked.Suggested fix
- endCall(c, server, *parsedMessage.CallEnd) + endCall(c, server, user.ID, *parsedMessage.CallEnd)-func endCall(ctx echo.Context, s *common.ServerState, message messages.CallEndMessage) { +func endCall(ctx echo.Context, s *common.ServerState, senderID string, message messages.CallEndMessage) { + s.Redis.Del(context.Background(), dedupeCallKey(senderID, message.Payload.ParticipantID)) + // Publish a message to the other participant payloadJSON, err := json.Marshal(message) if err != nil { ctx.Logger().Error(err) return }🤖 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 `@backend/internal/handlers/websocketHandlers.go` around lines 360 - 377, The dedupe key is only removed in acceptCall and rejectCall, but not when the caller cancels (call_end), leaving call:pending:* until TTL expires; update the call-end handling branch that processes messages of type call_end (the function or switch branch that handles messages.CallEndMessage / "call_end") to also call s.Redis.Del(context.Background(), dedupeCallKey(message.Payload.CallerID, <otherParticipantID>)) so the pending dedupe slot is released on caller cancellation; reuse the same dedupeCallKey helper as used in acceptCall/rejectCall and ensure you pass the correct counterpart ID (caller or callee depending on the payload) before publishing or returning.
🤖 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 `@backend/internal/handlers/websocketHandlers.go`:
- Around line 275-278: The dedupe TTL (callDedupeTTL) is shorter than the
ringing window (CallBanner) so duplicate call requests can slip through; update
the TTL used in the SetNX call to be at least the ringing duration (e.g., 60s)
or reference the existing ringing-window constant used by CallBanner (use that
symbolic constant instead of 30s) so the dedupe lock (dedupeKey) remains set for
the full outbound/inbound ringing period when calling s.Redis.SetNX.
In `@tauri/src/components/ui/call-banner.tsx`:
- Around line 104-114: The timeout currently calls handleReject()
unconditionally if tokensReceived is false, which can send a call_reject after
an accept is already in flight; modify the logic to guard against that by
checking a call-accept state before rejecting: introduce or use an existing
boolean (e.g. tokensReceived and a new callAccepted flag or existing acceptCall
flow indicator), only call handleReject() when tokensReceived is false AND
callAccepted is false, and ensure you clear the timeout or set callAccepted when
acceptCall() starts so the timeout cannot race; still always remove the
"call_tokens_callback" handler on timeout/cleanup but do not emit a reject after
an accept is initiated.
- Around line 73-84: The call-accept path calls tauriUtils.getUserSettings()
without protection so a thrown error leaves tokensReceived true and the accept
timeout/toast stuck; wrap the settings lookup in a try/catch and fall back to a
safe default settings object (at least providing start_mic_on_call and
start_camera_on_call) before calling setCallTokens, and ensure any error does
not short-circuit the existing flow that clears the accept timeout/toast; update
the code around setCallTokens / tokensReceived handling to use the
caught-or-default settings and proceed normally.
---
Outside diff comments:
In `@backend/internal/handlers/websocketHandlers.go`:
- Around line 360-377: The dedupe key is only removed in acceptCall and
rejectCall, but not when the caller cancels (call_end), leaving call:pending:*
until TTL expires; update the call-end handling branch that processes messages
of type call_end (the function or switch branch that handles
messages.CallEndMessage / "call_end") to also call
s.Redis.Del(context.Background(), dedupeCallKey(message.Payload.CallerID,
<otherParticipantID>)) so the pending dedupe slot is released on caller
cancellation; reuse the same dedupeCallKey helper as used in
acceptCall/rejectCall and ensure you pass the correct counterpart ID (caller or
callee depending on the payload) before publishing or returning.
🪄 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
Run ID: aed72e76-5182-43c0-9859-e3495a8cd73c
📒 Files selected for processing (5)
backend/internal/handlers/websocketHandlers.gobackend/internal/messages/messages.gotauri/src/components/ui/call-banner.tsxtauri/src/components/ui/participant-row-wo-livekit.tsxtauri/src/payloads.ts
| // Dedupe: prevent duplicate/glare call requests within ringing window. | ||
| // Symmetric key: pending A->B also blocks B->A. | ||
| const callDedupeTTL = 30 * time.Second | ||
| acquired, err := s.Redis.SetNX(rdbCtx, dedupeKey, "1", callDedupeTTL).Result() |
There was a problem hiding this comment.
Keep the dedupe TTL at least as long as the ringing window.
CallBanner keeps the incoming call active for 60 seconds, but this Redis slot expires after 30 seconds. That means a second call_request can reacquire the key while the first banner is still ringing, which reopens the duplicate-call path this PR is trying to close.
Suggested fix
- const callDedupeTTL = 30 * time.Second
+ // Keep this aligned with the maximum ringing window.
+ const callDedupeTTL = 60 * time.Second📝 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.
| // Dedupe: prevent duplicate/glare call requests within ringing window. | |
| // Symmetric key: pending A->B also blocks B->A. | |
| const callDedupeTTL = 30 * time.Second | |
| acquired, err := s.Redis.SetNX(rdbCtx, dedupeKey, "1", callDedupeTTL).Result() | |
| // Dedupe: prevent duplicate/glare call requests within ringing window. | |
| // Symmetric key: pending A->B also blocks B->A. | |
| // Keep this aligned with the maximum ringing window. | |
| const callDedupeTTL = 60 * time.Second | |
| acquired, err := s.Redis.SetNX(rdbCtx, dedupeKey, "1", callDedupeTTL).Result() |
🤖 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 `@backend/internal/handlers/websocketHandlers.go` around lines 275 - 278, The
dedupe TTL (callDedupeTTL) is shorter than the ringing window (CallBanner) so
duplicate call requests can slip through; update the TTL used in the SetNX call
to be at least the ringing duration (e.g., 60s) or reference the existing
ringing-window constant used by CallBanner (use that symbolic constant instead
of 30s) so the dedupe lock (dedupeKey) remains set for the full outbound/inbound
ringing period when calling s.Redis.SetNX.
| const settings = await tauriUtils.getUserSettings(); | ||
| setCallTokens({ | ||
| ...data.payload, | ||
| timeStarted: new Date(), | ||
| hasAudioEnabled: settings.start_mic_on_call, | ||
| hasCameraEnabled: settings.start_camera_on_call, | ||
| role: ParticipantRole.NONE, | ||
| isRemoteControlEnabled: true, | ||
| participants: [], | ||
| isInitialisingCall: true, | ||
| micLevel: 0, | ||
| }); |
There was a problem hiding this comment.
Guard the settings lookup in the accept path.
tauriUtils.getUserSettings() is unprotected here, unlike the outgoing-call flow. If it throws, this handler exits after tokensReceived = true, so the timeout will not reject, the toast never dismisses, and the accepted call gets stuck locally.
Suggested fix
- const settings = await tauriUtils.getUserSettings();
+ let settings = {
+ start_mic_on_call: false,
+ start_camera_on_call: false,
+ };
+ try {
+ settings = await tauriUtils.getUserSettings();
+ } catch {
+ // fall back to safe defaults
+ }
setCallTokens({
...data.payload,
timeStarted: new Date(),
hasAudioEnabled: settings.start_mic_on_call,
hasCameraEnabled: settings.start_camera_on_call,📝 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.
| const settings = await tauriUtils.getUserSettings(); | |
| setCallTokens({ | |
| ...data.payload, | |
| timeStarted: new Date(), | |
| hasAudioEnabled: settings.start_mic_on_call, | |
| hasCameraEnabled: settings.start_camera_on_call, | |
| role: ParticipantRole.NONE, | |
| isRemoteControlEnabled: true, | |
| participants: [], | |
| isInitialisingCall: true, | |
| micLevel: 0, | |
| }); | |
| let settings = { | |
| start_mic_on_call: false, | |
| start_camera_on_call: false, | |
| }; | |
| try { | |
| settings = await tauriUtils.getUserSettings(); | |
| } catch { | |
| // fall back to safe defaults | |
| } | |
| setCallTokens({ | |
| ...data.payload, | |
| timeStarted: new Date(), | |
| hasAudioEnabled: settings.start_mic_on_call, | |
| hasCameraEnabled: settings.start_camera_on_call, | |
| role: ParticipantRole.NONE, | |
| isRemoteControlEnabled: true, | |
| participants: [], | |
| isInitialisingCall: true, | |
| micLevel: 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 `@tauri/src/components/ui/call-banner.tsx` around lines 73 - 84, The
call-accept path calls tauriUtils.getUserSettings() without protection so a
thrown error leaves tokensReceived true and the accept timeout/toast stuck; wrap
the settings lookup in a try/catch and fall back to a safe default settings
object (at least providing start_mic_on_call and start_camera_on_call) before
calling setCallTokens, and ensure any error does not short-circuit the existing
flow that clears the accept timeout/toast; update the code around setCallTokens
/ tokensReceived handling to use the caught-or-default settings and proceed
normally.
| // Wait 5 seconds for tokens, otherwise show error and reject | ||
| setTimeout(() => { | ||
| if (!tokensReceived) { | ||
| toast.error("Failed to establish call. Please try again.", { | ||
| duration: 4_000, | ||
| }); | ||
| handleReject(); | ||
| } | ||
| // Clean up the socket listener after timeout regardless of success/failure | ||
| socketService.removeHandler("call_tokens_callback"); | ||
| }, 5000); |
There was a problem hiding this comment.
Avoid sending call_reject after call_accept is already in flight.
By the time this timeout fires, the server may already be inside acceptCall and can still publish call_tokens later. That creates conflicting terminal events (call_accept → call_reject → call_tokens) when token generation is just slow, which can leave caller and callee in different states.
🤖 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 `@tauri/src/components/ui/call-banner.tsx` around lines 104 - 114, The timeout
currently calls handleReject() unconditionally if tokensReceived is false, which
can send a call_reject after an accept is already in flight; modify the logic to
guard against that by checking a call-accept state before rejecting: introduce
or use an existing boolean (e.g. tokensReceived and a new callAccepted flag or
existing acceptCall flow indicator), only call handleReject() when
tokensReceived is false AND callAccepted is false, and ensure you clear the
timeout or set callAccepted when acceptCall() starts so the timeout cannot race;
still always remove the "call_tokens_callback" handler on timeout/cleanup but do
not emit a reject after an accept is initiated.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Improvements