Skip to content

Don't crash when window.onerror is a Firefox Restricted handler (#839) - #1488

Draft
devtools-agent[bot] wants to merge 2 commits into
masterfrom
fix/839-restricted-onerror
Draft

devtools-agent[bot] wants to merge 2 commits into
masterfrom
fix/839-restricted-onerror

Conversation

@devtools-agent

@devtools-agent devtools-agent Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Fixes #839. Linear: SDK-697.

Problem

In Firefox, if a browser extension has installed its own window.onerror (reported with axe DevTools, the Code Climate extension, and content scripts that initialize Sentry), the page doesn't get a normal function back. It gets an opaque Restricted {} object, and any property read on it throws Permission denied to access property "...".

captureUncaughtExceptions in src/browser/globalSetup.js walks the handler chain with an unguarded window.onerror._rollbarOldOnError. So with captureUncaught: true, new Rollbar() / Rollbar.init() threw:

Error: Permission denied to access property "_rollbarOldOnError"

In bundled apps (Next.js etc.) that exception escapes app bootstrap and the page renders blank. Reported on wise.com, gog.com, app.circleci.com and DatoCMS. The <script> snippet mostly avoided this because shim.js wraps setup in _wrapInternalErr. The npm entry point (src/browser/core.js) had no such guard.

Fix

  • src/browser/globalSetup.js
    • New _unwrapOnError() walks the _rollbarOldOnError chain with a guarded read. If a handler can't be read, it can't have been installed by Rollbar, so it's treated as the end of the chain. Rollbar still chains to it rather than silently dropping the extension's handler.
    • The previous handler is now called with Function.prototype.apply.call(old, window, args) instead of old.apply(window, args). old.apply is itself a property read, so it would throw on the same object every time an error happened.
  • src/browser/core.js: setupUnhandledCapture() runs each hooking step (captureUncaughtExceptions, wrapGlobals, captureUnhandledRejections) through _hookGlobals(), which catches and logs [Rollbar]: Internal error, matching the snippet shim. If hooking page globals fails for any reason, the page keeps working instead of the constructor throwing. Each step is guarded on its own, so one failure doesn't skip the others. The *Initialized flags are set before the hooks run, so a later configure() doesn't rerun a half-finished setup (that would chain a second Rollbar onerror and stack more addEventListener wrappers).
  • _rollbarWindowOnError updates anonymousErrorsPending in a finally, so a chained handler that throws can't skip it. That covers a Restricted handler Firefox won't let the page call. The handler's own error still propagates, so real bugs in a page's onerror stay visible.

Repro steps

Real browser (from the issue and its comments):

  1. In Firefox, install axe DevTools (or the Code Climate extension).
  2. Open a page whose bundle calls new Rollbar({ captureUncaught: true, ... }), e.g. an app using the npm package.
  3. Before this fix: the console shows [Rollbar]: Internal error Error: "Permission denied to access property "_rollbarOldOnError"" or an uncaught Permission denied..., and in bundled apps the page is blank. Disabling the extension or using a private window makes it go away.

In the test suite (no Firefox or extension needed): the tests model the Restricted {} handler as a callable Proxy whose get trap throws Permission denied to access property "<prop>". That's exactly how the object behaves from the page's side: callable, but every property read throws.

Tests

  • test/browser.globalSetup.test.ts (new): unit tests for captureUncaughtExceptions:
    • it doesn't throw when window.onerror is Restricted
    • it still reports the error, and chains to the extension's handler with the right args and this
    • it stops walking the chain at a Restricted link sitting behind a Rollbar-installed handler
    • regression check: a previously installed Rollbar handler still unwraps to the original onerror
    • Rollbar's bookkeeping still runs when the chained handler throws, and that error still propagates
  • test/browser.core.test.ts:
    • new Rollbar({ captureUncaught: true }) with a Restricted window.onerror doesn't throw, captures a real uncaught error from the fixture page, and calls the extension's handler
    • a failure inside global hooking (a throwing wrapGlobals component) doesn't propagate out of the constructor, doesn't skip unhandled-rejection capture, and isn't retried by a later configure() (wrapGlobals runs once, window.onerror isn't replaced again)

Proof the tests catch the bug:

  • Without the globalSetup.js change, the 4 Restricted-handler tests fail. The constructor throws Permission denied to access property "_rollbarOldOnError", or, with only the core guard in place, errors are no longer captured.
  • Without the core.js change, the constructor-guard test fails. Against the first revision of this PR (single try/catch), it fails because rejection capture is skipped.
  • Without the finally in globalSetup.js, the chained-handler-throws test fails (anonymousErrorsPending stays 0).
  • With both changes, everything passes.

Validation

  • npm run test:wtr: 53 files, 628 passed (Chromium)
  • npm run test:server: 149 passing
  • npx eslint and prettier --check on the changed files: clean
  • npm run typecheck, npm run typecheck:tests: clean
  • npm run build && npm run validate:es5: ES5 check passes. dist/ isn't committed; it's regenerated at release.

Caveat

I couldn't verify this in real Firefox with the extension installed. The fix doesn't depend on that: every property read on the handler is now guarded. The one open question is whether Firefox lets page script call a Restricted handler. If it doesn't, that call throws out of Rollbar's onerror after the item has already been reported and after Rollbar's own bookkeeping, the same as any previous handler throwing. It doesn't block init or reporting.

🤖 Generated with Claude Code

In Firefox, an onerror handler installed by a browser extension (axe
DevTools, Code Climate, content scripts using Sentry, ...) is exposed to
the page as an opaque `Restricted {}` object, and every property read on
it throws "Permission denied to access property". captureUncaughtExceptions
read `window.onerror._rollbarOldOnError` unguarded, so `new Rollbar()` /
`Rollbar.init()` threw and took the host page down with it.

- Walk the `_rollbarOldOnError` chain with a guarded read and treat an
  unreadable handler as the end of the chain.
- Invoke the previous handler with Function.prototype.apply.call rather
  than `old.apply`, which is itself a property read on that object.
- Guard setupUnhandledCapture in the npm/core entry point the same way the
  snippet shim already does, so a failure while hooking globals is logged
  as an internal error instead of thrown from the constructor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@rollbar-circleci-machine rollbar-circleci-machine left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Agent Review (openai, openai-astra)

Summary

This PR fixes #839: in Firefox, a browser extension can install an onerror handler that the page sees as a Restricted {} object, and reading any property from it throws. The PR adds _unwrapOnError(), which stops walking the _rollbarOldOnError chain when a property read throws. It also calls the old handler with Function.prototype.apply.call(old, …) so it never reads old.apply, and wraps setupUnhandledCapture() in try/catch so new Rollbar() / configure() can't throw while hooking page globals.

What I checked

  • globalSetup.js: the new unwrap loop matches the old while loop for normal chains. The shim path (shim.js:57-58 sets handler._rollbarOldOnError = window.onerror directly) and the core path (core.js:226, no shim flag) both end at the Restricted handler without reading any property from it. typeof and truthiness checks on that handler don't read properties either.
  • Function.prototype.apply.call is ES5-safe. For a non-callable old it fails the same way old.apply did before, so nothing regresses.
  • Tests: test/browser.globalSetup.test.ts matches the WTR globs (web-test-runner.config.js:5-7), and tsconfig.test.json (non-strict, checkJs: false) covers it. The Proxy stand-in only traps get, so [[Call]] passes through to the spy, as the tests expect. Each new core test restores window.onerror / components in a finally.

One issue (below): the new try/catch covers the whole setup routine at once. If wrapGlobals throws (the exact case the new core test simulates), the initialized flag is never set, so every later configure() repeats the partial setup. Unhandled-rejection capture is also silently skipped.

Not a finding, worth a thought: the tests run only in Chromium and use a Proxy as the stand-in, so they don't show that Firefox's real Restricted wrapper can actually be called from the page. If calling it throws ("Permission denied to access object"), globalSetup.js:71 would throw out of Rollbar's onerror after the item has been logged. A try/catch around the chained call would make this code safe either way.

No tests were run as part of this review.

Comment thread src/browser/core.js Outdated
Addresses review feedback on #1488.

- setupUnhandledCapture now guards each hooking step on its own and sets
  the *Initialized flags before running them. A failure in wrapGlobals no
  longer skips unhandled-rejection capture, and a later configure() no
  longer reruns a half-finished setup, which stacked a second onerror
  handler and more addEventListener wrappers and re-logged the error.
- _rollbarWindowOnError updates anonymousErrorsPending in a `finally`, so
  a chained handler that throws (including a Restricted one Firefox won't
  let the page call) can't skip it. The handler's own error still
  propagates rather than being swallowed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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 - Extensions with errors attempt to call Rollbar on page

2 participants