Handle background fetch transport failures by PeterDaveHello · Pull Request #1014 · ChatGPTBox-dev/chatGPTBox

PeterDaveHello · 2026-07-18T22:11:43Z

Summary

Reject fetchBg when background messaging fails.
Reject errors thrown while reconstructing a fulfilled response.
Cover successful responses, returned background errors, transport failures, and response-processing failures.

Why

A rejected Browser.runtime.sendMessage() previously left fetchBg pending and leaked an unhandled rejection. Callers such as Bing conversation startup could wait indefinitely.

Validation

npm test (46 test files)
npm run test:coverage on Node.js 22 (36.23% 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.

Summary by CodeRabbit

Bug Fixes
- Improved error handling when background communication fails, ensuring errors are properly returned to the caller.
- Ensured errors encountered while processing background responses are consistently propagated.
Tests
- Added coverage for successful responses, background-reported errors, communication failures, and response-processing errors.

coderabbitai · 2026-07-18T22:12:04Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0058af78-04d5-42e7-bc9a-84942e505982

📥 Commits

Reviewing files that changed from the base of the PR and between fc33991 and b784c29.

📒 Files selected for processing (2)

src/utils/fetch-bg.mjs
tests/unit/utils/fetch-bg.test.mjs

🚧 Files skipped from review as they are similar to previous changes (2)

src/utils/fetch-bg.mjs
tests/unit/utils/fetch-bg.test.mjs

📝 Walkthrough

Walkthrough

fetchBg now forwards rejected background-message promises through its outer promise. New unit tests cover successful responses, payload construction, background errors, sendMessage failures, and response-processing exceptions.

Changes

fetchBg error propagation

Layer / File(s)	Summary
Promise rejection handling and test coverage `src/utils/fetch-bg.mjs`, `tests/unit/utils/fetch-bg.test.mjs`	`fetchBg` adds `.catch(reject)` to propagate message and response-processing failures. Tests validate successful responses, emitted `FETCH` payloads, background errors, thrown `sendMessage` errors, and iterator failures.

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

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: making background fetch reject on transport failures.
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.

✨ 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

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

qodo-code-review · 2026-07-18T22:12:16Z

PR Summary by Qodo

Reject fetchBg when background messaging or response reconstruction fails

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Propagate Browser.runtime.sendMessage transport failures to fetchBg callers.
• Reject fetchBg when response reconstruction throws, avoiding hung Promises.
• Add unit coverage for success, background errors, transport failures, and processing errors.

Diagram

sequenceDiagram
  participant C as Caller
  participant F as fetchBg
  participant R as Browser.runtime
  participant B as Background

  C->>F: fetchBg(input, init)
  F->>R: sendMessage({type:"FETCH"})
  R->>B: deliver FETCH request
  alt Background returns response tuple
    B-->>R: [response, null]
    R-->>F: resolve(messageResponse)
    F-->>C: Response(body, status, headers)
  else Background returns error tuple
    B-->>R: [null, error]
    R-->>F: resolve(messageResponse)
    F-->>C: reject(error)
  else Transport failure
    R-->>F: reject(sendMessage error)
    F-->>C: reject(sendMessage error)
  else Response processing failure
    R-->>F: resolve(malformed messageResponse)
    F-->>C: reject(processing error)
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Rewrite fetchBg as async/await (no manual new Promise)

➕ Automatically propagates sendMessage rejections without extra .catch wiring
➕ Simplifies control flow and reduces risk of missed rejection paths
➖ Slightly larger refactor than the minimal fix in this PR
➖ May require touch-up to linting/style expectations for async functions

2. Return the sendMessage promise chain directly

➕ Avoids the Promise constructor anti-pattern while keeping current .then logic
➕ Keeps behavior identical with less boilerplate
➖ Still uses .then callbacks (less readable than async/await for some teams)

Recommendation: The PR’s approach is a safe, minimal fix that closes the primary correctness gap (pending promise on sendMessage rejection) and adds strong regression coverage. If this area is likely to evolve, consider a follow-up refactor to async/await or returning the promise chain directly to reduce the chance of future missed rejection paths.

Files changed (2) +88 / -0

Bug fix (1) +1 / -0

fetch-bg.mjsPropagate runtime messaging failures to fetchBg callers +1/-0

Propagate runtime messaging failures to fetchBg callers

• Adds a catch handler so rejected Browser.runtime.sendMessage calls reject the fetchBg promise instead of leaving it pending. This prevents unhandled rejections and callers waiting indefinitely when the message channel fails.

src/utils/fetch-bg.mjs

Tests (1) +87 / -0

fetch-bg.test.mjsAdd unit tests for fetchBg success and all failure modes +87/-0

Add unit tests for fetchBg success and all failure modes

• Introduces tests that validate correct request payloads and Response reconstruction on success. Also covers rejection when the background returns an error, when sendMessage rejects, and when messageResponse processing throws.

tests/unit/utils/fetch-bg.test.mjs

gemini-code-assist

Code Review

This pull request adds a .catch(reject) block to fetchBg to prevent the promise from hanging when errors occur during response processing, and introduces a comprehensive unit test suite for fetchBg. The reviewer suggests handling cases where the message response is null or undefined more gracefully inside the .then block to provide a more descriptive error message instead of a generic TypeError.

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.

kilo-code-bot · 2026-07-18T22:12:51Z

Code Review Summary

Status: No Issues Found (incremental) | Recommendation: Merge

Files Reviewed (2 files)

src/utils/fetch-bg.mjs - unchanged in this increment (1 active external finding on line 30, carried forward from prior review)
tests/unit/utils/fetch-bg.test.mjs - 1 issue resolved

Incremental Notes

The only change since the previous review (fc33991a..b784c29) is in tests/unit/utils/fetch-bg.test.mjs (lines 52-59): the background-error test now mocks the error as a plain object { message: ... } and uses assert.deepEqual instead of asserting Error identity. This directly resolves the prior external finding that the test did not match the production-shaped error payload. No new issues were introduced.

The src/utils/fetch-bg.mjs change (the .catch(reject) line 30) was already part of the previous review and is unchanged here; the previously noted suggestion to reject with a descriptive error when messageResponse is null/undefined remains an outstanding external comment but is outside this increment's scope.

Previous Review Summary (commit fc33991)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit `fc33991`)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)

src/utils/fetch-bg.mjs - 0 issues
tests/unit/utils/fetch-bg.test.mjs - 0 issues

_{Reviewed by hy3:free · Input: 25.6K · Output: 2.7K · Cached: 239.6K}

coderabbitai

🧹 Nitpick comments (1)

src/utils/fetch-bg.mjs (1)

8-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider using async/await to avoid the Promise constructor anti-pattern.

While the .catch(reject) fix correctly resolves the unhandled rejection issue, explicitly wrapping an existing Promise chain in new Promise is generally considered an anti-pattern.

Since Browser.runtime.sendMessage already returns a Promise, you can significantly simplify this code by converting fetchBg to an async function. This allows you to use await, which natively forwards rejections (including the sendMessage rejection) and synchronous exceptions without needing explicit .catch() or .then() blocks.

♻️ Proposed refactoring using async/await

-export function fetchBg(input, init) {
-  return new Promise((resolve, reject) => {
-    Browser.runtime
-      .sendMessage({
-        type: 'FETCH',
-        data: { input, init },
-      })
-      .then((messageResponse) => {
-        const [response, error] = messageResponse
-        if (response === null) {
-          reject(error)
-        } else {
-          const body = response.body ? new Blob([response.body]) : undefined
-          resolve(
-            new Response(body, {
-              status: response.status,
-              statusText: response.statusText,
-              headers: new Headers(response.headers),
-            }),
-          )
-        }
-      })
-      .catch(reject)
-  })
-}
+export async function fetchBg(input, init) {
+  const messageResponse = await Browser.runtime.sendMessage({
+    type: 'FETCH',
+    data: { input, init },
+  })
+
+  const [response, error] = messageResponse
+  if (response === null) {
+    throw error
+  }
+
+  const body = response.body ? new Blob([response.body]) : undefined
+  return new Response(body, {
+    status: response.status,
+    statusText: response.statusText,
+    headers: new Headers(response.headers),
+  })
+}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/fetch-bg.mjs` around lines 8 - 32, Convert fetchBg to an async
function and remove the surrounding Promise constructor and .then/.catch chain.
Await Browser.runtime.sendMessage, preserve the existing response/error handling
and Response construction, and allow rejected promises or synchronous exceptions
to propagate naturally.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/utils/fetch-bg.mjs`:
- Around line 8-32: Convert fetchBg to an async function and remove the
surrounding Promise constructor and .then/.catch chain. Await
Browser.runtime.sendMessage, preserve the existing response/error handling and
Response construction, and allow rejected promises or synchronous exceptions to
propagate naturally.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dd39914f-54de-40ed-9892-13ca7360bf3a

📥 Commits

Reviewing files that changed from the base of the PR and between 2419486 and fc33991.

📒 Files selected for processing (2)

src/utils/fetch-bg.mjs
tests/unit/utils/fetch-bg.test.mjs

qodo-code-review · 2026-07-18T22:15:12Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. ~~Unrealistic background error test~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The unit test mocks the background returning an Error instance and asserts strict identity, but the
real background FETCH handler returns a plain object like {message: ...} across messaging. This
means the test doesn’t cover the production-shaped error payload and may miss regressions in how
fetchBg callers handle background failures.

Code

tests/unit/utils/fetch-bg.test.mjs[R52-59]

+test('fetchBg rejects with the error received from the background script', async (t) => {
+  const backgroundError = new Error('background fetch failed')
+  mockSendMessage(t, async () => [null, backgroundError])
+
+  await assert.rejects(fetchBg('https://example.com'), (error) => {
+    assert.equal(error, backgroundError)
+    return true
+  })

Evidence

The test currently expects an Error instance to be returned unchanged, but the actual background
FETCH failure path returns a plain object { message: ... }, so the test is not representative of
production messaging behavior.

tests/unit/utils/fetch-bg.test.mjs[52-59]
src/background/index.mjs[748-751]

Agent prompt

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

## Issue description
`fetchBg`’s unit test for “error received from the background script” uses an `Error` instance as the mocked error payload and asserts object identity. In production, the background handler returns plain serializable objects (e.g. `{ message: error.message }`), so this test doesn’t validate the real error shape that goes over `runtime.sendMessage`.

## Issue Context
The background `FETCH` handler returns `[null, { message: error.message }]` in its catch path; structured cloning over extension messaging generally won’t preserve `Error` instances.

## Fix Focus Areas
- tests/unit/utils/fetch-bg.test.mjs[52-59]

## Suggested change
Update the test to mock the background error as a plain object (matching the background handler), and assert on that shape (e.g. `deepEqual`) or assert on `error.message` if you prefer normalizing errors:

```js
const backgroundError = { message: 'background fetch failed' }
mockSendMessage(t, async () => [null, backgroundError])

await assert.rejects(fetchBg('https://example.com'), (error) => {
 assert.deepEqual(error, backgroundError) // or assert.equal(error?.message, backgroundError.message)
 return true
})
```

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

Previous review results

Review updated until commit b784c29

Results up to commit fc33991

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

1. ~~Unrealistic background error test~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The unit test mocks the background returning an Error instance and asserts strict identity, but the
real background FETCH handler returns a plain object like {message: ...} across messaging. This
means the test doesn’t cover the production-shaped error payload and may miss regressions in how
fetchBg callers handle background failures.

Code

tests/unit/utils/fetch-bg.test.mjs[R52-59]

+test('fetchBg rejects with the error received from the background script', async (t) => {
+  const backgroundError = new Error('background fetch failed')
+  mockSendMessage(t, async () => [null, backgroundError])
+
+  await assert.rejects(fetchBg('https://example.com'), (error) => {
+    assert.equal(error, backgroundError)
+    return true
+  })

Evidence

The test currently expects an Error instance to be returned unchanged, but the actual background
FETCH failure path returns a plain object { message: ... }, so the test is not representative of
production messaging behavior.

tests/unit/utils/fetch-bg.test.mjs[52-59]
src/background/index.mjs[748-751]

Agent prompt

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

## Issue description
`fetchBg`’s unit test for “error received from the background script” uses an `Error` instance as the mocked error payload and asserts object identity. In production, the background handler returns plain serializable objects (e.g. `{ message: error.message }`), so this test doesn’t validate the real error shape that goes over `runtime.sendMessage`.

## Issue Context
The background `FETCH` handler returns `[null, { message: error.message }]` in its catch path; structured cloning over extension messaging generally won’t preserve `Error` instances.

## Fix Focus Areas
- tests/unit/utils/fetch-bg.test.mjs[52-59]

## Suggested change
Update the test to mock the background error as a plain object (matching the background handler), and assert on that shape (e.g. `deepEqual`) or assert on `error.message` if you prefer normalizing errors:

```js
const backgroundError = { message: 'background fetch failed' }
mockSendMessage(t, async () => [null, backgroundError])

await assert.rejects(fetchBg('https://example.com'), (error) => {
 assert.deepEqual(error, backgroundError) // or assert.equal(error?.message, backgroundError.message)
 return true
})
```

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

Reject background messaging and response-processing failures so callers do not remain pending when a request cannot complete. Add focused coverage for successful responses and both error paths.

coderabbitai · 2026-07-19T08:19:17Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot

Pull request overview

This PR fixes fetchBg so it reliably rejects when the background messaging transport fails (e.g., Browser.runtime.sendMessage() rejects) or when response reconstruction throws, preventing callers from hanging indefinitely in extension flows like Bing conversation startup.

Changes:

Ensure fetchBg rejects on sendMessage transport failures by forwarding promise rejections.
Ensure fetchBg rejects if response unpacking/reconstruction throws.
Add unit tests covering success, background-returned error, transport failure, and response-processing failure scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`src/utils/fetch-bg.mjs`	Propagates background transport/reconstruction failures by catching and rejecting the outer `fetchBg` promise.
`tests/unit/utils/fetch-bg.test.mjs`	Adds targeted tests for success, background error, sendMessage rejection, and response-processing exceptions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

qodo-code-review · 2026-07-19T08:21:53Z

Code review by qodo was updated up to the latest commit b784c29

gemini-code-assist Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread src/utils/fetch-bg.mjs

coderabbitai Bot reviewed Jul 18, 2026

View reviewed changes

qodo-code-review Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread tests/unit/utils/fetch-bg.test.mjs

Handle background fetch transport failures

b784c29

Reject background messaging and response-processing failures so callers do not remain pending when a request cannot complete. Add focused coverage for successful responses and both error paths.

PeterDaveHello force-pushed the fix-fetch-bg-rejection branch from fc33991 to b784c29 Compare July 18, 2026 22:24

PeterDaveHello requested a review from Copilot July 19, 2026 08:19

Copilot started reviewing on behalf of PeterDaveHello July 19, 2026 08:19 View session

Copilot AI reviewed Jul 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Handle background fetch transport failures#1014

Handle background fetch transport failures#1014
PeterDaveHello wants to merge 1 commit into
ChatGPTBox-dev:masterfrom
PeterDaveHello:fix-fetch-bg-rejection

PeterDaveHello commented Jul 18, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

qodo-code-review Bot commented Jul 18, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

kilo-code-bot Bot commented Jul 18, 2026 •

edited

Loading

Previous review (commit `fc33991`)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

qodo-code-review Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

qodo-code-review Bot commented Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

PeterDaveHello commented Jul 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Validation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

qodo-code-review Bot commented Jul 18, 2026

PR Summary by Qodo

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

kilo-code-bot Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Incremental Notes

Previous review (commit fc33991)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

qodo-code-review Bot commented Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PeterDaveHello commented Jul 18, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

kilo-code-bot Bot commented Jul 18, 2026 •

edited

Loading

Previous review (commit `fc33991`)

qodo-code-review Bot commented Jul 18, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading