Error logging refactoring (#454)

* Create new logging methods on yomichan object

* Use new yomichan.logError instead of global logError

* Remove old logError

* Handle unhandledrejection events

* Add addEventListener stub

* Update log function

* Update error conversion to support more types

* Add log event

* Add API log function

* Log errors to the backend

* Make error/warning logs update the badge

* Clear log error indicator on extension button click

* Log correct URL on the background page

* Fix incorrect error conversion

* Remove unhandledrejection handling

Firefox doesn't support it properly.

* Remove unused argument type from log function

* Improve function name

* Change console.warn to yomichan.logWarning

* Move log forwarding initialization into main scripts

21 files changed, 186 insertions, 45 deletions

diff --git a/.eslintrc.json b/.eslintrc.json
index 8882cb42..78fec27c 100644
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -80,7 +80,6 @@
                 "yomichan": "readonly",
                 "errorToJson": "readonly",
                 "jsonToError": "readonly",
-                "logError": "readonly",
                 "isObject": "readonly",
                 "hasOwn": "readonly",
                 "toIterable": "readonly",
diff --git a/ext/bg/js/backend.js b/ext/bg/js/backend.js
index 693a9ad6..3c47b14e 100644
--- a/ext/bg/js/backend.js
+++ b/ext/bg/js/backend.js
@@ -78,6 +78,7 @@ class Backend {
         this._isPrepared = false;
         this._prepareError = false;
         this._badgePrepareDelayTimer = null;
+        this._logErrorLevel = null;
 
         this._messageHandlers = new Map([
             ['yomichanCoreReady', {handler: this._onApiYomichanCoreReady.bind(this), async: false}],
@@ -112,7 +113,9 @@ class Backend {
             ['getDictionaryInfo', {handler: this._onApiGetDictionaryInfo.bind(this), async: true}],
             ['getDictionaryCounts', {handler: this._onApiGetDictionaryCounts.bind(this), async: true}],
             ['purgeDatabase', {handler: this._onApiPurgeDatabase.bind(this), async: true}],
-            ['getMedia', {handler: this._onApiGetMedia.bind(this), async: true}]
+            ['getMedia', {handler: this._onApiGetMedia.bind(this), async: true}],
+            ['log', {handler: this._onApiLog.bind(this), async: false}],
+            ['logIndicatorClear', {handler: this._onApiLogIndicatorClear.bind(this), async: false}]
         ]);
 
         this._commandHandlers = new Map([
@@ -164,7 +167,7 @@ class Backend {
             this._isPrepared = true;
         } catch (e) {
             this._prepareError = true;
-            logError(e);
+            yomichan.logError(e);
             throw e;
         } finally {
             if (this._badgePrepareDelayTimer !== null) {
@@ -260,7 +263,7 @@ class Backend {
             this.options = JsonSchema.getValidValueOrDefault(this.optionsSchema, utilIsolate(options));
         } catch (e) {
             // This shouldn't happen, but catch errors just in case of bugs
-            logError(e);
+            yomichan.logError(e);
         }
     }
 
@@ -767,8 +770,34 @@ class Backend {
         return await this.database.getMedia(targets);
     }
 
+    _onApiLog({error, level, context}) {
+        yomichan.log(jsonToError(error), level, context);
+
+        const levelValue = this._getErrorLevelValue(level);
+        if (levelValue <= this._getErrorLevelValue(this._logErrorLevel)) { return; }
+
+        this._logErrorLevel = level;
+        this._updateBadge();
+    }
+
+    _onApiLogIndicatorClear() {
+        if (this._logErrorLevel === null) { return; }
+        this._logErrorLevel = null;
+        this._updateBadge();
+    }
+
     // Command handlers
 
+    _getErrorLevelValue(errorLevel) {
+        switch (errorLevel) {
+            case 'info': return 0;
+            case 'debug': return 0;
+            case 'warn': return 1;
+            case 'error': return 2;
+            default: return 0;
+        }
+    }
+
     async _onCommandSearch(params) {
         const {mode='existingOrNewTab', query} = params || {};
 
@@ -890,7 +919,20 @@ class Backend {
         let color = null;
         let status = null;
 
-        if (!this._isPrepared) {
+        if (this._logErrorLevel !== null) {
+            switch (this._logErrorLevel) {
+                case 'error':
+                    text = '!!';
+                    color = '#f04e4e';
+                    status = 'Error';
+                    break;
+                default: // 'warn'
+                    text = '!';
+                    color = '#f0ad4e';
+                    status = 'Warning';
+                    break;
+            }
+        } else if (!this._isPrepared) {
             if (this._prepareError) {
                 text = '!!';
                 color = '#f04e4e';
diff --git a/ext/bg/js/context-main.js b/ext/bg/js/context-main.js
index e2086a96..dbba0272 100644
--- a/ext/bg/js/context-main.js
+++ b/ext/bg/js/context-main.js
@@ -17,7 +17,9 @@
 
 /* global
  * apiCommandExec
+ * apiForwardLogsToBackend
  * apiGetEnvironmentInfo
+ * apiLogIndicatorClear
  * apiOptionsGet
  */
 
@@ -52,8 +54,11 @@ function setupButtonEvents(selector, command, url) {
 }
 
 async function mainInner() {
+    apiForwardLogsToBackend();
     await yomichan.prepare();
 
+    await apiLogIndicatorClear();
+
     showExtensionInfo();
 
     apiGetEnvironmentInfo().then(({browser}) => {
diff --git a/ext/bg/js/database.js b/ext/bg/js/database.js
index 16612403..a94aa720 100644
--- a/ext/bg/js/database.js
+++ b/ext/bg/js/database.js
@@ -104,7 +104,7 @@ class Database {
             });
             return true;
         } catch (e) {
-            logError(e);
+            yomichan.logError(e);
             return false;
         }
     }
diff --git a/ext/bg/js/mecab.js b/ext/bg/js/mecab.js
index 597dceae..815ee860 100644
--- a/ext/bg/js/mecab.js
+++ b/ext/bg/js/mecab.js
@@ -24,7 +24,7 @@ class Mecab {
     }
 
     onError(error) {
-        logError(error, false);
+        yomichan.logError(error);
     }
 
     async checkVersion() {
diff --git a/ext/bg/js/search-main.js b/ext/bg/js/search-main.js
index 38b6d99a..5e4d7a20 100644
--- a/ext/bg/js/search-main.js
+++ b/ext/bg/js/search-main.js
@@ -17,6 +17,7 @@
 
 /* global
  * DisplaySearch
+ * apiForwardLogsToBackend
  * apiOptionsGet
  */
 
@@ -53,6 +54,7 @@ function injectSearchFrontend() {
 }
 
 (async () => {
+    apiForwardLogsToBackend();
     await yomichan.prepare();
 
     const displaySearch = new DisplaySearch();
diff --git a/ext/bg/js/search-query-parser.js b/ext/bg/js/search-query-parser.js
index eb3b681c..0001c9ff 100644
--- a/ext/bg/js/search-query-parser.js
+++ b/ext/bg/js/search-query-parser.js
@@ -45,7 +45,7 @@ class QueryParser extends TextScanner {
     }
 
     onError(error) {
-        logError(error, false);
+        yomichan.logError(error);
     }
 
     onClick(e) {
diff --git a/ext/bg/js/search.js b/ext/bg/js/search.js
index a5484fc3..cbd7b562 100644
--- a/ext/bg/js/search.js
+++ b/ext/bg/js/search.js
@@ -122,7 +122,7 @@ class DisplaySearch extends Display {
     }
 
     onError(error) {
-        logError(error, true);
+        yomichan.logError(error);
     }
 
     onSearchClear() {
diff --git a/ext/bg/js/settings/backup.js b/ext/bg/js/settings/backup.js
index bdfef658..faf4e592 100644
--- a/ext/bg/js/settings/backup.js
+++ b/ext/bg/js/settings/backup.js
@@ -133,7 +133,7 @@ async function _settingsImportSetOptionsFull(optionsFull) {
 }
 
 function _showSettingsImportError(error) {
-    logError(error);
+    yomichan.logError(error);
     document.querySelector('#settings-import-error-modal-message').textContent = `${error}`;
     $('#settings-import-error-modal').modal('show');
 }
diff --git a/ext/bg/js/settings/dictionaries.js b/ext/bg/js/settings/dictionaries.js
index 7eed4273..50add4c7 100644
--- a/ext/bg/js/settings/dictionaries.js
+++ b/ext/bg/js/settings/dictionaries.js
@@ -554,7 +554,7 @@ function dictionaryErrorsShow(errors) {
     if (errors !== null && errors.length > 0) {
         const uniqueErrors = new Map();
         for (let e of errors) {
-            logError(e);
+            yomichan.logError(e);
             e = dictionaryErrorToString(e);
             let count = uniqueErrors.get(e);
             if (typeof count === 'undefined') {
diff --git a/ext/bg/js/settings/main.js b/ext/bg/js/settings/main.js
index 308e92eb..f03cc631 100644
--- a/ext/bg/js/settings/main.js
+++ b/ext/bg/js/settings/main.js
@@ -21,6 +21,7 @@
  * ankiInitialize
  * ankiTemplatesInitialize
  * ankiTemplatesUpdateValue
+ * apiForwardLogsToBackend
  * apiOptionsSave
  * appearanceInitialize
  * audioSettingsInitialize
@@ -284,6 +285,7 @@ function showExtensionInformation() {
 
 
 async function onReady() {
+    apiForwardLogsToBackend();
     await yomichan.prepare();
 
     showExtensionInformation();
diff --git a/ext/bg/js/settings/popup-preview-frame-main.js b/ext/bg/js/settings/popup-preview-frame-main.js
index 2ab6af6b..8228125f 100644
--- a/ext/bg/js/settings/popup-preview-frame-main.js
+++ b/ext/bg/js/settings/popup-preview-frame-main.js
@@ -17,8 +17,10 @@
 
 /* global
  * SettingsPopupPreview
+ * apiForwardLogsToBackend
  */
 
 (() => {
+    apiForwardLogsToBackend();
     new SettingsPopupPreview();
 })();
diff --git a/ext/fg/js/content-script-main.js b/ext/fg/js/content-script-main.js
index 0b852644..277e6567 100644
--- a/ext/fg/js/content-script-main.js
+++ b/ext/fg/js/content-script-main.js
@@ -22,6 +22,7 @@
  * PopupProxy
  * PopupProxyHost
  * apiBroadcastTab
+ * apiForwardLogsToBackend
  * apiOptionsGet
  */
 
@@ -62,6 +63,7 @@ async function createPopupProxy(depth, id, parentFrameId) {
 }
 
 (async () => {
+    apiForwardLogsToBackend();
     await yomichan.prepare();
 
     const data = window.frontendInitializationData || {};
diff --git a/ext/fg/js/float-main.js b/ext/fg/js/float-main.js
index f056f707..5ef4b07c 100644
--- a/ext/fg/js/float-main.js
+++ b/ext/fg/js/float-main.js
@@ -17,6 +17,7 @@
 
 /* global
  * DisplayFloat
+ * apiForwardLogsToBackend
  * apiOptionsGet
  */
 
@@ -68,5 +69,6 @@ async function popupNestedInitialize(id, depth, parentFrameId, url) {
 }
 
 (async () => {
+    apiForwardLogsToBackend();
     new DisplayFloat();
 })();
diff --git a/ext/fg/js/float.js b/ext/fg/js/float.js
index 2a5eba83..fd3b92cc 100644
--- a/ext/fg/js/float.js
+++ b/ext/fg/js/float.js
@@ -84,7 +84,7 @@ class DisplayFloat extends Display {
         if (this._orphaned) {
             this.setContent('orphaned');
         } else {
-            logError(error, true);
+            yomichan.logError(error);
         }
     }
 
diff --git a/ext/fg/js/frontend-api-sender.js b/ext/fg/js/frontend-api-sender.js
index 1d539cab..0ad3f085 100644
--- a/ext/fg/js/frontend-api-sender.js
+++ b/ext/fg/js/frontend-api-sender.js
@@ -81,12 +81,12 @@ class FrontendApiSender {
     onAck(id) {
         const info = this.callbacks.get(id);
         if (typeof info === 'undefined') {
-            console.warn(`ID ${id} not found for ack`);
+            yomichan.logWarning(new Error(`ID ${id} not found for ack`));
             return;
         }
 
         if (info.ack) {
-            console.warn(`Request ${id} already ack'd`);
+            yomichan.logWarning(new Error(`Request ${id} already ack'd`));
             return;
         }
 
@@ -98,12 +98,12 @@ class FrontendApiSender {
     onResult(id, data) {
         const info = this.callbacks.get(id);
         if (typeof info === 'undefined') {
-            console.warn(`ID ${id} not found`);
+            yomichan.logWarning(new Error(`ID ${id} not found`));
             return;
         }
 
         if (!info.ack) {
-            console.warn(`Request ${id} not ack'd`);
+            yomichan.logWarning(new Error(`Request ${id} not ack'd`));
             return;
         }
 
diff --git a/ext/fg/js/popup-proxy.js b/ext/fg/js/popup-proxy.js
index cd3c1bc9..93418202 100644
--- a/ext/fg/js/popup-proxy.js
+++ b/ext/fg/js/popup-proxy.js
@@ -148,7 +148,7 @@ class PopupProxy {
             }
             this._frameOffsetUpdatedAt = now;
         } catch (e) {
-            logError(e);
+            yomichan.logError(e);
         } finally {
             this._frameOffsetPromise = null;
         }
diff --git a/ext/mixed/js/api.js b/ext/mixed/js/api.js
index 52f41646..afd68aa2 100644
--- a/ext/mixed/js/api.js
+++ b/ext/mixed/js/api.js
@@ -144,6 +144,14 @@ function apiGetMedia(targets) {
     return _apiInvoke('getMedia', {targets});
 }
 
+function apiLog(error, level, context) {
+    return _apiInvoke('log', {error, level, context});
+}
+
+function apiLogIndicatorClear() {
+    return _apiInvoke('logIndicatorClear');
+}
+
 function _apiInvoke(action, params={}) {
     const data = {action, params};
     return new Promise((resolve, reject) => {
@@ -171,3 +179,17 @@ function _apiInvoke(action, params={}) {
 function _apiCheckLastError() {
     // NOP
 }
+
+let _apiForwardLogsToBackendEnabled = false;
+function apiForwardLogsToBackend() {
+    if (_apiForwardLogsToBackendEnabled) { return; }
+    _apiForwardLogsToBackendEnabled = true;
+
+    yomichan.on('log', async ({error, level, context}) => {
+        try {
+            await apiLog(errorToJson(error), level, context);
+        } catch (e) {
+            // NOP
+        }
+    });
+}
diff --git a/ext/mixed/js/core.js b/ext/mixed/js/core.js
index 6a3298fc..fbe9943a 100644
--- a/ext/mixed/js/core.js
+++ b/ext/mixed/js/core.js
@@ -52,15 +52,28 @@ if (EXTENSION_IS_BROWSER_EDGE) {
  */
 
 function errorToJson(error) {
+    try {
+        if (isObject(error)) {
+            return {
+                name: error.name,
+                message: error.message,
+                stack: error.stack,
+                data: error.data
+            };
+        }
+    } catch (e) {
+        // NOP
+    }
     return {
-        name: error.name,
-        message: error.message,
-        stack: error.stack,
-        data: error.data
+        value: error,
+        hasValue: true
     };
 }
 
 function jsonToError(jsonError) {
+    if (jsonError.hasValue) {
+        return jsonError.value;
+    }
     const error = new Error(jsonError.message);
     error.name = jsonError.name;
     error.stack = jsonError.stack;
@@ -68,28 +81,6 @@ function jsonToError(jsonError) {
     return error;
 }
 
-function logError(error, alert) {
-    const manifest = chrome.runtime.getManifest();
-    let errorMessage = `${manifest.name} v${manifest.version} has encountered an error.\n`;
-    errorMessage += `Originating URL: ${window.location.href}\n`;
-
-    const errorString = `${error.toString ? error.toString() : error}`;
-    const stack = `${error.stack}`.trimRight();
-    if (!stack.startsWith(errorString)) { errorMessage += `${errorString}\n`; }
-    errorMessage += stack;
-
-    const data = error.data;
-    if (typeof data !== 'undefined') { errorMessage += `\nData: ${JSON.stringify(data, null, 4)}`; }
-
-    errorMessage += '\n\nIssues can be reported at https://github.com/FooSoft/yomichan/issues';
-
-    console.error(errorMessage);
-
-    if (alert) {
-        window.alert(`${errorString}\n\nCheck the developer console for more details.`);
-    }
-}
-
 
 /*
  * Common helpers
@@ -361,8 +352,77 @@ const yomichan = (() => {
             });
         }
 
+        logWarning(error) {
+            this.log(error, 'warn');
+        }
+
+        logError(error) {
+            this.log(error, 'error');
+        }
+
+        log(error, level, context=null) {
+            if (!isObject(context)) {
+                context = this._getLogContext();
+            }
+
+            let errorString;
+            try {
+                errorString = error.toString();
+                if (/^\[object \w+\]$/.test(errorString)) {
+                    errorString = JSON.stringify(error);
+                }
+            } catch (e) {
+                errorString = `${error}`;
+            }
+
+            let errorStack;
+            try {
+                errorStack = (typeof error.stack === 'string' ? error.stack.trimRight() : '');
+            } catch (e) {
+                errorStack = '';
+            }
+
+            let errorData;
+            try {
+                errorData = error.data;
+            } catch (e) {
+                // NOP
+            }
+
+            if (errorStack.startsWith(errorString)) {
+                errorString = errorStack;
+            } else if (errorStack.length > 0) {
+                errorString += `\n${errorStack}`;
+            }
+
+            const manifest = chrome.runtime.getManifest();
+            let message = `${manifest.name} v${manifest.version} has encountered a problem.`;
+            message += `\nOriginating URL: ${context.url}\n`;
+            message += errorString;
+            if (typeof errorData !== 'undefined') {
+                message += `\nData: ${JSON.stringify(errorData, null, 4)}`;
+            }
+            message += '\n\nIssues can be reported at https://github.com/FooSoft/yomichan/issues';
+
+            switch (level) {
+                case 'info': console.info(message); break;
+                case 'debug': console.debug(message); break;
+                case 'warn': console.warn(message); break;
+                case 'error': console.error(message); break;
+                default: console.log(message); break;
+            }
+
+            this.trigger('log', {error, level, context});
+        }
+
         // Private
 
+        _getLogContext() {
+            return {
+                url: window.location.href
+            };
+        }
+
         _onMessage({action, params}, sender, callback) {
             const handler = this._messageHandlers.get(action);
             if (typeof handler !== 'function') { return false; }
diff --git a/ext/mixed/js/text-scanner.js b/ext/mixed/js/text-scanner.js
index 0cd12cd7..1c32714b 100644
--- a/ext/mixed/js/text-scanner.js
+++ b/ext/mixed/js/text-scanner.js
@@ -201,7 +201,7 @@ class TextScanner {
     }
 
     onError(error) {
-        logError(error, false);
+        yomichan.logError(error);
     }
 
     async scanTimerWait() {
diff --git a/test/test-database.js b/test/test-database.js
index e9ec3f0b..3684051b 100644
--- a/test/test-database.js
+++ b/test/test-database.js
@@ -145,7 +145,10 @@ const vm = new VM({
     XMLHttpRequest,
     indexedDB: global.indexedDB,
     IDBKeyRange: global.IDBKeyRange,
-    JSZip: yomichanTest.JSZip
+    JSZip: yomichanTest.JSZip,
+    addEventListener() {
+        // NOP
+    }
 });
 vm.context.window = vm.context;