fix(android): use fromBundleOrNull in PhoneConnectionService.onStartCommand#301
Open
martin-ez wants to merge 2 commits into
Open
fix(android): use fromBundleOrNull in PhoneConnectionService.onStartCommand#301martin-ez wants to merge 2 commits into
martin-ez wants to merge 2 commits into
Conversation
…ommand
`CallMetadata.fromBundle` throws `IllegalArgumentException("Missing
required callId property in Bundle")` when the bundle has no callId.
But several `ServiceAction` cases handled in `onStartCommand` don't
carry a callId in their extras (`TearDownConnections`,
`CleanConnections`, `SyncAudioState`, `SyncConnectionState`), and the
ones that do already null-check `metadata?.callId` before using it.
The throw at line 103 makes the defensive null-checks below
unreachable, and crashes the foreground service every time the host
app triggers a teardown or sync action.
Switch to `fromBundleOrNull` so missing-callId bundles produce a null
metadata that the existing handlers already expect.
SERDUN
added a commit
that referenced
this pull request
Jun 14, 2026
#319) * fix: parse service intents into typed commands to avoid metadata crash Both PhoneConnectionService and StandaloneCallService eagerly parsed call metadata via CallMetadata.fromBundle at the top of onStartCommand, BEFORE the surrounding try/catch. Binder IPC can deliver a non-null but empty Bundle for the no-extras lifecycle commands (TearDownConnections, CleanConnections, SyncAudioState, SyncConnectionState), so the missing-callId IllegalArgumentException propagated uncaught out of onStartCommand and crashed the :callkeep_core process (confirmed in Play Vitals). Introduce PhoneServiceCommand / StandaloneServiceCommand sealed types with a from(intent) factory that parses the action and metadata once. Lifecycle commands carry no metadata and never touch the extras; Reserve/Pending carry a non-null callId; AddIncoming/Call carry non-null metadata. onStartCommand switches over the typed command, so metadata parsing only runs for actions that need it and any parse failure is non-fatal. This supersedes PR #301, which only switched PhoneConnectionService to fromBundleOrNull and left StandaloneCallService with the same latent crash. WT-1142 * fix: promote standalone call-setup to foreground before command parse When StandaloneServiceCommand.from() returned null (a call-setup intent with empty/truncated Binder extras), onStartCommand bailed out before promoteToForeground(). IncomingCall/OutgoingCall are started via startForegroundService, so skipping startForeground() crashes the process with ForegroundServiceDidNotStartInTimeException ~5s later. Decide the foreground promote from the raw StandaloneServiceAction (new isCallSetup predicate on the enum) BEFORE the command-parse bail-out, so the 5s window is satisfied even when metadata fails to parse. On the bail-out path, call the extracted stopIfIdle() so a promoted-but-unparsed call-setup intent does not linger as a zombie foreground service. Drop the now-unused isCallSetup property from StandaloneServiceCommand. WT-1142 * fix: reject connection on missing callId instead of crashing onCreateOutgoingConnection and onCreateIncomingConnection parsed metadata with CallMetadata.fromBundle, which throws IllegalArgumentException when callId is absent. request.extras comes from our own metadata.toBundle(), so a missing callId is a "should never happen" invariant violation (Binder truncation, process death/recovery, stale framework callback) -- but the uncaught throw would crash the whole :callkeep_core process, the same crash class fixed in onStartCommand. Use fromBundleOrNull and, on null, log an error and return a failed Connection (DisconnectCause.ERROR) so the framework rejects this one connection while the process stays alive. WT-1142 * fix: log distinct reason when a service command is ignored The command factory returns null both for an unrecognised action and for a known action missing its required callId/metadata, and onStartCommand logged a single generic message for both. Branch the log on whether the action resolves to a known enum value, restoring the diagnostic specificity (known-action missing field vs unknown action) lost in the ServiceCommand refactor. WT-1142
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
1 participant
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
PhoneConnectionService.onStartCommandcallsCallMetadata.fromBundle(it)on the intent extras.fromBundlethrowsIllegalArgumentException("Missing required callId property in Bundle")whencallIdis absent.But several
ServiceActioncases handled immediately below —TearDownConnections,CleanConnections,SyncAudioState,SyncConnectionState— don't carry acallIdin their extras. And the actions that do (ReserveAnswer,NotifyPending) already null-checkmetadata?.callIdbefore using it (lines 116-122).The throw at the parse step makes those defensive null-checks unreachable: the foreground service crashes every time the host app dispatches one of the no-callId actions.
This shows up in our Play Vitals as
IllegalArgumentExceptionoriginating fromCallMetadata\$Companion.fromBundleatCallMetaData.kt:121, triggered fromPhoneConnectionService.onStartCommandatPhoneConnectionService.kt:103.Fix
Use
fromBundleOrNullso missing-callIdbundles producemetadata == null, which the existing handlers already expect.