-
Notifications
You must be signed in to change notification settings - Fork 218
Enable set_ws_client and set_http_client injectable interfaces #1290
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| livekit-net: patch | ||
| livekit-uniffi: patch | ||
| livekit-api: patch | ||
| livekit: patch | ||
| livekit-ffi: patch | ||
| --- | ||
|
|
||
| Add `self_test_http_get` / `self_test_ws_echo` / `has_http_client` / `has_ws_client` UniFFI exports so foreign hosts can exercise the transport seam end-to-end. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,9 @@ pub use types::{Header, HttpResponse, TransportError}; | |
|
|
||
| use std::sync::{Arc, OnceLock}; | ||
|
|
||
| #[cfg(feature = "uniffi")] | ||
| uniffi::setup_scaffolding!(); | ||
|
|
||
| /// Render a URL for logging with secrets stripped: userinfo (`user:password@`) | ||
| /// and the query string (which can carry an access token). Keeps scheme, host, | ||
| /// port, and path. | ||
|
|
@@ -45,16 +48,60 @@ static HTTP: OnceLock<Arc<dyn HttpClient>> = OnceLock::new(); | |
| /// | ||
| /// Independent of [`set_http_client`]: a consumer that only needs HTTP (e.g. a | ||
| /// token source) can register that alone, and vice versa. | ||
| #[cfg_attr(feature = "uniffi", uniffi::export)] | ||
| pub fn set_ws_client(c: Arc<dyn WsClient>) { | ||
| let _ = WS.set(c); | ||
| } | ||
|
|
||
| /// Register the process-wide HTTP client. Call once at startup, before the first | ||
| /// request. A later call is ignored (first registration wins). | ||
| #[cfg_attr(feature = "uniffi", uniffi::export)] | ||
| pub fn set_http_client(c: Arc<dyn HttpClient>) { | ||
| let _ = HTTP.set(c); | ||
| } | ||
|
|
||
| /// Self-test: GET `url` via the registered HTTP client; returns the full response | ||
| /// (status + headers + body) so callers can assert the whole struct round-trips the FFI. | ||
| /// Errors if no client is registered or the transport fails. | ||
| #[cfg_attr(feature = "uniffi", uniffi::export)] | ||
| pub async fn self_test_http_get(url: String) -> Result<HttpResponse, TransportError> { | ||
| let c = | ||
| http_client().ok_or_else(|| TransportError::Other("no http client registered".into()))?; | ||
| c.request(HttpMethod::Get, url, Vec::new(), None).await | ||
| } | ||
|
|
||
| /// Self-test: connect, send `payload`, receive one frame, close; return the echoed bytes. | ||
| /// Errors if no client is registered, the transport fails, or the peer closes first. | ||
| #[cfg_attr(feature = "uniffi", uniffi::export)] | ||
| pub async fn self_test_ws_echo(url: String, payload: Vec<u8>) -> Result<Vec<u8>, TransportError> { | ||
| let c = ws_client().ok_or_else(|| TransportError::Other("no ws client registered".into()))?; | ||
| let conn = c.connect(url, Vec::new(), 5_000).await?.connection; | ||
| conn.send(payload).await?; | ||
| let got = conn.recv().await?.ok_or(TransportError::Closed)?; | ||
| conn.close().await; | ||
| Ok(got) | ||
| } | ||
|
|
||
| /// Test probe: whether a host has explicitly registered an HTTP client via | ||
| /// [`set_http_client`]. | ||
| /// | ||
| /// Reports registration, not resolvability: on native builds [`http_client`] | ||
| /// still yields the built-in client when this returns `false`. | ||
| #[cfg_attr(feature = "uniffi", uniffi::export)] | ||
| pub fn has_http_client() -> bool { | ||
| HTTP.get().is_some() | ||
| } | ||
|
|
||
| /// Test probe: whether a host has explicitly registered a WebSocket client via | ||
| /// [`set_ws_client`]. | ||
| /// | ||
| /// Reports registration, not resolvability: on native builds [`ws_client`] still | ||
| /// yields the built-in client when this returns `false`. | ||
| #[cfg_attr(feature = "uniffi", uniffi::export)] | ||
| pub fn has_ws_client() -> bool { | ||
| WS.get().is_some() | ||
| } | ||
|
Comment on lines
+63
to
+103
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Test-only helper functions are added to the crate's permanent public API Four self-test helpers are made permanently public ( AGENTS.md rule on new public API surfaceAGENTS.md states: "When introducing new API surface, always default to private or Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| /// Resolve the process-wide WebSocket client. | ||
| /// | ||
| /// Returns the explicitly registered client if any; otherwise, on native builds, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| // Copyright 2026 LiveKit, Inc. (Apache-2.0) | ||
| use livekit_net::{ | ||
| has_http_client, has_ws_client, self_test_http_get, self_test_ws_echo, set_http_client, | ||
| set_ws_client, Header, HttpMethod, HttpResponse, TransportError, WsClient, WsConnectResult, | ||
| WsConnection, | ||
| }; | ||
| use std::sync::{Arc, Mutex}; | ||
|
|
||
| struct EchoConn { | ||
| buf: Mutex<Option<Vec<u8>>>, | ||
| } | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl WsConnection for EchoConn { | ||
| async fn send(&self, frame: Vec<u8>) -> Result<(), TransportError> { | ||
| *self.buf.lock().unwrap() = Some(frame); | ||
| Ok(()) | ||
| } | ||
| async fn recv(&self) -> Result<Option<Vec<u8>>, TransportError> { | ||
| Ok(self.buf.lock().unwrap().take()) | ||
| } | ||
| async fn close(&self) {} | ||
| } | ||
|
|
||
| struct EchoWsClient; | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl WsClient for EchoWsClient { | ||
| async fn connect( | ||
| &self, | ||
| _url: String, | ||
| _headers: Vec<Header>, | ||
| _timeout_ms: u64, | ||
| ) -> Result<WsConnectResult, TransportError> { | ||
| Ok(WsConnectResult { connection: Arc::new(EchoConn { buf: Mutex::new(None) }) }) | ||
| } | ||
| } | ||
|
|
||
| struct CannedHttpClient; | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl livekit_net::HttpClient for CannedHttpClient { | ||
| async fn request( | ||
| &self, | ||
| _method: HttpMethod, | ||
| _url: String, | ||
| _headers: Vec<Header>, | ||
| _body: Option<Vec<u8>>, | ||
| ) -> Result<HttpResponse, TransportError> { | ||
| Ok(HttpResponse { | ||
| status: 201, | ||
| headers: vec![Header { name: "x-test".into(), value: "1".into() }], | ||
| body: b"hello".to_vec(), | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn self_tests_round_trip_through_registered_clients() { | ||
| // Own test binary ⇒ fresh OnceLock, so nothing is registered yet. The probes | ||
| // report registration only, so a native build's built-in fallback doesn't | ||
| // show up here. | ||
| assert!(!has_http_client()); | ||
| assert!(!has_ws_client()); | ||
|
|
||
| set_http_client(Arc::new(CannedHttpClient)); | ||
| set_ws_client(Arc::new(EchoWsClient)); | ||
| assert!(has_http_client()); | ||
| assert!(has_ws_client()); | ||
|
|
||
| let resp = self_test_http_get("http://example/x".into()).await.unwrap(); | ||
| assert_eq!(resp.status, 201); | ||
| assert_eq!(resp.body, b"hello"); | ||
| assert_eq!(resp.headers.len(), 1); | ||
|
|
||
| let echoed = self_test_ws_echo("ws://example/x".into(), b"ping".to_vec()).await.unwrap(); | ||
| assert_eq!(echoed, b"ping"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Built-in network self-test calls can crash when triggered from a foreign host
The new self-test calls are made available to foreign callers (
uniffi::exportatlivekit-net/src/lib.rs:66) without declaring which background worker should run them, so on builds that use the built-in networking they can abort the process instead of returning an error.Impact: A host app that calls the self-test probes on a build with the built-in transport, without first registering its own transport, can hit a hard crash rather than a clean failure.
Missing tokio async_runtime on exported async functions while the native fallback uses tokio-based reqwest/tokio-tungstenite
self_test_http_get(livekit-net/src/lib.rs:67-71) andself_test_ws_echo(livekit-net/src/lib.rs:76-83) resolve the client viahttp_client()/ws_client(), which fall back tonative::NativeTransportwhen nothing was registered (livekit-net/src/lib.rs:105-139). On--features uniffi,native-tokio(a combination the PR explicitly adds to CI,.github/workflows/tests.yml:178-179), that transport drivesreqwest/tokio-tungstenitefutures which require an active tokio reactor. UniFFI polls exported async functions on its own foreign executor thread unless the export is annotated with#[uniffi::export(async_runtime = "tokio")], so there is no tokio context and the tokio I/O driver panics ("there is no reactor running"). Thelivekit-net/Cargo.toml:59-64comment asserts "No runtime feature: the exported traits arewith_foreign... and the exported setters are sync", which is no longer true now that async free functions that can drive native Rust futures are exported.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.