Skip to content

fix(sync): notification can get stuck.#20363

Open
Giyutomioka-SS wants to merge 1 commit intoankidroid:mainfrom
Giyutomioka-SS:stuck-sync-notification-19830
Open

fix(sync): notification can get stuck.#20363
Giyutomioka-SS wants to merge 1 commit intoankidroid:mainfrom
Giyutomioka-SS:stuck-sync-notification-19830

Conversation

@Giyutomioka-SS
Copy link
Contributor

@Giyutomioka-SS Giyutomioka-SS commented Feb 25, 2026

Purpose / Description

This change makes sure the sync notifications (“Syncing…” and “Syncing media…”) are always removed, even when sync is cancelled or fails with an error. Previously they were only cleared on a clean, successful sync, so in rare cases they could stay stuck for a long time.

Fixes

Approach

When the main sync worker and the media sync worker finish, we now always call notificationManager.cancel(...) from a finally block. This runs no matter how the work ends: success, error, or when the user presses the Cancel button on the notification, so the ongoing sync notifications are reliably dismissed.

How Has This Been Tested?

Verified that both sync workers now always cancel their notifications from a finally block on all exit paths (success, error, and user cancel), and that the project builds and ktlint passes locally.

but i haven’t been able to reliably reproduce the original stuck-notification issue, so I’d really appreciate it if someone who has seen it could help confirm the fix end to end, or share the exact steps to reproduce it.

Related:

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

Copy link
Contributor

@criticalAY criticalAY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logically seems correct to me but can you add tests to verify this, add tests to see if you can reproduce the error there and your solution later should pass the tests.

@criticalAY criticalAY added Needs Author Reply Waiting for a reply from the original author and removed Needs Review labels Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Author Reply Waiting for a reply from the original author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sync notification can get stuck

2 participants