Skip to content

Add global error listeners for uncaught errors on special pages#2442

Draft
noisysocks wants to merge 2 commits intomainfrom
worktree-error-reporting
Draft

Add global error listeners for uncaught errors on special pages#2442
noisysocks wants to merge 2 commits intomainfrom
worktree-error-reporting

Conversation

@noisysocks
Copy link
Contributor

@noisysocks noisysocks commented Mar 12, 2026

Asana Task/Github Issue: https://app.asana.com/1/137249556945/project/1209121419454298/task/1213433496794376?focus=true

Description

Registers window.addEventListener('error') and window.addEventListener('unhandledrejection') listeners on three special pages — onboarding, new-tab, and history — to catch uncaught JavaScript errors that fall outside Preact's ErrorBoundary (event handlers, async callbacks, timers, observers, etc.). Caught errors are reported to native via reportInitException with [uncaught] or [unhandledrejection] prefixes so native can distinguish these from actual init failures.

Key details:

  • Listeners are registered immediately after the messaging instance is created, before init() is called
  • New Tab's reportInitException takes a bare string (matching its existing API); onboarding and history take { message }
  • ErrorBoundary-caught render errors go through reportPageException — no double-reporting
  • Added reportInitException: {} to onboarding's mock default responses to prevent infinite error loops on Windows

Manual verification

Deliberate throw new Error(...) statements were inserted into real Preact callbacks across all three pages, rebuilt with npm run build.dev, and tested in a headless Chromium browser to confirm the global listeners catch errors and call reportInitException.

What the global listeners catch

Page Error location Error type Caught?
History SearchForm.input() — onInput handler Preact onInput callback Yes
History useButtonClickHandler — document click listener Raw DOM event handler Yes
Onboarding WelcomeContent.complete() — setTimeout inside useEffect setTimeout callback Yes
All pages setTimeout(() => { throw ... }) Async throw Yes
All pages Promise.reject(new Error(...)) Unhandled rejection Yes

In every case, reportInitException was called (confirmed via mock transport logging "unhandled notification") and the page continued to render normally — errors did not break the UI.

What they don't catch (expected)

Page Error location Error type Caught?
New Tab ThemeSection onClick Preact onClick handler No
New Tab PlusIconWrapper onClick Preact onClick handler No

Preact catches errors thrown inside onClick handlers via its internal error recovery (options.__e) and does not propagate them to window.onerror. These errors would instead be caught by Preact's ErrorBoundary and reported via reportPageException — so no gap in coverage exists, just a different reporting path.

False positive check

All three pages were loaded and interacted with normally. Zero reportInitException calls were observed during regular operation (confirmed by checking console output for "unhandled notification" messages).

Testing Steps

npm run test-int -- --grep "global error listeners"

Checklist

  • I have tested this change locally
  • I have tested this change locally in all supported browsers
  • This change will be visible to users
  • I have added automated tests that cover this change
  • I have ensured the change is gated by config
  • This change was covered by a ship review
  • This change was covered by a tech design
  • Any dependent config has been merged

🤖 Generated with Claude Code

Register window 'error' and 'unhandledrejection' listeners on
onboarding, new-tab, and history pages to catch JS errors that fall
outside Preact's ErrorBoundary (event handlers, async callbacks,
timers, observers, etc.). Caught errors are reported to native via
reportInitException with [uncaught] or [unhandledrejection] prefixes.

Also adds integration tests for each page verifying the listeners
catch errors and produce no false positives during normal operation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 12, 2026

Build Branch

Branch pr-releases/worktree-error-reporting
Commit 406cdf632c
Updated March 12, 2026 at 10:09:22 AM UTC

Static preview entry points

QR codes (mobile preview)
Entry point QR code
Docs QR for docs preview
Static pages QR for static pages preview
Integration pages QR for integration pages preview

Integration commands

npm (Android / Extension):

npm i github:duckduckgo/content-scope-scripts#pr-releases/worktree-error-reporting

Swift Package Manager (Apple):

.package(url: "https://github.com/duckduckgo/content-scope-scripts.git", branch: "pr-releases/worktree-error-reporting")

git submodule (Windows):

git -C submodules/content-scope-scripts fetch origin pr-releases/worktree-error-reporting
git -C submodules/content-scope-scripts checkout origin/pr-releases/worktree-error-reporting
Pin to exact commit

npm (Android / Extension):

npm i github:duckduckgo/content-scope-scripts#406cdf632cbb81fd823cd2feea060122d4dccdaa

Swift Package Manager (Apple):

.package(url: "https://github.com/duckduckgo/content-scope-scripts.git", revision: "406cdf632cbb81fd823cd2feea060122d4dccdaa")

git submodule (Windows):

git -C submodules/content-scope-scripts fetch origin pr-releases/worktree-error-reporting
git -C submodules/content-scope-scripts checkout 406cdf632cbb81fd823cd2feea060122d4dccdaa

@github-actions github-actions bot added the semver-patch Bug fix / internal — no release needed label Mar 12, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 12, 2026

[Beta] Generated file diff

Time updated: Thu, 12 Mar 2026 10:09:51 GMT

Apple
    - apple/pages/history/dist/index.js
  • apple/pages/new-tab/dist/index.js
  • apple/pages/onboarding/dist/index.js

File has changed

Integration
    - integration/pages/history/dist/index.js
  • integration/pages/new-tab/dist/index.js
  • integration/pages/onboarding/dist/index.js

File has changed

Windows
    - windows/pages/history/dist/index.js
  • windows/pages/new-tab/dist/index.js
  • windows/pages/onboarding/dist/index.js

File has changed

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Web Compatibility Assessment

No compatibility regressions found in this diff.

  • special-pages/pages/history/src/index.js (line range 86-94), severity info: Global error/unhandledrejection listeners are registered before init(), and payload shape sent to reportInitException({ message }) matches existing API contract.
  • special-pages/pages/new-tab/src/index.js (line range 113-121), severity info: Listener behavior and contract (reportInitException(string)) are consistent with the existing New Tab wrapper method.
  • special-pages/pages/onboarding/app/index.js (line range 40-48), severity info: Listener behavior and contract (reportInitException({ message })) are consistent with onboarding messaging.
  • special-pages/pages/*/integration-tests/* (new global listener suites), severity info: Added positive and false-positive coverage for uncaught errors/unhandled rejections; this reduces rollback risk.

Security Assessment

No security vulnerabilities identified in the changed code.

  • special-pages/pages/history/src/index.js (line range 86-94), severity info: Outgoing notify payload is bounded to { message: string }; no nativeData propagation path introduced.
  • special-pages/pages/new-tab/src/index.js (line range 113-121), severity info: No trust-boundary relaxation, no new cross-origin messaging, no dynamic code execution.
  • special-pages/pages/onboarding/app/index.js (line range 40-48), severity info: Listener-only instrumentation; no new external fetch/postMessage/eval surface.

Risk Level

Medium Risk — runtime behavior changed in three special-page entrypoints by adding global listeners that emit new native notifications, but changes are localized and covered by integration tests.

Recommendations

  1. Add a robustness test for unhandledrejection where event.reason is a non-Error object with a throwing coercion path (e.g., custom toString/Symbol.toPrimitive), and harden message extraction with local try/catch.
  2. Consider lightweight de-duplication/rate limiting for repeated identical global errors to avoid noisy native telemetry if a timer-driven fault loops.
  3. Keep the onboarding mock default for reportInitException (already added) to prevent test-environment feedback loops when notification handlers are absent.

Validation run: npm run test-int -- --grep "global error listeners" --reporter list (12 passed).

Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

@github-actions
Copy link
Contributor

⚠️ Cursor review was not successful.

This PR requires a manual review and approval from a member of one of the following teams:

  • @duckduckgo/content-scope-scripts-owners
  • @duckduckgo/apple-devs
  • @duckduckgo/android-devs
  • @duckduckgo/team-windows-development
  • @duckduckgo/extension-owners
  • @duckduckgo/config-aor
  • @duckduckgo/breakage-aor
  • @duckduckgo/breakage

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

Labels

semver-patch Bug fix / internal — no release needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant