Skip to content

fix: don't log expected publisher data channel close as unexpected#1224

Open
longcw wants to merge 2 commits into
mainfrom
longc/fix-publisher-dc-closed-unexpectedly-on-room-delete
Open

fix: don't log expected publisher data channel close as unexpected#1224
longcw wants to merge 2 commits into
mainfrom
longc/fix-publisher-dc-closed-unexpectedly-on-room-delete

Conversation

@longcw

@longcw longcw commented Jul 7, 2026

Copy link
Copy Markdown

Fixes the spurious ERROR logs reported in livekit/agents#6250.

When the server tears a session down (e.g. the room is deleted) it closes the publisher data channels from the remote side before the engine sets its closed flag, so the close was logged at ERROR as "publisher data channel closed unexpectedly" even though it was expected.

This records the terminal disconnect (server Leave{Disconnect}) so the close it triggers isn't logged as unexpected. Genuine unexpected closes (e.g. an oversized message silently killing the channel) still log, and Resume/Reconnect are unaffected.

When the server tears a session down (e.g. the room is deleted) it closes the publisher data channels from the remote side before the engine sets its own `closed` flag, so the close was logged at ERROR as "publisher data channel closed unexpectedly" even though it was expected.

Record the terminal disconnect (server `Leave{Disconnect}`) and skip the error for the close it triggers. Genuine unexpected closes — e.g. an oversized message silently killing the channel — still log, and Resume/Reconnect are unaffected.
@longcw longcw requested a review from ladvoc as a code owner July 7, 2026 03:52
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Changeset

The following package versions will be affected by this PR:

Package Bump
livekit patch
livekit-ffi patch

@longcw longcw requested a review from a team July 7, 2026 03:54

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@davidzhao davidzhao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we should also set this bit when the user disconnects intentionally.. so close would have a reason of ClientInitiated.

@longcw

longcw commented Jul 7, 2026

Copy link
Copy Markdown
Author

we should also set this bit when the user disconnects intentionally.. so close would have a reason of ClientInitiated.

user disconnects using close() already set closed that prevents the error log

                if !inner.closed.load(Ordering::Acquire)
                    && !inner.disconnecting.load(Ordering::Acquire)
                    && inner.publisher_pc.is_connected()
                {
                    log::error!("publisher data channel '{}' closed unexpectedly", label);
                }

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.

2 participants