Break out livekit-rpc into its own crate - #1376
Open
1egoman wants to merge 11 commits into
Open
Conversation
1egoman
force-pushed
the
break-out-livekit-rpc
branch
from
August 28, 2026 20:50
0787d35 to
dff1720
Compare
1egoman
commented
Aug 28, 2026
| method: &str, | ||
| payload: &str, | ||
| response_timeout: Duration, | ||
| ) -> Result<(), crate::room::RoomError> { |
Contributor
Author
There was a problem hiding this comment.
Note to reviewers - This new RpcTransportError properly encapsulates the rpc errors for this new livekit-rpc crate. They are converted back to RoomError on the livekit crate end.
Comment on lines
+15
to
+26
| //! Re-exports of the [`livekit_rpc`] crate at the paths RPC has always occupied. | ||
| //! | ||
| //! This module exists purely for source compatibility — it has no callers inside this crate. | ||
| //! Code within `livekit` imports from `livekit_rpc` directly; the production transport lives | ||
| //! in [`super::rpc_transport`]. | ||
|
|
||
| pub use livekit_rpc::api::*; | ||
|
|
||
| // Historically public at `livekit::rpc::*`, but not usable without a transport, which is | ||
| // internal. Kept reachable so existing code compiles, hidden from the docs. | ||
| #[doc(hidden)] | ||
| pub use livekit_rpc::backend::{HandleRequestOptions, RpcClientManager, RpcServerManager}; |
Contributor
Author
There was a problem hiding this comment.
Note to reviewers - I've kept all old symbols still reachable under livekit::room::rpc::* for backwards compatibility. This arguably is a bit overkill (given these symbols are all reexported from the crate root) and it might be worth just dropping this file outright. Curious what others think here.
1egoman
force-pushed
the
break-out-livekit-rpc
branch
from
August 31, 2026 15:47
cef24a3 to
73327b5
Compare
1egoman
requested review from
MaxHeimbrock,
cloudwebrtc,
jhugman and
lukasIO
as code owners
August 31, 2026 16:00
Groundwork for moving the RPC implementation out of the `livekit` crate, alongside the existing `livekit-data-stream` and `livekit-datatrack` crates. Adds the manifest, README, CHANGELOG and AGENTS.md, plus an empty `lib.rs` so the crate builds; the implementation moves over in the following commit. Also registers the crate everywhere a new workspace member has to be declared: the workspace `members` list and `[workspace.dependencies]`, a `[packages]` block in knope.toml (CI rejects a publishable crate that knope does not manage), CODEOWNERS, and a `cargo test -p livekit-rpc` step in tests.yml. That step is the point of the extraction: the crate does not depend on libwebrtc, so its tests run without building WebRTC.
Pure file moves, with no edits to the contents, so the diff records renames rather than a deletion and a matching addition. `rpc/mod.rs` becomes `types.rs` because the domain types are the bulk of it; the constants and the transport trait are split back out in the next commit. This commit does not build on its own: the moved code still refers to `crate::room::RoomError` and friends, and the `livekit` crate still declares a `rpc` module that no longer exists. The following two commits sever those couplings and reconnect `livekit` to the new crate.
Severs the four references the moved code still had into the `livekit` crate, so it builds standalone. The transport seam it goes through was already in place; only its error type had to change. - `crate::room::RoomError` becomes `RpcTransportError`, a message-only newtype owned by this crate. Both consumers only ever stringify the error, so the concrete engine error type can stay in `livekit`. This mirrors `livekit_data_stream::api::SendError`, which solves the same problem. - `libwebrtc::native::create_random_uuid` becomes a local `uuid::Uuid::new_v4` helper. This is the only reason RPC needed libwebrtc, so dropping it is what lets the crate build without WebRTC. `livekit-data-stream` already made the same substitution for stream ids, so no new dependency enters the lockfile. - `livekit_signaling::CLIENT_PROTOCOL_DATA_STREAM_RPC` becomes the `livekit_common` constant that livekit-signaling was already re-exporting. - `crate::data_stream::api`, `crate::room::id` and `crate::room::participant` become `livekit_data_stream::api` and `livekit_common`, which is what those paths were re-exporting anyway. Splits the moved `types.rs` into `constants.rs` (wire topics, versions and stream attribute keys) and `transport.rs` (the `RpcTransport` trait), and adds the `api`/`backend` facade split that livekit-data-stream and livekit-datatrack use: `api` is the end-user surface, `backend` is what the `livekit` crate drives. Anything with no caller outside the crate is now private, including the RPC version constants, the `lk.rpc_request_*` attribute keys, `RpcHandlerFn` and `send_v1_request`. `RpcError::built_in` had to become `pub` because `LocalParticipant::perform_rpc` calls it. The production transport impl, `SessionTransport`, needs `RoomSession`'s private fields and so cannot live here; the next commit rehomes it.
Restores the build. From a user's perspective nothing changes: `livekit::rpc`,
`livekit::participant` and `livekit::prelude` expose exactly the types and
paths they did before, so livekit-ffi and examples/rpc compile untouched.
From an author's perspective the types now come from `livekit-rpc`. Internal
code imports `livekit_rpc::{api, backend}` directly rather than hopping through
the re-export, which leaves `room/rpc.rs` as a pure compatibility shim with no
callers inside this crate.
`SessionTransport` lands in a new private `room/rpc_transport.rs`. It needs
`RoomSession`'s `rtc_engine` field and `DataPacketKind`, so it could not move
into livekit-rpc; keeping it out of `room/rpc.rs` is what lets that file stay a
pure re-export. Its `publish_data` now maps the engine error with
`RpcTransportError::new`, which also drops an `EngineError` -> `RoomError`
conversion that only existed to be stringified.
`livekit` also loses its `semver` dependency, which only the RPC client used.
`RpcServerManager::handle_request` was split into `handle_v1_request` and `handle_v2_request_stream` when RPC v2 landed, but the doc link was left behind. It went unnoticed because the module was private inside `livekit`, so rustdoc never resolved it; now that this is a crate root, it warns.
`perform_rpc` gates on the server being at least 1.8.0 and parsed the reported version with `Version::parse(..).unwrap()`, so a server whose `server_info` version is not valid semver took down the calling task. A version we cannot parse is not evidence that the server is too old, so fail open: log it and let the call proceed. The minimum is now a `Version::new` constant rather than a parsed literal, which removes the second unwrap too.
`pending_acks` and `pending_responses` were only ever drained by a response, an ack or a timeout. Nothing removed entries when the participant being called left the room, so `RpcErrorCode::RecipientDisconnected` was unreachable in practice: the only code that dropped a pending sender was `#[cfg(test)]`. A caller whose recipient disconnected sat there for the full response timeout (15s by default) and then reported `ResponseTimeout`, which is both slow and the wrong error. Pending entries now carry the identity they were sent to, so `handle_participant_disconnected` can fail exactly the calls addressed to the departing participant and leave the rest alone. `RoomSession` calls it from `handle_participant_disconnect`, next to the existing `AbortStreamsFrom` that terminates that participant's data streams for the same reason, and calls `fail_all_pending` when the room closes. Waiting for the ack also distinguished `Ok(Ok(()))` from `Ok(Err(_))` only by accident: a dropped ack sender fell into the "ack received" arm and went on to wait for a response that would never come. It now reports the disconnect.
`publish_rpc_response_packet` took `payload: Option<String>` and `error: Option<proto::RpcError>` and did `payload.unwrap()` whenever `error` was `None`. Every caller happened to pass `Some` in that case, so it never fired, but the invariant lived only in the callers. The two arguments were always a `Result`, so take one. The unwrap is gone because the state it guarded against can no longer be constructed, and the v1 handler's `(Option, Option)` shuffle collapses into the match it was emulating.
Decoding a v1 `RpcResponse` mapped both `CompressedPayload` and a missing
`value` to `(None, None)`. The client treats "no payload and no error" as a
successful empty response, so either case resolved the caller with `Ok("")` —
a silent wrong answer rather than a failure.
Both now produce an `ApplicationError` naming the reason. Actually reading
compressed responses is a separate feature; this only stops us reporting one
as an empty success.
The uuid helper, `MIN_RPC_SERVER_VERSION` and the `Pending` struct were each inserted between `RpcClientManager`'s doc comment and its declaration, so the comment ended up documenting whatever landed directly beneath it. The public struct was left undocumented and the private constant carried a summary of the manager fused onto its own. Moves the three lines back down to sit immediately above the struct.
1egoman
force-pushed
the
break-out-livekit-rpc
branch
from
August 31, 2026 16:08
73327b5 to
ebcd02d
Compare
Contributor
Changeset ✓This PR includes a changeset covering all affected packages:
|
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.
Summary
In the original rpc v2 change, I kept all of the newly introduced rpc logic in the
livekitcrate. At the time this made sense becauselivekitwas the only consumer. However, we're rapidly moving towards a world where much of the client logic in each sdk will be shared rust code, and as part of that, rpc will need to be exposed vialivekit-uniffi.So, in preparation for this, relocate all rpc code out of
livekitand into a newlivekit-rpccrate. This new crate matches quite closely with howlivekit-datatrackandlivekit-data-streamare organized - there's anapinamespace with all of the symbols to re-export publicly vialivekit, and abackendnamespace with all symbols which are required to wire up theRpcClientManagerandRpcServerManagerinto a client.To keep this small / isolated, I've opted to save the actual uniffi integration and wiring up into
livekit-uniffifor a follow up pull request. It should be pretty straightforward though, largely just addlivekit-rpcas a dependency tolivekit-uniffi, then expose everything inapiandbackendvia the ffi for other sdks to consume. It's not super high priority though since all sdks already support rpc v2 via their own downstream language implementations.Warning
This pull request is probably most easily reviewed going commit by commit. I have structured it so that each commit is fairly atomic and easy to understand, but because of the file moves the diff of all commits merged together is fairly complex to reason about.
Drive by bug fixes
As part of this change, a LLM found a few bugs in the existing rpc v2 code, which I've opted to fix at the same time (these are each in their own commit for ease of review):
RecipientDisconnected. Previously, pending calls were never purged on disconnect, so the caller waited out its full response timeout (15s by default) and gotResponseTimeoutinstead.RpcResponsecarrying the unused compressed payload field, or no value at all, now fails with anApplicationErrorinstead of resolving the caller with an empty successful response.unwrapwhen building a v1 response packet, by giving the function a signature that cannot represent the invalid state.TODO
mainbefore merginglivekit-rpccrate to crates.io with trusted publishing enabled