-
Notifications
You must be signed in to change notification settings - Fork 157
Reconnect errors #845
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
base: main
Are you sure you want to change the base?
Reconnect errors #845
Changes from 7 commits
c101579
f57e969
abf2933
8e66f55
199d70b
b60711c
5885307
5d73842
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| patch type="fixed" "Reconnect sequence stuck in failed state" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,23 +32,29 @@ extension Room: SignalClientDelegate { | |
| // engine is currently connected state | ||
| case .connected = _state.connectionState | ||
| { | ||
| do { | ||
| try await startReconnect(reason: .websocket) | ||
| } catch { | ||
| log("Failed calling startReconnect, error: \(error)", .error) | ||
| Task { | ||
| do { | ||
| try await startReconnect(reason: .websocket) | ||
| } catch { | ||
| log("Failed calling startReconnect, error: \(error)", .error) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func signalClient(_: SignalClient, didReceiveLeave canReconnect: Bool, reason: Livekit_DisconnectReason) async { | ||
| log("canReconnect: \(canReconnect), reason: \(reason)") | ||
|
|
||
| let error = LiveKitError.from(reason: reason) | ||
|
|
||
| if canReconnect { | ||
| // force .full for next reconnect | ||
| _state.mutate { $0.nextReconnectMode = .full } | ||
| // Abort current connection attempt | ||
| await signalClient.cleanUp(withError: error) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is more or less equivalent to JS: case LeaveRequest_Action.RECONNECT:
this.fullReconnectOnNext = true;
// reconnect immediately instead of waiting for next attempt
this.handleDisconnect(leaveReconnect);
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (if-minor): maybe this is a good moment to adopt the We don't actually need a full reconnect on every leave request. But if it makes more sense as a follow up that also sounds good.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, moved to the action param here 👍 Re: backwards compatibility, I discarded the legacy |
||
| } else { | ||
| // Server indicates it's not recoverable | ||
| await cleanUp(withError: LiveKitError.from(reason: reason)) | ||
| await cleanUp(withError: error) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -319,17 +325,19 @@ extension Room: SignalClientDelegate { | |
| } | ||
| } | ||
|
|
||
| func signalClient(_: SignalClient, didReceiveAnswer answer: LKRTCSessionDescription) async { | ||
| func signalClient(_: SignalClient, didReceiveAnswer answer: LKRTCSessionDescription, offerId: UInt32) async { | ||
| log("Received answer for offerId: \(offerId)") | ||
|
|
||
| do { | ||
| let publisher = try requirePublisher() | ||
| try await publisher.set(remoteDescription: answer) | ||
| try await publisher.set(remoteDescription: answer, offerId: offerId) | ||
| } catch { | ||
| log("Failed to set remote description, error: \(error)", .error) | ||
| log("Failed to set remote description with offerId: \(offerId), error: \(error)", .error) | ||
| } | ||
| } | ||
|
|
||
| func signalClient(_ signalClient: SignalClient, didReceiveOffer offer: LKRTCSessionDescription) async { | ||
| log("Received offer, creating & sending answer...") | ||
| func signalClient(_ signalClient: SignalClient, didReceiveOffer offer: LKRTCSessionDescription, offerId: UInt32) async { | ||
| log("Received offer with offerId: \(offerId), creating & sending answer...") | ||
|
|
||
| guard let subscriber = _state.subscriber else { | ||
| log("Failed to send answer, subscriber is nil", .error) | ||
|
|
@@ -340,9 +348,9 @@ extension Room: SignalClientDelegate { | |
| try await subscriber.set(remoteDescription: offer) | ||
| let answer = try await subscriber.createAnswer() | ||
| try await subscriber.set(localDescription: answer) | ||
| try await signalClient.send(answer: answer) | ||
| try await signalClient.send(answer: answer, offerId: offerId) | ||
| } catch { | ||
| log("Failed to send answer with error: \(error)", .error) | ||
| log("Failed to send answer for offerId: \(offerId), error: \(error)", .error) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ internal import LiveKitWebRTC | |
| actor Transport: NSObject, Loggable { | ||
| // MARK: - Types | ||
|
|
||
| typealias OnOfferBlock = @Sendable (LKRTCSessionDescription) async throws -> Void | ||
| typealias OnOfferBlock = @Sendable (LKRTCSessionDescription, UInt32) async throws -> Void | ||
|
|
||
| // MARK: - Public | ||
|
|
||
|
|
@@ -56,6 +56,7 @@ actor Transport: NSObject, Loggable { | |
| private var _reNegotiate: Bool = false | ||
| private var _onOffer: OnOfferBlock? | ||
| private var _isRestartingIce: Bool = false | ||
| private var _latestOfferId: UInt32 = 0 | ||
|
|
||
| // forbid direct access to PeerConnection | ||
| private let _pc: LKRTCPeerConnection | ||
|
|
@@ -110,6 +111,20 @@ actor Transport: NSObject, Loggable { | |
| await _iceCandidatesQueue.process(candidate, if: remoteDescription != nil && !_isRestartingIce) | ||
| } | ||
|
|
||
| func set(remoteDescription sd: LKRTCSessionDescription, offerId: UInt32) async throws { | ||
| if signalingState != .haveLocalOffer { | ||
| log("Received answer with unexpected signaling state: \(signalingState), expected .haveLocalOffer", .warning) | ||
| } | ||
|
|
||
| if offerId == 0 { | ||
| log("Skipping validation for legacy server (missing offerId), latestOfferId: \(_latestOfferId)", .warning) | ||
| } else if offerId != _latestOfferId { | ||
| throw LiveKitError(.invalidState, message: "OfferId mismatch, expected \(_latestOfferId) but got \(offerId)") | ||
| } | ||
|
|
||
| try await set(remoteDescription: sd) | ||
| } | ||
|
|
||
| func set(remoteDescription sd: LKRTCSessionDescription) async throws { | ||
| try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, Error>) in | ||
| _pc.setRemoteDescription(sd) { error in | ||
|
|
@@ -157,12 +172,14 @@ actor Transport: NSObject, Loggable { | |
|
|
||
| // Actually negotiate | ||
| func _negotiateSequence() async throws { | ||
| _latestOfferId += 1 | ||
| let offer = try await createOffer(for: constraints) | ||
| try await set(localDescription: offer) | ||
| try await _onOffer(offer) | ||
| try await _onOffer(offer, _latestOfferId) | ||
| } | ||
|
|
||
| if signalingState == .haveLocalOffer, iceRestart, let sd = remoteDescription { | ||
| _reNegotiate = false // Clear flag to prevent double offer | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if (this._pc && this._pc.signalingState === 'have-local-offer') {
const currentSD = this._pc.remoteDescription;
if (options?.iceRestart && currentSD) {
// 1. Rollback: Sets remote description
await this._pc.setRemoteDescription(currentSD);
// 2. DOES NOT set this.renegotiate = true
// 3. Falls through to create offer (line 291)
} else {
// 1. Defer: Sets renegotiate = true
this.renegotiate = true;
// 2. Returns immediately (skips offer creation)
return;
}
}vs Swift if signalingState == .haveLocalOffer {
if !(iceRestart && remoteDescription != nil) {
// 1. Defer: Sets _reNegotiate = true
_reNegotiate = true
// 2. Returns immediately
return
}
// Else: ICE Restart path falls through...
}
// ... (offer ID increment) ...
if signalingState == .haveLocalOffer, iceRestart, let sd = remoteDescription {
// 1. Force clear _reNegotiate (Matches JS not setting it)
_reNegotiate = false
// 2. Rollback: Sets remote description
try await set(remoteDescription: sd)
// 3. Creates new offer immediately
return try await _negotiateSequence()
} |
||
| try await set(remoteDescription: sd) | ||
| return try await _negotiateSequence() | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.