fix: suppress internal error details from users in production mode

[JohnMcLear]

Summary

• pad_utils.ts (globalExceptionHandler): In production mode (window.clientVars.mode === 'production'), the gritter error popup now shows only a generic reload message with the ErrorId. Error messages, file paths, line numbers, stack traces, and user agent strings are hidden. In development mode, full details are still shown.
• basic_error_handler.ts (page load errors): No longer renders error details, stack traces, or file paths in the DOM. Logs them to console.error instead. This handler runs before clientVars is available so it always uses the generic message.
• Server-side logging unchanged: The $.post('../jserror', ...) still sends full error details to the server's jserror endpoint for server-side logging.

What users see

Production (before):

An error occurred
TypeError: b.lineAttributes.list is undefined
at https://example.com/padbootstrap-sxsSWavtLWA.min.js at line 20
ErrorId: vYIhQJ3X8o7VeqsTlv2t
Uncaught exception
URL: https://example.com/p/test
UserAgent: Mozilla/5.0 ...

Production (after):

An error occurred
Please press and hold Ctrl and press F5 to reload this page
If the problem persists, please contact your webmaster.
ErrorId: vYIhQJ3X8o7VeqsTlv2t

The ErrorId is preserved so users can reference it when contacting their admin, and admins can correlate it with server logs.

Test plan

• Backend tests pass (10/10)
• Manual (production): Trigger a JS error → should see generic message with ErrorId only
• Manual (development): Trigger a JS error → should see full error details as before
• Manual (page load error): Break a JS file → should see generic message, details in console

Fixes #5765

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Suppress internal error details from users in production mode

🐞 Bug fix

Walkthroughs

Description

• Hide sensitive error details from users in production mode
• Show only generic reload message with ErrorId for correlation
• Log full error details to console instead of DOM display
• Preserve server-side error logging for admin debugging

Diagram

flowchart LR
  A["Error Occurs"] --> B{Production Mode?}
  B -->|Yes| C["Show Generic Message + ErrorId"]
  B -->|No| D["Show Full Error Details"]
  C --> E["Log Details to Console"]
  D --> E
  E --> F["Send to Server jserror Endpoint"]

Loading

File Changes

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

Hide page load error details from users

• Replaced detailed error display with generic reload message
• Removed DOM rendering of error messages, file paths, and stack traces
• Added console.error logging for debugging without user exposure
• Maintains original error handler chain

src/static/js/basic_error_handler.ts

---

2. src/static/js/pad_utils.ts 🐞 Bug fix +30/-14

Conditionally suppress error details by environment

• Added production mode detection via window.clientVars.mode
• Conditional error message rendering based on environment
• Production: shows only generic reload message with ErrorId
• Development: preserves full error details including URL, line number, user agent

src/static/js/pad_utils.ts

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

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

1. isProduction error UI untested 📘 Rule violation ☼ Reliability

Description

The PR changes user-facing error sanitization behavior but does not include an automated regression
test to ensure the sanitized message remains enforced. Reverting these changes would not be caught
by CI, risking re-exposure of internal error details.

Code

src/static/js/pad_utils.ts[R428-457]

+          // In production mode, hide internal error details (file paths, line numbers,
+          // error messages) from end users. Only show a generic reload message.
+          // See https://github.com/ether/etherpad-lite/issues/5765
+          const isProduction = (window as any).clientVars?.mode === 'production';
+
+          const errorMsg = isProduction
+            ? [
+              $('<p>')
+                .append($('<b>').text('Please press and hold Ctrl and press F5 to reload this page')),
+              $('<p>')
+                .text('If the problem persists, please contact your webmaster.')
+                .append($('<br>'))
+                .append($('<span>').css('font-size', '.8em').text(`ErrorId: ${errorId}`)),
+            ]
+            : (() => {
+              const txt = document.createTextNode.bind(document);
+              return [
+                $('<p>')
+                  .append($('<b>').text('Please press and hold Ctrl and press F5 to reload this page')),
+                $('<p>')
+                  .text('If the problem persists, please send this error message to your webmaster:'),
+                $('<div>').css('text-align', 'left').css('font-size', '.8em').css('margin-top', '1em')
+                  .append($('<b>').addClass('error-msg').text(msg)).append($('<br>'))
+                  .append(txt(`at ${url} at line ${linenumber}`)).append($('<br>'))
+                  .append(txt(`ErrorId: ${errorId}`)).append($('<br>'))
+                  .append(txt(type)).append($('<br>'))
+                  .append(txt(`URL: ${window.location.href}`)).append($('<br>'))
+                  .append(txt(`UserAgent: ${navigator.userAgent}`)).append($('<br>')),
+              ];
+            })();

Evidence

PR Compliance ID 2 requires a regression test for bug fixes. The diff shows behavior changes that
suppress error details in production mode and during page load errors, but no accompanying test
changes are present in the PR diff.

src/static/js/pad_utils.ts[428-457]
src/static/js/basic_error_handler.ts[26-34]
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 that suppresses internal error details from end users was merged without an automated regression test, so CI will not fail if the change is reverted.

## Issue Context
The production-mode `globalExceptionHandler` output is now sanitized, and the page-load error handler no longer renders stack/file details in the DOM.

## Fix Focus Areas
- src/static/js/pad_utils.ts[428-457]
- src/static/js/basic_error_handler.ts[26-34]
- src/tests/frontend-new/specs/error_sanitization.spec.ts[1-200]

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

---

2. Mode check leaks details 🐞 Bug ⛨ Security

Description

globalExceptionHandler decides whether to hide details with `window.clientVars?.mode ===
'production', but window.clientVars is initially created without mode and mode` is only
populated after the async CLIENT_VARS handshake. Errors before that (or if NODE_ENV is not exactly
'production') will incorrectly take the non-production path and show internal details to end
users.

Code

src/static/js/pad_utils.ts[R431-457]

+          const isProduction = (window as any).clientVars?.mode === 'production';
+
+          const errorMsg = isProduction
+            ? [
+              $('<p>')
+                .append($('<b>').text('Please press and hold Ctrl and press F5 to reload this page')),
+              $('<p>')
+                .text('If the problem persists, please contact your webmaster.')
+                .append($('<br>'))
+                .append($('<span>').css('font-size', '.8em').text(`ErrorId: ${errorId}`)),
+            ]
+            : (() => {
+              const txt = document.createTextNode.bind(document);
+              return [
+                $('<p>')
+                  .append($('<b>').text('Please press and hold Ctrl and press F5 to reload this page')),
+                $('<p>')
+                  .text('If the problem persists, please send this error message to your webmaster:'),
+                $('<div>').css('text-align', 'left').css('font-size', '.8em').css('margin-top', '1em')
+                  .append($('<b>').addClass('error-msg').text(msg)).append($('<br>'))
+                  .append(txt(`at ${url} at line ${linenumber}`)).append($('<br>'))
+                  .append(txt(`ErrorId: ${errorId}`)).append($('<br>'))
+                  .append(txt(type)).append($('<br>'))
+                  .append(txt(`URL: ${window.location.href}`)).append($('<br>'))
+                  .append(txt(`UserAgent: ${navigator.userAgent}`)).append($('<br>')),
+              ];
+            })();

Evidence

The handler is installed before the handshake completes, while initial window.clientVars lacks
mode and the server sets mode directly from process.env.NODE_ENV (which can be undefined or a
non-'production' value such as 'test'). This makes the strict equality check unreliable and can
expose the detailed (dev) error UI in production scenarios.

src/static/js/pad_utils.ts[420-457]
src/static/js/pad.ts[410-423]
src/static/js/pad.ts[316-325]
src/templates/padBootstrap.js[6-10]
src/templates/padViteBootstrap.js[4-8]
src/node/handler/PadMessageHandler.ts[1060-1062]

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()` currently treats "production" as a strict `clientVars.mode === 'production'` check. Because `window.clientVars.mode` is not available until after the async CLIENT_VARS handshake (and is set from `process.env.NODE_ENV` without a default), the code can incorrectly show the detailed error message UI to end users.

## Issue Context
- `pad.init()` installs the global exception handler before awaiting `handshake()`.
- The bootstrap scripts initialize `window.clientVars` with only `randomVersionString` (no `mode`).
- Server sets `clientVars.mode = process.env.NODE_ENV`, which can be `undefined` or other strings (e.g., `test`).

## Fix Focus Areas
- src/static/js/pad_utils.ts[420-457]
- src/static/js/pad.ts[410-423]
- src/templates/padBootstrap.js[6-10]
- src/templates/padViteBootstrap.js[4-8]
- src/node/handler/PadMessageHandler.ts[1060-1062]

## Suggested fix
- Change the logic to a secure default, e.g. compute `showDetails = window.clientVars?.mode === 'development'` and use `showDetails ? detailed : generic`.
- (Optional but best) Ensure `mode` is present as early as possible (e.g., include `mode` in the initial `window.clientVars` object in the bootstrap templates) so development builds still show full details even before the handshake.

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

---

3. Error popup dedupe broken 🐞 Bug ☼ Reliability

Description

The duplicate-notification suppression scans .gritter-item .error-msg, but the new production-mode
gritter message no longer renders an element with the error-msg class. In production, identical
errors will therefore bypass deduplication and can spam multiple sticky popups.

Code

src/static/js/pad_utils.ts[R425-441]

        });

        if (!msgAlreadyVisible) {
-          const txt = document.createTextNode.bind(document); // Convenience shorthand.
-          const errorMsg = [
-            $('<p>')
-              .append($('<b>').text('Please press and hold Ctrl and press F5 to reload this page')),
-            $('<p>')
-              .text('If the problem persists, please send this error message to your webmaster:'),
-            $('<div>').css('text-align', 'left').css('font-size', '.8em').css('margin-top', '1em')
-              .append($('<b>').addClass('error-msg').text(msg)).append($('<br>'))
-              .append(txt(`at ${url} at line ${linenumber}`)).append($('<br>'))
-              .append(txt(`ErrorId: ${errorId}`)).append($('<br>'))
-              .append(txt(type)).append($('<br>'))
-              .append(txt(`URL: ${window.location.href}`)).append($('<br>'))
-              .append(txt(`UserAgent: ${navigator.userAgent}`)).append($('<br>')),
-          ];
+          // In production mode, hide internal error details (file paths, line numbers,
+          // error messages) from end users. Only show a generic reload message.
+          // See https://github.com/ether/etherpad-lite/issues/5765
+          const isProduction = (window as any).clientVars?.mode === 'production';
+
+          const errorMsg = isProduction
+            ? [
+              $('<p>')
+                .append($('<b>').text('Please press and hold Ctrl and press F5 to reload this page')),
+              $('<p>')
+                .text('If the problem persists, please contact your webmaster.')
+                .append($('<br>'))
+                .append($('<span>').css('font-size', '.8em').text(`ErrorId: ${errorId}`)),
+            ]

Evidence

Deduplication only considers the presence/text of .error-msg. That marker exists only in the
detailed (development) branch; the production branch omits it entirely, so the scan finds nothing
and msgAlreadyVisible stays false.

src/static/js/pad_utils.ts[420-457]

Agent prompt

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

## Issue description
The gritter dedupe logic relies on a DOM element (`.error-msg`) that no longer exists in the production-mode notification body, so repeated errors can generate multiple sticky popups.

## Issue Context
- Dedupe scans `.gritter-item .error-msg` and compares its text to `msg`.
- The production message intentionally does not include `msg` or any `.error-msg` element.

## Fix Focus Areas
- src/static/js/pad_utils.ts[420-457]

## Suggested fix
- Replace DOM-based dedupe with an internal in-memory cache (e.g., a `Set` keyed by `type + msg + source + linenumber`, or just `msg`) that prevents repeated notifications regardless of display mode.
- If you must keep DOM-based dedupe, add a non-user-visible marker that does not reveal the message (e.g., `data-error-key` with a hash) and dedupe using that attribute instead of `.error-msg` text.

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

---

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

[qodo-free-for-open-source-projects]

1. isproduction error ui untested 📘 Rule violation ☼ Reliability

The PR changes user-facing error sanitization behavior but does not include an automated regression
test to ensure the sanitized message remains enforced. Reverting these changes would not be caught
by CI, risking re-exposure of internal error details.

Agent Prompt

## Issue description
A bug fix that suppresses internal error details from end users was merged without an automated regression test, so CI will not fail if the change is reverted.

## Issue Context
The production-mode `globalExceptionHandler` output is now sanitized, and the page-load error handler no longer renders stack/file details in the DOM.

## Fix Focus Areas
- src/static/js/pad_utils.ts[428-457]
- src/static/js/basic_error_handler.ts[26-34]
- src/tests/frontend-new/specs/error_sanitization.spec.ts[1-200]

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

[qodo-free-for-open-source-projects]

2. Mode check leaks details 🐞 Bug ⛨ Security

globalExceptionHandler decides whether to hide details with `window.clientVars?.mode ===
'production', but window.clientVars is initially created without mode and mode` is only
populated after the async CLIENT_VARS handshake. Errors before that (or if NODE_ENV is not exactly
'production') will incorrectly take the non-production path and show internal details to end
users.

Agent Prompt

## Issue description
`setupGlobalExceptionHandler()` currently treats "production" as a strict `clientVars.mode === 'production'` check. Because `window.clientVars.mode` is not available until after the async CLIENT_VARS handshake (and is set from `process.env.NODE_ENV` without a default), the code can incorrectly show the detailed error message UI to end users.

## Issue Context
- `pad.init()` installs the global exception handler before awaiting `handshake()`.
- The bootstrap scripts initialize `window.clientVars` with only `randomVersionString` (no `mode`).
- Server sets `clientVars.mode = process.env.NODE_ENV`, which can be `undefined` or other strings (e.g., `test`).

## Fix Focus Areas
- src/static/js/pad_utils.ts[420-457]
- src/static/js/pad.ts[410-423]
- src/templates/padBootstrap.js[6-10]
- src/templates/padViteBootstrap.js[4-8]
- src/node/handler/PadMessageHandler.ts[1060-1062]

## Suggested fix
- Change the logic to a secure default, e.g. compute `showDetails = window.clientVars?.mode === 'development'` and use `showDetails ? detailed : generic`.
- (Optional but best) Ensure `mode` is present as early as possible (e.g., include `mode` in the initial `window.clientVars` object in the bootstrap templates) so development builds still show full details even before the handshake.

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