Skip to content

Comments

Fix: Publisher re-enable tracks after room connected in case of timeout#3674

Open
BillCarsonFr wants to merge 1 commit intolivekitfrom
valere/bug/auto_recover_publish_timeout
Open

Fix: Publisher re-enable tracks after room connected in case of timeout#3674
BillCarsonFr wants to merge 1 commit intolivekitfrom
valere/bug/auto_recover_publish_timeout

Conversation

@BillCarsonFr
Copy link
Member

@BillCarsonFr BillCarsonFr commented Jan 14, 2026

Fixes https://github.com/element-hq/voip-internal/issues/442

We were calling LocalParticipant#set(Microphone|Camera)Enabled expecting it to be very resistent to early calls.
It has support for waiting for the room to be connected in order to publish the track, but it is not unlimited, it will timeout
https://github.com/livekit/client-sdk-js/blob/1744ee4ec7aaed4e71c510c34fd21667364fcce6/src/room/participant/LocalParticipant.ts#L896

We want to support this case a little bit better by ensuring the expected tracks are published once the room is really connected (signal connected).

This is not yet a good "connecting" experience. Because until you are connected to livekit, your video loop back will be lacking, and unmuting would be unresponsive.

@BillCarsonFr BillCarsonFr added the PR-Bug-Fix Release note category. A PR that fixes a bug. label Jan 14, 2026
@BillCarsonFr BillCarsonFr marked this pull request as ready for review January 14, 2026 15:24
@BillCarsonFr BillCarsonFr requested a review from a team as a code owner January 14, 2026 15:24
@toger5
Copy link
Contributor

toger5 commented Jan 16, 2026

Do we think this is an actual issue? Or is it more likely that the transport setup sometimes was racing and we ended up with a transport switch -> local connection switch which then ends up in a locked or not cleaned up old transport -> unable to unmute

@BillCarsonFr
Copy link
Member Author

BillCarsonFr commented Jan 16, 2026

Do we think this is an actual issue? Or is it more likely that the transport setup sometimes was racing and we ended up with a transport switch -> local connection switch which then ends up in a locked or not cleaned up old transport -> unable to unmute

Yes it is, as shown by the test. But it is not a final/big issue given that you can recover by muting/unmuting when it happens. Probably not occuring often but found 5 occurences in RS

@toger5
Copy link
Contributor

toger5 commented Jan 16, 2026

you can recover by muting/unmuting when it happens

This definitely implies its another thing. (the publisher bug would make exactly that impossible!!)

// If it is PublisherTrackError (408), i.e the publishing timed out,
// because it took too long to connet to the room, we are safe because
// we registered to the RoomEvent.Connected to create the tracks once connected.
this.logger.error("Failed to enable tracks", e);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to inform the console log about that as well:
because it took too long to connet to the room, we are safe

and (very minor) connet -> connect`

@BillCarsonFr
Copy link
Member Author

you can recover by muting/unmuting when it happens

This definitely implies its another thing. (the publisher bug would make exactly that impossible!!)

In this case it is unresponsive for at least 15s. But it is not final like with the local transport hot swap

@toger5
Copy link
Contributor

toger5 commented Jan 16, 2026

Let s do this:

and (very minor) connet -> connect`

and merge?

@toger5
Copy link
Contributor

toger5 commented Jan 23, 2026

Should we merge this now, that we have the rc branch created?

Comment on lines 90 to +94
ParticipantEvent.LocalTrackPublished,
this.onLocalTrackPublished.bind(this),
);

this.connection.livekitRoom.once(LivekitRoomEvent.Connected, () => {
Copy link
Member

Choose a reason for hiding this comment

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

I'm noticing some leaks: both the LocalTrackPublished callback and the Connected callback should not remain set after the scope ends

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR-Bug-Fix Release note category. A PR that fixes a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants