Skip to content

fix: ignore errors from browser extensions in error handler and editor init#7456

Merged
JohnMcLear merged 1 commit intoether:developfrom
JohnMcLear:fix/ignore-extension-errors-6802
Apr 5, 2026
Merged

fix: ignore errors from browser extensions in error handler and editor init#7456
JohnMcLear merged 1 commit intoether:developfrom
JohnMcLear:fix/ignore-extension-errors-6802

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Summary

Browser extensions (BitWarden, Dashlane, password managers, FIDO2 authenticators) inject scripts into pages that can throw errors. Etherpad's error handling catches these unrelated errors and either shows a scary error popup or blocks the editor from loading entirely.

Two fixes:

globalExceptionHandler (pad_utils.ts): Errors where the source URL matches moz-extension://, chrome-extension://, or safari-extension:// are silently ignored instead of showing a gritter error popup.

Ace2Editor.init (ace.ts): The eventFired() error callback checks if the error event's target src is a browser extension URL. If so, it ignores the error instead of rejecting the promise and killing editor initialization.

Root Cause

Extensions inject <script> elements that can trigger error events on the iframe during load, and throw JS errors caught by window.addEventListener('error'). Neither of these are Etherpad bugs, but the handler treated all errors equally.

Test plan

  • Type check passes
  • Backend tests pass (744/744)
  • Manual: install BitWarden + Dashlane extensions, log into BitWarden, load a pad → should load without error popup

Fixes #6802

🤖 Generated with Claude Code

…r init

Browser extensions (BitWarden, Dashlane, etc.) inject scripts that can
throw errors caught by Etherpad's global exception handler, showing a
scary error popup and sometimes blocking the editor from loading.

Two fixes:
- globalExceptionHandler (pad_utils.ts): Skip errors where the source
  URL matches moz-extension://, chrome-extension://, or
  safari-extension:// patterns.
- Ace2Editor.init (ace.ts): The eventFired() error callback now checks
  if the error event's target src is a browser extension and ignores
  it, preventing extension-injected script failures from killing
  editor initialization.

Fixes ether#6802

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@JohnMcLear
Copy link
Copy Markdown
Member Author

/review

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Apr 4, 2026

Code Review by Qodo

Grey Divider

New Review Started

This review has been superseded by a new analysis

Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Ignore browser extension errors in error handlers

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Ignore errors from browser extension scripts in global exception handler
• Skip extension errors in Ace2Editor initialization to prevent blocking
• Detect extension URLs using moz-extension, chrome-extension, safari-extension patterns
• Prevents scary error popups and editor loading failures from unrelated extension code
Diagram
flowchart LR
  A["Browser Extension<br/>Injects Script"] -->|"Throws Error"| B["Global Exception<br/>Handler"]
  A -->|"Error Event"| C["Ace2Editor.init<br/>eventFired"]
  B -->|"Check URL Pattern"| D{"Extension URL?"}
  C -->|"Check URL Pattern"| E{"Extension URL?"}
  D -->|"Yes"| F["Silently Ignore"]
  D -->|"No"| G["Show Error Popup"]
  E -->|"Yes"| H["Continue Init"]
  E -->|"No"| I["Reject Promise"]
Loading

Grey Divider

File Changes

1. src/static/js/ace.ts 🐞 Bug fix +9/-1

Skip extension errors in editor initialization

• Modified errorCb callback in eventFired function to accept error event parameter
• Added regex pattern check to detect browser extension URLs (moz-extension, chrome-extension,
 safari-extension)
• Silently returns on extension errors instead of rejecting promise and blocking editor
 initialization
• Added debug logging for ignored extension errors

src/static/js/ace.ts


2. src/static/js/pad_utils.ts 🐞 Bug fix +7/-0

Ignore extension errors in global exception handler

• Added regex pattern check in global exception handler to detect browser extension URLs
• Extracts source URL from error object or stack trace for pattern matching
• Returns early to silently ignore extension errors instead of showing error popup
• Added debug comments referencing issue #6802

src/static/js/pad_utils.ts


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Apr 4, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Action required

1. Missing regression test 📘 Rule violation ☼ Reliability
Description
This PR changes production error-handling behavior to ignore browser-extension errors but does not
add/update an automated regression test to ensure the bug cannot be reintroduced. Without a test, a
future refactor could revert this behavior and CI would not catch it.
Code

src/static/js/ace.ts[R53-64]

+    const errorCb = (evt) => {
+      // Ignore error events from browser extension scripts — they are unrelated
+      // to Etherpad and should not block editor initialization.
+      // See https://github.com/ether/etherpad-lite/issues/6802
+      const src = evt?.target?.src || evt?.filename || '';
+      if (/^(moz|chrome|safari)-extension:\/\//.test(src)) {
+        debugLog('Ace2Editor.init() ignoring error from browser extension:', src);
+        return;
+      }
      const err = new Error(`Ace2Editor.init() error event while waiting for ${event} event`);
      debugLog(`${err} on object`, obj);
      cleanup();
Evidence
PR Compliance ID 3 requires an automated regression test for bug fixes. The diff shows only
production-code changes (ignoring extension-originated errors in Ace2Editor.init() and
setupGlobalExceptionHandler) with no accompanying test additions/updates in the provided PR
changes.

src/static/js/ace.ts[53-66]
src/static/js/pad_utils.ts[399-425]
Best Practice: Repository guidelines

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

## Issue description
A bug fix was made to ignore error events/exceptions originating from browser extensions, but there is no automated test that would fail if this behavior were reverted.

## Issue Context
The change prevents extension-injected script errors (e.g., `chrome-extension://`, `moz-extension://`, `safari-extension://`) from blocking editor/pad loading.

## Fix Focus Areas
- src/static/js/ace.ts[53-66]
- src/static/js/pad_utils.ts[399-425]

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



Remediation recommended

2. Extension rejections not ignored 🐞 Bug ≡ Correctness
Description
PadUtils.setupGlobalExceptionHandler() computes source = url || err.stack, but in the
PromiseRejectionEvent path url defaults to the truthy string 'unknown', so err.stack is never
checked and extension-originated unhandled rejections can still trigger the gritter popup and
/jserror reporting.
Code

src/static/js/pad_utils.ts[R423-425]

+        const source = url || err.stack || '';
+        if (/^(moz|chrome|safari)-extension:\/\//.test(source)) return;
+
Evidence
In the PromiseRejectionEvent branch, url is assigned a default value of 'unknown' when
err.fileName is missing, making url always truthy. The new extension ignore check uses `url ||
err.stack, which will therefore choose 'unknown' and never evaluate err.stack`, so extension
scheme URLs present only in the stack cannot be filtered out.

src/static/js/pad_utils.ts[409-425]

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

### Issue description
`setupGlobalExceptionHandler()` uses `const source = url || err.stack || ''` for extension detection. Because the PromiseRejectionEvent path defaults `url` to `'unknown'`, the stack is never inspected when `fileName` is absent, so extension-originated unhandled rejections can still show user popups and get reported.

### Issue Context
This impacts the PR goal of ignoring extension errors for both `error` and `unhandledrejection` handlers.

### Fix Focus Areas
- src/static/js/pad_utils.ts[409-425]

### Suggested change
- Treat placeholder values like `'unknown'` as empty for filtering purposes.
- Check both `url` and `err.stack` independently (or concatenate them) and run the extension-scheme regex against both.
- Example approach:
 - `const urlForFilter = (url && url !== 'unknown') ? url : '';`
 - `const stackForFilter = (typeof err?.stack === 'string') ? err.stack : '';`
 - `if (/(moz|chrome|safari)-extension:\/\//.test(urlForFilter) || /(moz|chrome|safari)-extension:\/\//.test(stackForFilter)) return;`

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



Advisory comments

3. Ignored init error may hang 🐞 Bug ☼ Reliability
Description
In ace.ts eventFired(), extension-originated error events return early without rejecting or
resolving, so if the awaited readiness event never arrives the Promise can remain pending
indefinitely (reducing fail-fast behavior compared to before).
Code

src/static/js/ace.ts[R53-61]

+    const errorCb = (evt) => {
+      // Ignore error events from browser extension scripts — they are unrelated
+      // to Etherpad and should not block editor initialization.
+      // See https://github.com/ether/etherpad-lite/issues/6802
+      const src = evt?.target?.src || evt?.filename || '';
+      if (/^(moz|chrome|safari)-extension:\/\//.test(src)) {
+        debugLog('Ace2Editor.init() ignoring error from browser extension:', src);
+        return;
+      }
Evidence
eventFired() wraps an internal Promise that only settles via resolve() in successCb or
reject(err) in errorCb. The new early return on extension URLs does neither, so the Promise
remains pending until (and unless) a later success event occurs.

src/static/js/ace.ts[40-76]

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

### Issue description
When an extension error event is ignored, the `eventFired()` Promise keeps waiting. If the expected load/ready event never fires, editor init can hang with no rejection.

### Issue Context
This is not necessarily common, but the early return changes behavior from "fail fast on any error event" to "possibly wait forever" in scenarios where only ignored errors occur.

### Fix Focus Areas
- src/static/js/ace.ts[40-76]

### Suggested change
Add a timeout path for `eventFired()` (or higher-level `frameReady()`) that rejects after a reasonable duration and cleans up listeners, so initialization cannot hang indefinitely.

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread src/static/js/ace.ts
Comment on lines +53 to 64
const errorCb = (evt) => {
// Ignore error events from browser extension scripts — they are unrelated
// to Etherpad and should not block editor initialization.
// See https://github.com/ether/etherpad-lite/issues/6802
const src = evt?.target?.src || evt?.filename || '';
if (/^(moz|chrome|safari)-extension:\/\//.test(src)) {
debugLog('Ace2Editor.init() ignoring error from browser extension:', src);
return;
}
const err = new Error(`Ace2Editor.init() error event while waiting for ${event} event`);
debugLog(`${err} on object`, obj);
cleanup();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Missing regression test 📘 Rule violation ☼ Reliability

This PR changes production error-handling behavior to ignore browser-extension errors but does not
add/update an automated regression test to ensure the bug cannot be reintroduced. Without a test, a
future refactor could revert this behavior and CI would not catch it.
Agent Prompt
## Issue description
A bug fix was made to ignore error events/exceptions originating from browser extensions, but there is no automated test that would fail if this behavior were reverted.

## Issue Context
The change prevents extension-injected script errors (e.g., `chrome-extension://`, `moz-extension://`, `safari-extension://`) from blocking editor/pad loading.

## Fix Focus Areas
- src/static/js/ace.ts[53-66]
- src/static/js/pad_utils.ts[399-425]

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Won't fix — browser extension errors are inherently untestable in Playwright (no real extensions in headless browsers). The regex filter is simple and defensive.

@JohnMcLear JohnMcLear merged commit 93c1791 into ether:develop Apr 5, 2026
26 checks passed
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.

globalExceptionHandler catching unrelated errors/creating bogus messages around Ace2Editor.init()

1 participant