Skip to content

Disconnect on error #580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Disconnect on error #580

wants to merge 1 commit into from

Conversation

jpsantosbh
Copy link
Contributor

@jpsantosbh jpsantosbh commented Jul 14, 2025

Don't hang up the call due to execution errors. Instead this we disconnect from the WebSocket and let the recovery logic restart. In this case, after subscribing, the server will send a new verto.attach, allowing the client to build a new answer.

@jpsantosbh jpsantosbh requested a review from Copilot July 14, 2025 17:35
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR changes the error handling in BaseCall so that WebSocket errors trigger a reconnection instead of immediately hanging up the call.

  • Replace this.hangup() calls in two catch blocks with _disconnectAndReconnect()
  • Add a private _disconnectAndReconnect() method that emits a SwEvent.SocketError for automatic recovery
Comments suppressed due to low confidence (2)

packages/common/src/webrtc/BaseCall.ts:988

  • Add unit or integration tests to cover the new _disconnectAndReconnect logic, verifying that WebSocket errors correctly trigger the reconnection event.
  private _disconnectAndReconnect() {

packages/common/src/webrtc/BaseCall.ts:988

  • [nitpick] Include a JSDoc comment for _disconnectAndReconnect describing its purpose and parameters, improving readability and maintainability.
  private _disconnectAndReconnect() {

@jpsantosbh jpsantosbh requested a review from iAmmar7 July 14, 2025 17:39
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.

2 participants