Commit 7196fe1
authored
livekit-datatrack: stop aborting when a foreign E2EE provider errors (#1429)
## Problem
`EncryptionError` and `DecryptionError` are the error types of
`EncryptionProvider` and `DecryptionProvider`, both
`#[uniffi::export(with_foreign)]`. Foreign code implements those traits
-- `DataTrackCryptor` in the Android and Swift SDKs bridges data track
frames onto each platform's AES-GCM path -- so uniffi has to lift these
errors *into* Rust.
Both were `uniffi(flat_error)`. A flat error can be lowered but not
lifted: uniffi emits a `Lift` impl that exists only to satisfy trait
bounds and panics if called.
fn try_read(buf: &mut &[u8]) -> Result<Self> { panic!("Can't lift flat
errors") }
A panic in an FFI callback has nowhere to unwind to, so every failed
decrypt aborted the host process:
Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 31589 (Thread-24)
Abort message: 'Can't lift flat errors'
That reached anything that can fail a decrypt: a subscriber with no E2EE
manager, a key mismatch, or a single corrupt frame. Reproduced on
Android against a real SFU; iOS ships the same cryptor and [the same
exposure](https://github.com/livekit/client-sdk-swift/blob/caf6a7b4f8cd9955b8257882bae9eb118705a3bd/Sources/LiveKit/E2EE/DataTrackE2EE.swift#L55).
Present since these types were introduced in #1034 -- data tracks had
simply never run a failed decrypt across the boundary.
## Fix
Drop `flat_error` from both enums and carry the detail in the variant:
#[error("Decryption failed: {reason}")]
Failed { reason: String },
This is the shape `PacketDeliveryError` already uses for the same
reason; it is the only other error returned across a `with_foreign`
trait. Both enums also gain its `From<UnexpectedUniFFICallbackError>`
catch-all, so a foreign provider throwing something other than the
declared type surfaces as an error rather than aborting.
`reason` is free-form host context (logged, not parsed), and it is the
first time the string a foreign cryptor builds reaches Rust at all:
under `flat_error` that message was write-only, since lowering
synthesized it from `Display` and lifting never happened.
The four construction sites in `livekit/src/room/e2ee/data_track.rs`
were all `map_err(|_| ..)`. Now that there is somewhere to put the
cause, they propagate it.
## Breaking Changes
* `EncryptionError::Failed` and `DecryptionError::Failed` are struct
variants.
* Rust callers construct `Failed { reason }`.
* Foreign callers still pass a single string:
* Kotlin's positional `Failed(msg)` is source-compatible
* Swift's `Failed(message:)` becomes `Failed(reason:)`
## Test
`cargo test -p livekit-datatrack --features uniffi`: 109 passed.
Regenerated the Kotlin and Swift bindings from the built cdylib. The
converter now reads the field instead of a synthesized `Display` string,
i.e. the real `Lift` impl replaced the panicking stub:
1 -> DecryptionException.Failed(FfiConverterString.read(buf))
Built the Android AAR locally and re-ran the two e2e tests that
previously aborted (Pixel 6, real SFU): both pass, `logcat -b crash`
clean. A failed decrypt now logs and drops the frame, leaving the room
connected and the track published.1 parent c944510 commit 7196fe1
4 files changed
Lines changed: 48 additions & 10 deletions
File tree
- .changeset
- livekit-datatrack/src
- livekit/src/room/e2ee
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
91 | 95 | | |
92 | 96 | | |
93 | 97 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
36 | | - | |
37 | 36 | | |
38 | | - | |
39 | | - | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
40 | 48 | | |
41 | 49 | | |
42 | 50 | | |
43 | 51 | | |
44 | 52 | | |
45 | | - | |
46 | 53 | | |
47 | | - | |
48 | | - | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
49 | 65 | | |
50 | 66 | | |
51 | 67 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
40 | | - | |
| 40 | + | |
41 | 41 | | |
42 | 42 | | |
43 | 43 | | |
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
50 | | - | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
51 | 55 | | |
52 | 56 | | |
53 | 57 | | |
| |||
79 | 83 | | |
80 | 84 | | |
81 | 85 | | |
82 | | - | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
83 | 89 | | |
84 | 90 | | |
85 | 91 | | |
0 commit comments