Handle failures when opening extension tabs

[PeterDaveHello]

Summary

• Return the complete tab lookup and update/create flow from openUrl.
• Log query, update, and create failures internally so fire-and-forget callers do not leak rejected promises.
• Add focused success and rejection regression tests with isolated browser API overrides.

Why

The helper previously returned before browser tab operations completed. Rejected tab API calls could become unhandled rejections in every current fire-and-forget call path.

Validation

• npm test (46 test files)
• npm run test:coverage on Node.js 22 (36.18% lines)
• npm run lint
• npm run build
• Required Chromium build artifacts verified
• git diff --check
• Manual browser smoke not run; it requires an interactive browser-extension session.

[gemini-code-assist]

Code Review

This pull request updates the openUrl utility to return its promise chain, handle errors via a .catch block, and await tab operations. It also introduces comprehensive unit tests for various success and failure scenarios. The review feedback suggests refactoring openUrl to use a cleaner, more idiomatic async/await syntax with a try/catch block instead of mixing promise chains with async callbacks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[coderabbitai]

📝 Walkthrough

Walkthrough

openUrl now awaits browser tab operations, logs failures, and has unit coverage for existing-tab activation, new-tab creation, and query/update/create errors.

Changes

URL tab opening error handling

Layer / File(s)	Summary
Awaited tab opening flow src/utils/open-url.mjs	openUrl awaits tab updates and creation, then logs operation failures with console.error.
Tab opening behavior tests tests/unit/utils/open-url.test.mjs	Tests cover browser tab stubbing, existing-tab activation, new-tab creation, and failures from each tab operation.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: making tab-opening logic handle failures gracefully.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Handle browser tab API failures in openUrl without unhandled rejections

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Return the full tab query + activate/create flow from openUrl.
• Catch and log tab API failures to avoid unhandled promise rejections.
• Add unit tests covering success paths and query/update/create rejection paths.

Diagram

graph TD
  A([Caller]) --> B["openUrl(url)"] --> C["Browser.tabs.query"] --> D{Tab exists?}
  D -->|"Yes"| E["Browser.tabs.update (activate)"] --> G["Promise fulfills"]
  D -->|"No"| F["Browser.tabs.create"] --> G
  C -."reject".-> H["console.error"] --> G
  E -."reject".-> H
  F -."reject".-> H

  subgraph Legend
    direction LR
    _caller([Caller]) ~~~ _fn["Function"] ~~~ _decision{Decision} ~~~ _side["Side effect"]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Propagate rejection and require callers to handle/await

• ➕ Preserves error semantics for consumers
• ➕ Avoids potentially hiding real operational issues
• ➖ Requires touching all fire-and-forget call sites to add void openUrl(...).catch(...) or await
• ➖ Higher risk of regressions across the extension

2. Provide a `mode`/`onError` option (log vs throw)

• ➕ Keeps current safe default while enabling stricter callers to fail fast
• ➕ Centralizes error handling policy in one place
• ➖ More API surface and documentation burden
• ➖ May be overkill if no callers need strict behavior

3. Return a structured result (e.g., `{ ok: boolean, error? }`)

• ➕ Makes outcomes explicit without exceptions
• ➕ Allows callers to optionally react without try/catch
• ➖ More refactor for existing callers
• ➖ Less idiomatic than promise rejection for failures

Recommendation: Current approach (internally catching + logging) is appropriate given the stated goal: prevent unhandled rejections from existing fire-and-forget call paths while still making failures visible via logs. If future call sites need stricter behavior, consider adding an optional onError/mode parameter rather than changing default semantics again.

Files changed (2) +135 / -7

Bug fix (1) +12 / -7

open-url.mjsReturn and harden the openUrl tab activation/create flow +12/-7

Return and harden the openUrl tab activation/create flow

• 'openUrl' now returns the entire 'tabs.query' → 'tabs.update'/'tabs.create' promise chain and awaits the tab operation. All failures are caught and logged so callers do not trigger unhandled promise rejections.

src/utils/open-url.mjs

Tests (1) +123 / -0

open-url.test.mjsAdd unit coverage for openUrl success and rejection scenarios +123/-0

Add unit coverage for openUrl success and rejection scenarios

• Introduces focused tests verifying activation vs creation behavior and that failures in 'tabs.query', 'tabs.update', and 'tabs.create' are logged while the returned promise fulfills. Uses isolated runtime overrides of 'Browser.tabs' methods and restores them after each test.

tests/unit/utils/open-url.test.mjs

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)

• src/utils/open-url.mjs
• tests/unit/utils/open-url.test.mjs

---

_{Reviewed by hy3:free · Input: 56.9K · Output: 1.6K · Cached: 160.2K}

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Failure masked as success ✗ Dismissed 🐞 Bug ≡ Correctness

Description

openUrl() logs and swallows Browser.tabs query/update/create rejections and always fulfills, so any
caller awaiting it cannot distinguish success from failure (and therefore can’t implement
retry/fallback based on the return). The new tests explicitly lock in this “fulfill on failure”
contract, increasing the chance the helper is later reused in a context that needs to detect failure
but can’t.

Code

src/utils/open-url.mjs[R4-15]

+  return Browser.tabs
+    .query({ url, currentWindow: true })
+    .then(async (tabs) => {
+      if (tabs.length > 0) {
+        await Browser.tabs.update(tabs[0].id, { active: true })
+      } else {
+        await Browser.tabs.create({ url })
+      }
+    })
+    .catch((error) => {
+      console.error('failed to open url', error)
+    })

Evidence

The implementation’s terminal .catch() logs the error and returns nothing, which fulfills the
returned promise with undefined even when query/update/create rejects. The unit tests assert this
fulfill-on-failure behavior, and current production call sites invoke openUrl() without awaiting
it (so they also cannot observe failures).

src/utils/open-url.mjs[3-16]
tests/unit/utils/open-url.test.mjs[65-123]
src/background/index.mjs[643-650]
src/content-script/menu-tools/index.mjs[19-33]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`openUrl()` currently catches all async tab-operation failures, logs them, and then fulfills with `undefined`. This prevents awaited callers from detecting whether the URL was actually opened/activated.

## Issue Context
The function is used in fire-and-forget call sites today, but the PR also changed `openUrl()` to return the full async flow, which makes it look awaitable for “did it work?” semantics.

## Fix Focus Areas
- src/utils/open-url.mjs[3-16]
- tests/unit/utils/open-url.test.mjs[24-123]

## Suggested fix
Change `openUrl()` to resolve an explicit status (e.g., `true` on success, `false` on any failure) while still logging internally. Update the unit tests to assert the boolean status for both success and failure cases.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---