Don't treat benign browser error events as client crashes

ResizeObserver loops and cross-origin 'Script error.' notifications arrive
without an Error object and don't mean anything is broken, so they are ignored
instead of closing the connection and showing the crash overlay.

Also drop the location line when the browser has no real one (it passes the
document URL with line/column 0), open the technical details whenever the user
is asked to copy them, and close the connection before collecting the context
so the reporter-failed overlay never sits on top of a live session.

Author: VTT-AI-Agent

client/js/tracing.js

@@ -37,6 +37,13 @@ export function describeError(error, fallback) {
   return fallback + (error === undefined || error === null ? '' : '\n' + stringifyValue(error));
 }
 
+// some error events don't mean that the client is broken: a ResizeObserver loop is simply retried
+// on the next frame and a cross-origin 'Script error.' usually comes from a browser extension.
+// both are reported without an Error object, which is what tells them apart from a real crash.
+export function isNonFatalError(msg, error) {
+  return !error && /^(ResizeObserver loop|Script error\.?$)/.test(`${msg}`);
+}
+
 // a rejection reason is often a plain object like { status: 500 } - String() would turn that
 // into a useless [object Object], while JSON.stringify fails on cyclic values and BigInt
 function stringifyValue(value) {
@@ -76,7 +83,18 @@ onLoad(function() {
     showOverlay('clientErrorOverlay');
   }
 
+  // the status line asks the user to copy the technical details, so make sure they are visible
+  const askForManualReport = function() {
+    $('#clientErrorStatus').style.display = '';
+    $('#clientErrorOverlay details').open = true;
+  }
+
   const reportError = function(description) {
+    // close the connection before collecting the context: if that throws, the user still ends up
+    // with a terminal overlay over a terminated session instead of a still running one
+    preventReconnect();
+    connection.close();
+
     const details = {
       error: description,
       undoProtocol,
@@ -95,16 +113,14 @@ onLoad(function() {
       playerName,
       html: document.documentElement.outerHTML
     };
-    preventReconnect();
-    connection.close();
 
     const button = $('#clientErrorOverlay button');
     // what the user typed is the most valuable part of the report - keep the textarea intact
     // and put the reason for the failure into the technical details instead
     const submitFailed = function(reason) {
       button.disabled = false;
       button.textContent = 'Try again';
-      $('#clientErrorStatus').style.display = '';
+      askForManualReport();
       $('#clientErrorStack').textContent = `${details.error}\n\nSubmitting the report failed:\n${reason}`;
     }
 
@@ -158,7 +174,7 @@ onLoad(function() {
       // is nothing to submit in that case, so ask for a manual report and offer a plain reload.
       $('#clientErrorQuestion').style.display = 'none';
       $('#clientErrorInput').style.display = 'none';
-      $('#clientErrorStatus').style.display = '';
+      askForManualReport();
       const button = $('#clientErrorOverlay button');
       button.textContent = 'Reload';
       button.addEventListener('click', _=>window.location.reload());
@@ -167,8 +183,13 @@ onLoad(function() {
   }
 
   window.onerror = function(msg, url, line, col, err) {
-    // browsers report cross-origin errors without a location - 'at :0:0' would just look broken
-    errorHandler(err, `${msg}` + (url ? `\n    at ${url}:${line}:${col}` : ''));
+    // tearing the session down over a non-fatal event would be worse than ignoring it, which is
+    // what happened anyway before this handler learned to survive a missing Error object
+    if(isNonFatalError(msg, err))
+      return;
+    // when the browser has no real location it passes the document URL with line and column 0 -
+    // reporting 'at <page>:0:0' would just look like a truncation bug, so leave the line out
+    errorHandler(err, `${msg}` + (url && line ? `\n    at ${url}:${line}:${col}` : ''));
   };
   window.addEventListener("unhandledrejection", function(promiseRejectionEvent) {
     errorHandler(promiseRejectionEvent.reason, 'Unhandled promise rejection');

tests/client/error-report.test.js

@@ -1,4 +1,4 @@
-import { describeError } from '../../client/js/tracing.js';
+import { describeError, isNonFatalError } from '../../client/js/tracing.js';
 
 describe("Scenarios: Describing a client error for the error report", () => {
   describe("Given an Error object", () => {
@@ -46,3 +46,23 @@ describe("Scenarios: Describing a client error for the error report", () => {
     });
   });
 });
+
+describe("Scenarios: Deciding whether an error event should crash the client", () => {
+  describe("Given a browser event that does not indicate a broken client", () => {
+    test("Then it is treated as non-fatal", () => {
+      expect(isNonFatalError('ResizeObserver loop completed with undelivered notifications.')).toBe(true);
+      expect(isNonFatalError('ResizeObserver loop limit exceeded')).toBe(true);
+      expect(isNonFatalError('Script error.')).toBe(true);
+      expect(isNonFatalError('Script error')).toBe(true);
+    });
+  });
+
+  describe("Given a real error", () => {
+    test("Then it is reported even if the message looks harmless", () => {
+      expect(isNonFatalError('Uncaught TypeError: x is not a function')).toBe(false);
+      expect(isNonFatalError('Script error.', new Error('Script error.'))).toBe(false);
+      expect(isNonFatalError('ResizeObserver loop completed', new Error('boom'))).toBe(false);
+      expect(isNonFatalError(undefined)).toBe(false);
+    });
+  });
+});