BugFix: Bot-343, DataTracks return from subscribe() and read() from rust sdk#119
Open
stephen-derosa wants to merge 3 commits intolivekit:mainfrom
Open
BugFix: Bot-343, DataTracks return from subscribe() and read() from rust sdk#119stephen-derosa wants to merge 3 commits intolivekit:mainfrom
stephen-derosa wants to merge 3 commits intolivekit:mainfrom
Conversation
stephen-derosa
requested review from
alan-george-lk and
xianshijing-lk
as code owners
alan-george-lk
approved these changes
May 6, 2026
Collaborator
alan-george-lk
left a comment
alan-george-lk
left a comment
There was a problem hiding this comment.
Looks great, love the bundled unit test
There was a problem hiding this comment.
Pull request overview
This PR aligns the C++ SDK’s remote DataTrack streaming behavior with the Rust SDK update (rust-sdks#1057) by preserving and surfacing terminal subscription errors delivered via the FFI stream EOS, and by exercising that behavior in unit tests.
Changes:
- Add
DataTrackStream::terminalError()and store an optional terminal subscription error when EOS arrives with an error payload. - Log terminal subscription errors when the data reader thread exits after
read()ends. - Add unit tests validating that normal EOS produces no terminal error, while subscribe-failure EOS records a typed error.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
include/livekit/data_track_stream.h |
Exposes terminalError() and stores an optional terminal subscription error; adds test-only friend access. |
src/data_track_stream.cpp |
Parses EOS error payload from FFI events and persists it as a terminal error; clears it on local close before EOS. |
src/subscription_thread_dispatcher.cpp |
Logs terminal subscription errors after the data read loop ends. |
src/tests/unit/test_data_track_stream.cpp |
New unit tests covering terminal error behavior for normal EOS vs subscribe-failure EOS. |
src/tests/CMakeLists.txt |
Links protobuf for unit tests across platforms (supports protobuf usage in new unit test and static-link scenarios). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ladvoc
approved these changes
May 6, 2026
Contributor
ladvoc
left a comment
ladvoc
left a comment
There was a problem hiding this comment.
LGTM, just the one question. Also, have you tested this against the Rust branch?
xianshijing-lk
approved these changes
May 6, 2026
Collaborator
xianshijing-lk
left a comment
xianshijing-lk
left a comment
There was a problem hiding this comment.
lgtm if you address the comments
stephen-derosa
commented
May 7, 2026
stephen-derosa
changed the title
stephen-derosa
force-pushed
the
sderosa/BOT-343-float-up-DT-error
branch
from
b781eda to
03dfe13
Compare
integration test: publishes a data track, waits for the remote track, unpublishes before subscribing, then asserts the stream ends via read() == false with terminalError().code update rust hash
stephen-derosa
force-pushed
the
sderosa/BOT-343-float-up-DT-error
branch
from
3d966d6 to
10a809d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
aligns with changes here:
livekit/rust-sdks#1057
Depends on: