From d4bc6246f16fddd6c79cd6761717e45efb75033a Mon Sep 17 00:00:00 2001
From: Dmitry S <dsagal+git@gmail.com>
Date: Mon, 22 May 2023 15:55:20 -0400
Subject: [PATCH] (core) Fix breakage on Firefox iOS

Summary:
Grist recently stopped working on Firefox on iOS. The cause turns out an uncaught error, which is reported as an unhelpful "Script Error", but the act of reporting it causes additional errors, leading to an infinite loop and an unusable browser tab.

Firefox-iOS is to blame, but a workaround is preventing a flood of "Script Error" messages. Specifically, we report only the first of these, and only to the server, suppressing the user-visible toast.

Test Plan: Tested manually on Firefox on iOS. Added a test case, and improve other tests of uncaught errors.

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3902
---
 app/client/models/errors.ts | 26 +++++++++++++++++++++++---
 test/nbrowser/gristUtils.ts | 13 +++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/app/client/models/errors.ts b/app/client/models/errors.ts
index ee388556..c884b04b 100644
--- a/app/client/models/errors.ts
+++ b/app/client/models/errors.ts
@@ -70,6 +70,18 @@ export function reportSuccess(msg: MessageType, options?: Partial<INotifyOptions
   return reportMessage(msg, {level: 'success', ...options});
 }
 
+// Errors from cross-origin scripts, and some add-ons, show up as unhelpful sanitized "Script
+// error." messages. We want to know if they occur, but they are useless to the user, and useless
+// to report multiple times. We report them just once to the server.
+//
+// In particular, this addresses a bug on iOS version of Firefox, which produces uncaught
+// sanitized errors on load AND on attempts to report them, leading to a loop that hangs the
+// browser. Reporting just once is a sufficient workaround.
+function isUnhelpful(ev: ErrorEvent) {
+  return !ev.filename && !ev.lineno && ev.message?.toLowerCase().includes('script error');
+}
+let unhelpfulErrors = 0;
+
 /**
  * Report an error to the user using the global Notifier instance. If the argument is a UserError
  * or an error with a status in the 400 range, it indicates a user error. Otherwise, it's an
@@ -78,12 +90,21 @@ export function reportSuccess(msg: MessageType, options?: Partial<INotifyOptions
  * Not all errors will be shown as an error toast, depending on the content of the error
  * this function might show a simple toast message.
  */
-export function reportError(err: Error|string): void {
+export function reportError(err: Error|string, ev?: ErrorEvent): void {
   log.error(`ERROR:`, err);
   if (String(err).match(/GristWSConnection disposed/)) {
     // This error can be emitted while a page is reloaded, and isn't worth reporting.
     return;
   }
+  if (ev && isUnhelpful(ev)) {
+    // Report just once to the server. There is little point reporting subsequent such errors once
+    // we know they happen, since each individual error has no useful information.
+    if (++unhelpfulErrors <= 1) {
+      logError(err);
+    }
+    return;
+  }
+
   logError(err);
   if (_notifier && !_notifier.isDisposed()) {
     if (!isError(err)) {
@@ -154,8 +175,7 @@ export function setUpErrorHandling(doReportError = reportError, koUtil?: any) {
   }
 
   // Report also uncaught JS errors and unhandled Promise rejections.
-  G.window.onerror = ((ev: any, url: any, lineNo: any, colNo: any, err: any) =>
-    doReportError(err || ev));
+  G.window.addEventListener('error', (ev: ErrorEvent) => doReportError(ev.error || ev.message, ev));
 
   G.window.addEventListener('unhandledrejection', (ev: any) => {
     const reason = ev.reason || (ev.detail && ev.detail.reason);
diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts
index 63b3755e..e9b9250d 100644
--- a/test/nbrowser/gristUtils.ts
+++ b/test/nbrowser/gristUtils.ts
@@ -3068,6 +3068,19 @@ export async function assertIsRickRowing(expected: boolean) {
   assert.equal(await driver.find('iframe#youtube-player-dQw4w9WgXcQ').isPresent(), expected);
 }
 
+
+export function produceUncaughtError(message: string) {
+  // Simply throwing an error from driver.executeScript() may produce a sanitized "Script error",
+  // depending on browser/webdriver version. This is a trick to ensure the uncaught error is
+  // considered same-origin by the main window.
+  return driver.executeScript((msg: string) => {
+    const script = document.createElement("script");
+    script.type = "text/javascript";
+    script.innerText = 'setTimeout(() => { throw new Error(' + JSON.stringify(msg) + '); }, 0)';
+    document.head.appendChild(script);
+  }, message);
+}
+
 } // end of namespace gristUtils
 
 stackWrapOwnMethods(gristUtils);