Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle errors thrown in signal client callbacks #1063

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Handle errors thrown in signal client callbacks #1063

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2fa4c83
Catch errors when setRemoteDescription or addIceCandidate on peer con…
HermanBilous Mar 5, 2024
cd7d13f
Create fast-seahorses-juggle.md
HermanBilous Mar 5, 2024
8b828b6
Fix prettier
HermanBilous Mar 5, 2024
3e5e096
Add reconnect when InvalidStateError happens
HermanBilous Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fast-seahorses-juggle.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"livekit-client": patch
---

Catch errors when peer connection signaling state is closed
31 changes: 27 additions & 4 deletions src/room/RTCEngine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,16 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
return;
}
this.log.debug('received server answer', { ...this.logContext, RTCSdpType: sd.type });
await this.pcManager.setPublisherAnswer(sd);

try {
await this.pcManager.setPublisherAnswer(sd);
} catch (error) {
const errorMessage = error instanceof Error ? error.message : 'unknown error';
log.error(`failed to setPublisherAnswer: ${errorMessage}`, {
...this.logContext,
RTCSdpType: sd.type,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably trigger a reconnect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into it again, and it looks like I'm not going to catch an InvalidStateError here at all, and instead I will get a NegotiationError. Should I look for it instead? This looks like an oddly specific error to handle and to try reconnect on.

}
};

// add candidate on trickle
Expand All @@ -456,16 +465,30 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
return;
}
this.log.trace('got ICE candidate from peer', { ...this.logContext, candidate, target });
this.pcManager.addIceCandidate(candidate, target);
this.pcManager.addIceCandidate(candidate, target).catch((error) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

in case of an InvalidStateError this should probably also trigger a reconnect

log.error(`failed to addIceCandidate: ${error.message}`, {
...this.logContext,
candidate,
target,
});
});
};

// when server creates an offer for the client
this.client.onOffer = async (sd) => {
if (!this.pcManager) {
return;
}
const answer = await this.pcManager.createSubscriberAnswerFromOffer(sd);
this.client.sendAnswer(answer);

try {
const answer = await this.pcManager.createSubscriberAnswerFromOffer(sd);
await this.client.sendAnswer(answer);
} catch (error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

const errorMessage = error instanceof Error ? error.message : 'unknown error';
log.error(`failed to createSubscriberAnswerFromOffer: ${errorMessage}`, {
...this.logContext,
});
}
};

this.client.onLocalTrackPublished = (res: TrackPublishedResponse) => {
Expand Down
Loading