Skip to content

fix: more graceful handling of SFU join failures - #1853

Merged
oliverlaz merged 6 commits into
mainfrom
join-failures
Jul 18, 2025
Merged

fix: more graceful handling of SFU join failures#1853
oliverlaz merged 6 commits into
mainfrom
join-failures

Conversation

@oliverlaz

@oliverlaz oliverlaz commented Jul 14, 2025

Copy link
Copy Markdown
Member

💡 Overview

Keeps track of failed SFU join requests.
Once we fail to connect to the same SFU twice, we ask the Coordinator for another one.

📝 Spec: https://www.notion.so/stream-wiki/SFU-WebSocket-client-tracing-19d6a5d7f9f6806d8a55c4ae0d6a415c
🎫 Ticket: https://linear.app/stream/issue/REACT-488/graceful-sfu-join-failure-handling
🔗 Ref: https://github.com/GetStream/video-sfu/pull/1182

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Addresses SFU join failures by tracking retry attempts and requesting a new SFU after repeated errors.

  • Tracks per-SFU join failures and sets migrating_from on the second failure.
  • Cleans up the migrating_from field after retries and changes doJoin to private.
Comments suppressed due to low confidence (1)

packages/client/src/Call.ts:872

  • The new branch that sets migrating_from after two failures should have corresponding unit tests to verify behavior on both success-after-migration and ultimate failure.
        if ((sfuJoinFailures.get(sfuId) || 0) >= 2) {

Comment thread packages/client/src/Call.ts
Comment thread packages/client/src/Call.ts
Comment thread packages/client/src/Call.ts
Comment thread packages/client/src/Call.ts Outdated
Comment thread packages/client/src/Call.ts Outdated
}

// remove the migrating_from field from the join data
delete joinData.migrating_from;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe I'm missing something, but what's the point of this? joinData is local to this join() method, so why bother cleaning stuff up?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We re-use joinData for reconnects. If we keep the migrating_from there, on the next reconnect event, it will "mislead" the coordinator.

@oliverlaz oliverlaz changed the title fix: improved SFU join failures fix: more graceful handling of SFU join failures Jul 18, 2025
@oliverlaz
oliverlaz merged commit f38a4b5 into main Jul 18, 2025
17 checks passed
@oliverlaz
oliverlaz deleted the join-failures branch July 18, 2025 13:53
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.

3 participants