Skip to content

Conversation

@iparaskev
Copy link
Contributor

@iparaskev iparaskev commented Jan 2, 2026

Ends the call for our participants when the livekit participants disconnects. Closes #208

Summary by CodeRabbit

  • Bug Fixes
    • Improved connection state monitoring to automatically terminate calls when the connection is unexpectedly lost.

✏️ Tip: You can customize this high-level summary in your review settings.

@iparaskev iparaskev requested a review from konsalex as a code owner January 2, 2026 13:35
@netlify
Copy link

netlify bot commented Jan 2, 2026

Deploy Preview for hoppdocs ready!

Name Link
🔨 Latest commit c9ab5c1
🔍 Latest deploy log https://app.netlify.com/projects/hoppdocs/deploys/6957c99bc5c7bb0008290cb8
😎 Deploy Preview https://deploy-preview-214--hoppdocs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Jan 2, 2026

📝 Walkthrough

Walkthrough

The call-center component was refactored to properly handle room connection state changes. Event handlers for RoomEvent.Connected and the new RoomEvent.ConnectionStateChanged event are now dedicated callbacks with appropriate cleanup via room.on and room.off. When the connection state becomes Disconnected, the call automatically ends.

Changes

Cohort / File(s) Summary
Call Center Event Handling
tauri/src/components/ui/call-center.tsx
Refactored Connected event handler into reusable callback; added ConnectionStateChanged event subscription that triggers call termination on disconnect state; implemented proper cleanup functions and updated effect dependencies

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A hop and a skip, connection stayed true,
When internet faltered, the call lingered too,
Now state changes whisper their disconnecting song,
And calls end gracefully when networks go wrong!
Chirp

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: handling disconnection when LiveKit disconnects by ending the call.
Linked Issues check ✅ Passed The implementation addresses the core requirement from issue #208 by ending the call when LiveKit disconnects, preventing calls from remaining active after connection loss.
Out of Scope Changes check ✅ Passed All changes in call-center.tsx are directly related to handling RoomEvent.ConnectionStateChanged and ending calls on disconnection, which aligns with the issue requirements.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09a352e and c9ab5c1.

📒 Files selected for processing (1)
  • tauri/src/components/ui/call-center.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
tauri/src/components/ui/call-center.tsx (1)
tauri/src/lib/hooks.ts (1)
  • useEndCall (352-402)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: backend-go / test-backend-go
  • GitHub Check: precommit / pre-commit
🔇 Additional comments (2)
tauri/src/components/ui/call-center.tsx (2)

605-613: LGTM!

The refactoring of the Connected event handler is well-structured. The callback is properly memoized and the effect correctly manages subscription/cleanup.


659-675: The implementation is correct. LiveKit's SDK automatically handles reconnection attempts with exponential backoff when network interruptions occur, transitioning through intermediate states like Reconnecting and SignalReconnecting. The connection only reaches ConnectionState.Disconnected after these automatic retry attempts are exhausted, making it the appropriate point to end the call. This aligns with issue #208's requirement to attempt resuming the connection before terminating.

No code changes are needed. The optional suggestion to display UI feedback during Reconnecting states remains valid but is not critical.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@konsalex konsalex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the logic is prone to errors, if not mistaken.

Here we periodically check participants, and if the backend loses the Redis connection for whatever reason for a brief moment the call will end 🤔

useEffect(() => {
    if (!callTokens || !callParticipant) return;

    if (!callParticipant.is_active) {
      handleEndCall();
    }
  }, [callParticipant, teammates, callTokens, handleEndCall]);

Thinking if this logic should also be more robust.

In the case of the bug, this can also be the case right?

An example flow I am thinking with some AI help:

Time    Participant B                     Participant A
────────────────────────────────────────────────────────────────
t=0     Internet drops                    In call, seeing B
        
t=0.5   WebSocket closes                  
        → Redis channel gone              
        → is_active = false               

t=1-10  (Waiting for internet)            Refetch teammates (every 10s)
                                          Sees is_active=false
                                          → handleEndCall() ❌

t=3     Internet returns
        WebSocket reconnects
        → is_active = true again

t=3+    LiveKit reconnects                Already left the call
        Still thinks call is active

And in this case, the participant B will not get a room disconnection event, as they were offline at that point in time if it happens?

Its a bit weird how we hanlde is_active now, and it will lead to this bug again? maybe I miss some context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Call active after internet loss

3 participants