harden reconnect behaviour#1148
Conversation
ChangesetThe following package versions will be affected by this PR:
|
| // A server-requested reconnect signals retry_now_notify to collapse | ||
| // this wait so the next attempt fires immediately. | ||
| let backoff = reconnect_backoff_delay(i); | ||
| tokio::select! { |
There was a problem hiding this comment.
issue: If the room is explicitly closed by the user, this task will not be properly cancelled. The addition of this select! brought my attention here, but this is a pre-existing issue.
You will find there is a close_notifier referenced on L805 with a comment mentioning it needs to be used as a signal to cancel this task—however, this was wired up. Solution is to wire this up and use it as a branch in your select! in order to be able to break out of the loop early when the room is closed.
lukasIO
requested review from
MaxHeimbrock,
alan-george-lk,
stephen-derosa and
xianshijing-lk
as code owners
…into lukas/reconnect
There was a problem hiding this comment.
wanted to call this out specifically. It's "just" a spec generated by https://github.com/juxt/allium, but in case someone is opposed to adding this in, let me know
2 participants
Before you submit your PR
Make sure the following is true before submitting your PR:
PR description
Describe the changes in this PR. Explain what the PR is meant to solve and how to reproduce the issue in the first place.
Breaking changes
If this PR introduces breaking changes, list them here and document the rationale for introducing such a change.
MSRV
If the PR modifies the crate's MSRV (Minimum Supported Rust Version), document it here.
Testing
Ideally, unit test the code you add, but ensure you're not repeating existing test cases. Use as many already written scaffolding, utilities as possible; write your own, when needed. If external services, APIs, tokens are required (e.g., running an LK server instance), provide the necessary information. Make sure your tests perform useful, context-aware assertions and do not simply emulate "happy paths".
Async
We want the project to be runtime-agnostic, so please reuse what's already in livekit-runtime and feel free to add anything missing. It's ok to use Tokio directly, when writing unit tests, if necessary. When testing, do not use artificial delays for the state to "catch up"; instead, respect the event flow and subscribe properly using channels or other mechanisms.