fix(notification): cancel() crashes on Android with empty notifications list (fix #2341)#3430
Open
avebeatrix wants to merge 1 commit into
Open
Conversation
…ns list (fix tauri-apps#2341) The `cancel` Android command parses `CancelArgs.notifications` as a `lateinit var List<Int>`, but the JS `cancelAll()` API and the Rust SDK's `NotificationExt::cancel_all()` both dispatch the `cancel` command with no arguments. The handler then throws `UninitializedPropertyAccessException` when it reads `args.notifications`, which on the JS side surfaces as a rejected `invoke()` promise that the polyfill silently swallows — so callers see no notifications cancelled and no error. Make `notifications` an optional list defaulting to `[]`, and route an empty list through a new `TauriNotificationManager.cancelAll()` that enumerates saved IDs from `NotificationStorage.getSavedNotificationIds()` and reuses the existing `cancel()` cleanup path (dismiss visible + cancel timer + delete storage entry). Reported in tauri-apps#2341.
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.
Fixes #2341 (the
cancelAll()part — other items in that issue are unaddressed by this PR).Problem
NotificationPlugin.kt:42declares the cancel-command argument shape as:The handler then unconditionally reads
args.notificationsand forwards it tomanager.cancel(...). Both supported callers of "cancel all" hit this dead-end:cancelAll()from@tauri-apps/plugin-notificationinvokesplugin:notification|cancelwith nonotificationsfield.NotificationExt::cancel_all()(mobile.rs) doesrun_mobile_plugin("cancel", ())— same effect.In both cases Kotlin throws
UninitializedPropertyAccessExceptionwhen the handler readsargs.notifications. On the JS side this bubbles up as a rejectedinvoke()promise that thesendNotificationpolyfill silently drops, so callers see no notifications cancelled and no error.This was observed downstream while shipping a reminder-style notification feature — debugging required tracing through the Rust SDK, the plugin's Android Kotlin, and finally
dumpsys alarmto confirm that the schedule loop was being aborted by the failingcancelAll()before anynotifycall ever ran.Fix
Three small changes, all in
plugins/notification/android/src/main/java/:NotificationPlugin.kt—CancelArgs.notificationsis nowvar notifications: List<Int> = listOf()(optional, defaults to empty). The handler routes an empty list through a newmanager.cancelAll(); non-empty still goes through the existingmanager.cancel(ids).TauriNotificationManager.kt— newcancelAll()method that enumerates saved IDs via the existingNotificationStorage.getSavedNotificationIds()and reuses the existing per-idcancel()cleanup path (dismiss visible + cancel timer + delete storage entry). No duplicated cleanup logic..changes/notification-cancel-all-android.md— patch bump on bothnotificationandnotification-jsper the repo.changes/readme.mdrule («both packages need to be bumped with the same increment even when only one side changed»).Tests
The plugin's
plugins/notification/android/src/test/currently contains only the placeholderExampleUnitTest.kt— there is no existing scaffold for handler/manager unit tests. I deliberately did not add a one-off test in this PR so the change stays minimal; happy to add a focused test forcancel(empty)if maintainers want the coverage and can confirm a preferred mocking approach forInvoke/manager.Compatibility
cancelAll(): starts working as documented.cancel([1, 2, 3]): unchanged — still routed throughmanager.cancel(ids).NotificationExt::cancel_all(): starts working.NotificationExt::cancel(vec![1, 2, 3]): unchanged.No public API change. No migration needed.