Skip to content

fix(browser-extension): handle unreachable background script on Firef…#2649

Open
Joly0 wants to merge 1 commit intokarakeep-app:mainfrom
Joly0:main
Open

fix(browser-extension): handle unreachable background script on Firef…#2649
Joly0 wants to merge 1 commit intokarakeep-app:mainfrom
Joly0:main

Conversation

@Joly0
Copy link
Copy Markdown

@Joly0 Joly0 commented Apr 3, 2026

…ox Android
Fixes #2122

When saving a bookmark via the Firefox Android extension, the popup tries to send a badge refresh message to the background script. On Firefox Android the background script isn't always reachable from the popup context, causing chrome.runtime.sendMessage to throw. This error bubbled up through the mutation's onSuccess callback and showed an error to the user even though the bookmark was saved.

Tested on firefox nightly on android

…ox Android

Signed-off-by: Joly0 <13993216+Joly0@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 3, 2026

Walkthrough

Both BookmarkSavedPage and SavePage updated to wrap badge refresh logic in try/catch blocks, treating Chrome tab queries and runtime message sends as best-effort operations. Errors are suppressed while navigation and success flows continue unconditionally.

Changes

Cohort / File(s) Summary
Badge refresh error handling
apps/browser-extension/src/BookmarkSavedPage.tsx, apps/browser-extension/src/SavePage.tsx
Wrapped Chrome tab query and runtime message sending in try/catch blocks within success handlers. Badge refresh is now treated as best-effort, preventing failures from disrupting subsequent navigation or success flows.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: handling unreachable background script on Firefox Android, which matches the changeset's error handling improvements in both files.
Linked Issues check ✅ Passed The PR fully addresses issue #2122 by wrapping chrome.runtime.sendMessage and chrome.tabs.query calls in try/catch blocks in both SavePage.tsx and BookmarkSavedPage.tsx, preventing errors from propagating to users when the background script is unreachable on Firefox Android.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the Firefox Android background script communication issue; no unrelated modifications were introduced beyond the necessary error handling improvements.
Description check ✅ Passed The description clearly explains the bug (unreachable background script on Firefox Android), its impact (error shown despite successful save), and the affected files, directly relating to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 3, 2026

Greptile Summary

This PR fixes a spurious error shown to users when saving a bookmark via the Firefox Android extension, caused by chrome.runtime.sendMessage throwing when the background script is unreachable in that environment.

The fix wraps the badge-refresh calls (chrome.tabs.query + chrome.runtime.sendMessage) in a try/catch in both SavePage.tsx and BookmarkSavedPage.tsx, treating badge updates as best-effort. The bookmark operation itself is unaffected — errors from the actual save/delete are still surfaced through the existing onError handlers, and navigation after delete still executes correctly outside the try block.

Confidence Score: 5/5

Safe to merge — the fix is minimal, targeted, and correctly isolates badge-refresh failures from the bookmark operation result.

Both changes are identical in structure, well-commented, and correctly scoped: real errors (bookmark save/delete failures) still reach the user via onError, while the non-critical badge refresh is now silently swallowed on platforms where it cannot succeed. No logic changes to the core bookmark flow.

No files require special attention.

Important Files Changed

Filename Overview
apps/browser-extension/src/SavePage.tsx Wraps the post-save badge refresh calls in a try-catch so that an unreachable background script on Firefox Android no longer surfaces an error to the user after a successful bookmark save.
apps/browser-extension/src/BookmarkSavedPage.tsx Same best-effort try-catch applied to the badge refresh in the delete-bookmark success handler; navigation to "/bookmarkdeleted" is correctly kept outside the try-catch so it always runs.

Reviews (1): Last reviewed commit: "fix(browser-extension): handle unreachab..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Firefox Extension on android shows error message after hoarding

1 participant