fix(host): convert undeclared callback throws instead of aborting the process - #452
Merged
Merged
Conversation
TarikGul
reviewed
Aug 19, 2026
TarikGul
reviewed
Aug 19, 2026
TarikGul
reviewed
Aug 19, 2026
TarikGul
approved these changes
Aug 19, 2026
TarikGul
left a comment
Member
There was a problem hiding this comment.
Nice job just a few comments
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
HostRejection,HostStorageError,HostNavigateRejectionandChainProviderError, instead of letting UniFFI's generic converter panicon_core_logandauth_state_changed, which have no error type to convert intoThrowablerather thanExceptionin every hand-written Kotlin adapter, and bound the reason a product receives at 256 charactersChainMessageListener::on_messageandon_closedfallible, closing the connection while still deliveringon_closedwhen a listener failsrust/crates/truapi-provider/to the filter that decides whether the iOS job runs at allThree changes, one defect class: untrusted input, or a host that fails in a way it did not declare, must not abort the trusted process.
Cargo.tomlsetspanic = "abort"on the shipping profile, so every one of these was a process kill rather than an error.Foreign callbacks that throw an undeclared type
A host callback that throws an error type it does not declare is now reported to the product as a rejection. Each of the four error types converts an unexpected foreign error into its own
Unknownvariant and logs it; without that conversion UniFFI's generic converter panics.The two infallible callbacks have nowhere to convert into, so the Kotlin adapter stops the exception before it reaches the FFI rather than funnelling it. iOS is not skipped here: Swift declares both members non-throwing (
func onCoreLog(marker:detail:),func authStateChanged(state:)), so a Swift host cannot throw from them at all, and only Kotlin needs the guard.Each hand-written adapter funnels host throws into the type its callback declares, catching
Throwablerather thanExceptionso a KotlinError(TODO(),OutOfMemoryError) cannot escape into a success the product never got. The reason a product receives keeps the message a host wrote, and falls back to the error's type name for values whose stored properties would otherwise cross the boundary.This covers every host callback, not only Chat: storage, chain, navigation, permissions, confirmation, theme and preimage lookups all reach the same converter.
Recursive decode is depth-bounded
Host-initiated subscription items decode with a depth limit of 64. A recursive product-supplied payload, in practice a custom renderer tree, otherwise decodes until the thread stack is gone, and a stack overflow is not a panic: nothing catches it, and
panic = "abort"is irrelevant to it.64 is measured against the real
ProductChatCustomMessageRenderItem: 64 accepted, 65 refused. The deepest tree anywhere in this repo is three or four levels. Depth maps to UI nesting rather than breadth, so a flat list of ten thousand siblings costs one level.A refused item ends its own subscription and is logged with its request id. The peer sees the same stop frame as a clean teardown and the host sees the same completion, so the log line is the only signal that anything was refused.
The provider's listener callbacks are fallible
ChainMessageListener::on_messageandon_closedreturnResult<(), ChainProviderError>. A listener that fails closes the connection and still receiveson_closed: the response stream is take-once, so it cannot be pumped again, and leaving the handle open would queue every later send against a receiver that is gone.Breaking
ChainMessageListener's two methods now throw, andChainProviderErrorgains a.listener(reason:)case, which is source-breaking for an exhaustive Swiftswitch.The crate's UniFFI surface has never shipped: there is no
@parity/ios-providerrelease, the release allowlist excludes it, the Maven publication is local-only, and nothing in this repo implements the trait. The npm@parity/truapi-providerbuilds are wasm, wheremod ffiis compiled out entirely.Validation
cargo test --workspace --all-features: 934 passed, 15 ignoredcargo clippy --workspace --all-targets --all-features -- -D warningscargo +nightly fmt --checkcargo check --target wasm32-unknown-unknown -p truapi-server./scripts/codegen.shwith no tracked-file drift;make uniffireproduces the committed iOS bindings, andmake provider-swift-checkthe provider'sThree tests carry the claims, and each was mutation-proven against the defect it describes: the unexpected-error conversion fails by panicking when its
Fromimpl is removed; the depth bound decodes 256 levels happily when reverted to plaindecode; and the throwing-listener test loseson_closedwhen the pump returns instead of breaking.The binding checks are the point of the new CI step.
rebuild.shoverwrites the committed provider bindings in place, so before this change nothing could fail on drift.Notes
A host still cannot tell a clean
on_closedfrom one that followed its own listener failing. Widening the signature to say which is a larger change than this PR wants, and nothing in tree builds retry or reconnect on that callback yet; it is worth an issue for whoever does.NativeCustomRendererObserveris the one remaining foreign trait with infallible methods. Its only implementation arrives with the Android chat surface in the PR stacked on this one, and carries its guard there.The Swift compile is left to CI rather than run locally: the
TRUAPIProviderproduct added in #276 resolves an unpublished@parity/ios-providerrelease asset, soxcodebuildneeds bothTRUAPI_USE_LOCAL_BINARY=1andTRUAPI_PROVIDER_USE_LOCAL_BINARY=1with both xcframeworks built first, which is what theiOS package (swift compile)job does.