From 38ae01d6b59ca2649a7a4e5cfdf8d50a67232fe4 Mon Sep 17 00:00:00 2001 From: "Michael Mattiello (mcm)" Date: Mon, 31 May 2021 13:22:56 +0000 Subject: [PATCH] [IMP] web: improve error management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Error handlers have been simplified, handlers don't returns functions anymore and take 3 params: env, uncaughtError and originalError. - Source maps have been reintroduced. - The original error message and name are now concatenated to the "wrapper" error ones. closes odoo-dev/odoo#895 Related: odoo-dev/enterprise#154 Signed-off-by: Géry Debongnie (ged) --- addons/web/static/lib/qunit/qunit-2.9.1.js | 4 +- .../static/src/core/errors/error_dialogs.xml | 2 +- .../static/src/core/errors/error_handlers.js | 230 +++++++++--------- .../static/src/core/errors/error_service.js | 103 ++++---- .../web/static/src/core/errors/error_utils.js | 90 +++++++ .../legacy/legacy_promise_error_handler.js | 34 +-- .../src/legacy/legacy_rpc_error_handler.js | 67 ++--- .../tests/core/errors/error_service_tests.js | 64 ++++- .../tests/core/network/rpc_service_tests.js | 15 +- addons/web/static/tests/qunit.js | 5 + 10 files changed, 388 insertions(+), 226 deletions(-) create mode 100644 addons/web/static/src/core/errors/error_utils.js diff --git a/addons/web/static/lib/qunit/qunit-2.9.1.js b/addons/web/static/lib/qunit/qunit-2.9.1.js index ddbdcee3e75..7efc1f1f759 100644 --- a/addons/web/static/lib/qunit/qunit-2.9.1.js +++ b/addons/web/static/lib/qunit/qunit-2.9.1.js @@ -14,6 +14,8 @@ global$1 = global$1 && global$1.hasOwnProperty('default') ? global$1['default'] : global$1; + const debug = odoo.debug; + var window$1 = global$1.window; var self$1 = global$1.self; var console = global$1.console; @@ -5338,7 +5340,7 @@ // Odoo Customisation!!! // Crappy hack to display traceback with sourcemaps if debug=assets - if (lastError && QUnit.annotateTraceback && odoo && odoo.debug && odoo.debug.includes("assets")) { + if (lastError && QUnit.annotateTraceback && debug && debug.includes("assets")) { const pre = assertLi.querySelector("pre"); QUnit.annotateTraceback(lastError).then(traceback => { diff --git a/addons/web/static/src/core/errors/error_dialogs.xml b/addons/web/static/src/core/errors/error_dialogs.xml index 429552c7565..0d2c6e6c7b3 100644 --- a/addons/web/static/src/core/errors/error_dialogs.xml +++ b/addons/web/static/src/core/errors/error_dialogs.xml @@ -56,7 +56,7 @@

diff --git a/addons/web/static/src/core/errors/error_handlers.js b/addons/web/static/src/core/errors/error_handlers.js index cc871d39aae..48d01e9ca6b 100644 --- a/addons/web/static/src/core/errors/error_handlers.js +++ b/addons/web/static/src/core/errors/error_handlers.js @@ -14,7 +14,6 @@ import { UncaughtClientError, UncaughtCorsError, UncaughtPromiseError } from "./ /** * @typedef {import("../../env").OdooEnv} OdooEnv * @typedef {import("./error_service").UncaughtError} UncaughError - * @typedef {(error: UncaughError) => boolean | void} ErrorHandler */ const errorHandlerRegistry = registry.category("error_handlers"); @@ -26,19 +25,18 @@ const errorDialogRegistry = registry.category("error_dialogs"); /** * @param {OdooEnv} env - * @returns {ErrorHandler} + * @param {UncaughError} error + * @returns {boolean} */ -function corsErrorHandler(env) { - return (error) => { - if (error instanceof UncaughtCorsError) { - env.services.dialog.open(NetworkErrorDialog, { - traceback: error.traceback || error.stack, - message: error.message, - name: error.name, - }); - return true; - } - }; +function corsErrorHandler(env, error) { + if (error instanceof UncaughtCorsError) { + env.services.dialog.open(NetworkErrorDialog, { + traceback: error.traceback, + message: error.message, + name: error.name, + }); + return true; + } } errorHandlerRegistry.add("corsErrorHandler", corsErrorHandler, { sequence: 95 }); @@ -48,19 +46,18 @@ errorHandlerRegistry.add("corsErrorHandler", corsErrorHandler, { sequence: 95 }) /** * @param {OdooEnv} env - * @returns {ErrorHandler} + * @param {UncaughError} error + * @returns {boolean} */ -function clientErrorHandler(env) { - return (error) => { - if (error instanceof UncaughtClientError) { - env.services.dialog.open(ClientErrorDialog, { - traceback: error.traceback || error.stack, - message: error.message, - name: error.name, - }); - return true; - } - }; +function clientErrorHandler(env, error) { + if (error instanceof UncaughtClientError) { + env.services.dialog.open(ClientErrorDialog, { + traceback: error.traceback, + message: error.message, + name: error.name, + }); + return true; + } } errorHandlerRegistry.add("clientErrorHandler", clientErrorHandler, { sequence: 96 }); @@ -70,42 +67,41 @@ errorHandlerRegistry.add("clientErrorHandler", clientErrorHandler, { sequence: 9 /** * @param {OdooEnv} env - * @returns {ErrorHandler} + * @param {UncaughError} error + * @param {Error} originalError + * @returns {boolean} */ -function rpcErrorHandler(env) { - return (uncaughtError) => { - if (!(uncaughtError instanceof UncaughtPromiseError)) { - return; +function rpcErrorHandler(env, error, originalError) { + if (!(error instanceof UncaughtPromiseError)) { + return false; + } + if (originalError instanceof RPCError) { + // When an error comes from the server, it can have an exeption name. + // (or any string truly). It is used as key in the error dialog from + // server registry to know which dialog component to use. + // It's how a backend dev can easily map its error to another component. + // Note that for a client side exception, we don't use this registry + // as we can directly assign a value to `component`. + // error is here a RPCError + error.unhandledRejectionEvent.preventDefault(); + const exceptionName = originalError.exceptionName; + let ErrorComponent = null; + if (exceptionName && errorDialogRegistry.contains(exceptionName)) { + ErrorComponent = errorDialogRegistry.get(exceptionName); } - const error = uncaughtError.originalError; - if (error instanceof RPCError) { - // When an error comes from the server, it can have an exeption name. - // (or any string truly). It is used as key in the error dialog from - // server registry to know which dialog component to use. - // It's how a backend dev can easily map its error to another component. - // Note that for a client side exception, we don't use this registry - // as we can directly assign a value to `component`. - // error is here a RPCError - uncaughtError.unhandledRejectionEvent.preventDefault(); - const exceptionName = error.exceptionName; - let ErrorComponent = null; - if (exceptionName && errorDialogRegistry.contains(exceptionName)) { - ErrorComponent = errorDialogRegistry.get(exceptionName); - } - env.services.dialog.open(ErrorComponent || RPCErrorDialog, { - traceback: error.stack, - message: error.message, - name: error.name, - exceptionName: error.exceptionName, - data: error.data, - subType: error.subType, - code: error.code, - type: error.type, - }); - return true; - } - }; + env.services.dialog.open(ErrorComponent || RPCErrorDialog, { + traceback: error.traceback, + message: originalError.message, + name: originalError.name, + exceptionName: originalError.exceptionName, + data: originalError.data, + subType: originalError.subType, + code: originalError.code, + type: originalError.type, + }); + return true; + } } errorHandlerRegistry.add("rpcErrorHandler", rpcErrorHandler, { sequence: 97 }); @@ -113,47 +109,49 @@ errorHandlerRegistry.add("rpcErrorHandler", rpcErrorHandler, { sequence: 97 }); // Lost connection errors // ----------------------------------------------------------------------------- +let connectionLostNotifId = null; /** * @param {OdooEnv} env - * @returns {ErrorHandler} + * @param {UncaughError} error + * @param {Error} originalError + * @returns {boolean} */ -function lostConnectionHandler(env) { - let connectionLostNotifId; - return (uncaughtError) => { - const error = uncaughtError.originalError; - if (error instanceof ConnectionLostError) { - if (connectionLostNotifId) { - // notification already displayed (can occur if there were several - // concurrent rpcs when the connection was lost) - return true; - } - connectionLostNotifId = env.services.notification.create( - env._t("Connection lost. Trying to reconnect..."), - { sticky: true } - ); - let delay = 2000; - browser.setTimeout(function checkConnection() { - env.services - .rpc("/web/webclient/version_info", {}) - .then(function () { - env.services.notification.close(connectionLostNotifId); - connectionLostNotifId = null; - env.services.notification.create( - env._t("Connection restored. You are back online."), - { - type: "info", - } - ); - }) - .catch(() => { - // exponential backoff, with some jitter - delay = delay * 1.5 + 500 * Math.random(); - browser.setTimeout(checkConnection, delay); - }); - }, delay); +function lostConnectionHandler(env, error, originalError) { + if (!(error instanceof UncaughtPromiseError)) { + return false; + } + if (originalError instanceof ConnectionLostError) { + if (connectionLostNotifId) { + // notification already displayed (can occur if there were several + // concurrent rpcs when the connection was lost) return true; } - }; + connectionLostNotifId = env.services.notification.create( + env._t("Connection lost. Trying to reconnect..."), + { sticky: true } + ); + let delay = 2000; + browser.setTimeout(function checkConnection() { + env.services + .rpc("/web/webclient/version_info", {}) + .then(function () { + env.services.notification.close(connectionLostNotifId); + connectionLostNotifId = null; + env.services.notification.create( + env._t("Connection restored. You are back online."), + { + type: "info", + } + ); + }) + .catch(() => { + // exponential backoff, with some jitter + delay = delay * 1.5 + 500 * Math.random(); + browser.setTimeout(checkConnection, delay); + }); + }, delay); + return true; + } } errorHandlerRegistry.add("lostConnectionHandler", lostConnectionHandler, { sequence: 98 }); @@ -163,20 +161,19 @@ errorHandlerRegistry.add("lostConnectionHandler", lostConnectionHandler, { seque /** * @param {OdooEnv} env - * @returns {ErrorHandler} + * @param {UncaughError} error + * @returns {boolean} */ -function emptyRejectionErrorHandler(env) { - return (uncaughtError) => { - if (uncaughtError instanceof UncaughtPromiseError) { - const error = uncaughtError.originalError; - env.services.dialog.open(ClientErrorDialog, { - traceback: error.traceback || error.stack, - message: error.message, - name: error.name, - }); - return true; - } - }; +function emptyRejectionErrorHandler(env, error) { + if (!(error instanceof UncaughtPromiseError)) { + return false; + } + env.services.dialog.open(ClientErrorDialog, { + traceback: error.traceback, + message: error.message, + name: error.name, + }); + return true; } errorHandlerRegistry.add("emptyRejectionErrorHandler", emptyRejectionErrorHandler, { sequence: 99, @@ -188,16 +185,15 @@ errorHandlerRegistry.add("emptyRejectionErrorHandler", emptyRejectionErrorHandle /** * @param {OdooEnv} env - * @returns {ErrorHandler} + * @param {UncaughError} error + * @returns {boolean} */ -function defaultHandler(env) { - return (error) => { - env.services.dialog.open(ErrorDialog, { - traceback: error.traceback || error.stack, - message: error.message, - name: error.name, - }); - return true; - }; +function defaultHandler(env, error) { + env.services.dialog.open(ErrorDialog, { + traceback: error.traceback, + message: error.message, + name: error.name, + }); + return true; } errorHandlerRegistry.add("defaultHandler", defaultHandler, { sequence: 100 }); diff --git a/addons/web/static/src/core/errors/error_service.js b/addons/web/static/src/core/errors/error_service.js index 1a353198148..be707b9ac6a 100644 --- a/addons/web/static/src/core/errors/error_service.js +++ b/addons/web/static/src/core/errors/error_service.js @@ -1,9 +1,9 @@ /** @odoo-module **/ import { browser } from "../browser/browser"; -import { isBrowserChrome } from "../browser/feature_detection"; import { _lt } from "../l10n/translation"; import { registry } from "../registry"; +import { annotateTraceback, formatTraceback, getErrorTechnicalName } from "./error_utils"; /** * Uncaught Errors have 4 properties: @@ -14,41 +14,70 @@ import { registry } from "../registry"; * necessarily an error (for ex, if some code does throw "boom") */ export class UncaughtError extends Error { - constructor(message, name) { + constructor(message) { super(message); - this.name = name || "UncaughtError"; - this.originalError = null; + this.name = getErrorTechnicalName(this); this.traceback = null; } } export class UncaughtClientError extends UncaughtError { constructor(message = _lt("Uncaught Javascript Error")) { - super(message, "UncaughtClientError"); + super(message); } } export class UncaughtPromiseError extends UncaughtError { constructor(message = _lt("Uncaught Promise")) { - super(message, "UncaughtPromiseError"); + super(message); this.unhandledRejectionEvent = null; } } export class UncaughtCorsError extends UncaughtError { constructor(message = _lt("Uncaught CORS Error")) { - super(message, "UncaughtCorsError"); + super(message); + } +} + +/** + * @param {UncaughtError} uncaughtError + * @param {Error} originalError + * @returns {string} + */ +function combineErrorNames(uncaughtError, originalError) { + const originalErrorName = getErrorTechnicalName(originalError); + const uncaughtErrorName = getErrorTechnicalName(uncaughtError); + if (originalErrorName === Error.name) { + return uncaughtErrorName; + } else { + return `${uncaughtErrorName} > ${originalErrorName}`; + } +} + +/** + * @param {import("../../env").OdooEnv} env + * @param {UncaughtError} uncaughtError + * @param {Error} originalError + * @returns {Promise} + */ +async function completeUncaughtError(env, uncaughtError, originalError) { + uncaughtError.name = combineErrorNames(uncaughtError, originalError); + if (env.debug.includes("assets")) { + uncaughtError.traceback = await annotateTraceback(originalError); + } else { + uncaughtError.traceback = formatTraceback(originalError); + } + if (originalError.message) { + uncaughtError.message = `${uncaughtError.message} > ${originalError.message}`; } } export const errorService = { start(env) { - const handlers = registry - .category("error_handlers") - .getAll() - .map((builder) => builder(env)); + const handlers = registry.category("error_handlers").getAll(); - function handleError(error, retry = true) { + function handleError(error, originalError, retry = true) { const services = env.services; if (!services.dialog || !services.notification || !services.rpc) { // here, the environment is not ready to provide feedback to the user. @@ -56,25 +85,25 @@ export const errorService = { // recover. if (retry) { browser.setTimeout(() => { - handleError(error, false); + handleError(error, originalError, false); }, 1000); } return; } for (let handler of handlers) { - if (handler(error, env)) { + if (handler(env, error, originalError)) { break; } } env.bus.trigger("ERROR_DISPATCHED", error); } - window.addEventListener("error", (ev) => { - const { colno, error: eventError, filename, lineno, message } = ev; - let err; + window.addEventListener("error", async (ev) => { + const { colno, error: originalError, filename, lineno, message } = ev; + let uncaughtError; if (!filename && !lineno && !colno) { - err = new UncaughtCorsError(); - err.traceback = env._t( + uncaughtError = new UncaughtCorsError(); + uncaughtError.traceback = env._t( `Unknown CORS error\n\n` + `An unknown CORS error occured.\n` + `The error probably originates from a JavaScript file served from a different origin.\n` + @@ -82,37 +111,23 @@ export const errorService = { ); } else { // ignore Chrome video internal error: https://crbug.com/809574 - if (!eventError && message === "ResizeObserver loop limit exceeded") { + if (!originalError && message === "ResizeObserver loop limit exceeded") { return; } - let stack = eventError ? eventError.stack : ""; - if (!isBrowserChrome()) { - // transforms the stack into a chromium stack - // Chromium stack example: - // Error: Mock: Can't write value - // _onOpenFormView@http://localhost:8069/web/content/425-baf33f1/web.assets.js:1064:30 - // ... - stack = `${message}\n${stack}`.replace(/\n/g, "\n "); - } - err = new UncaughtClientError(); - err.originalError = eventError; - err.traceback = `${message}\n\n${filename}:${lineno}\n${env._t( - "Traceback" - )}:\n${stack}`; + uncaughtError = new UncaughtClientError(); + await completeUncaughtError(env, uncaughtError, originalError); } - handleError(err); + handleError(uncaughtError, originalError); }); - window.addEventListener("unhandledrejection", (ev) => { - const uncaughtError = ev.reason; - const error = new UncaughtPromiseError(); - error.unhandledRejectionEvent = ev; - error.originalError = uncaughtError; - if (uncaughtError instanceof Error) { - error.message = uncaughtError.message; - error.traceback = uncaughtError.stack; // todo: do same computation as regular errors + window.addEventListener("unhandledrejection", async (ev) => { + const originalError = ev.reason; + const uncaughtError = new UncaughtPromiseError(); + uncaughtError.unhandledRejectionEvent = ev; + if (originalError instanceof Error) { + await completeUncaughtError(env, uncaughtError, originalError); } - handleError(error); + handleError(uncaughtError, originalError); }); }, }; diff --git a/addons/web/static/src/core/errors/error_utils.js b/addons/web/static/src/core/errors/error_utils.js new file mode 100644 index 00000000000..3ce0e9ed6f1 --- /dev/null +++ b/addons/web/static/src/core/errors/error_utils.js @@ -0,0 +1,90 @@ +/** @odoo-module **/ + +import { loadAssets } from "../assets"; +import { isBrowserChrome } from "../browser/feature_detection"; + +/** + * @param {Error} error + * @returns {string} + */ +export function getErrorTechnicalName(error) { + return error.name !== Error.name ? error.name : error.constructor.name; +} + +/** + * Format the traceback of an error. Basically, we just add the error message + * in the traceback if necessary (Chrome already does it by default, but not + * other browser.) + * + * @param {Error} error + * @returns {string} + */ +export function formatTraceback(error) { + let traceback = error.stack; + const errorName = getErrorTechnicalName(error); + if (!isBrowserChrome()) { + // transforms the stack into a chromium stack + // Chromium stack example: + // Error: Mock: Can't write value + // _onOpenFormView@http://localhost:8069/web/content/425-baf33f1/web.assets.js:1064:30 + // ... + traceback = `${errorName}: ${error.message}\n${error.stack}`.replace(/\n/g, "\n "); + } else { + // Chromium stack starts with the error's name but the name is "Error" by default + // so we replace it to have the error type name + traceback = error.stack.replace(/^[^:]*/g, errorName); + } + return traceback; +} + +/** + * Returns an annotated traceback from an error. This is asynchronous because + * it needs to fetch the sourcemaps for each script involved in the error, + * then compute the correct file/line numbers and add the information to the + * correct line. + * + * @param {Error} error + * @returns {Promise} + */ +export async function annotateTraceback(error) { + const traceback = formatTraceback(error); + await loadAssets({ + jsLibs: ["/web/static/lib/stacktracejs/stacktrace.js"], + }); + // In Firefox, the error stack generated by anonymous code (example: invalid + // code in a template) is not compatible with the stacktrace lib. This code + // corrects the stack to make it compatible with the lib stacktrace. + if (error.stack) { + const regex = / line (\d*) > (Function):(\d*)/gm; + const subst = `:$1`; + error.stack = error.stack.replace(regex, subst); + } + // eslint-disable-next-line no-undef + const frames = await StackTrace.fromError(error); + const lines = traceback.split("\n"); + if (lines[lines.length - 1].trim() === "") { + // firefox traceback have an empty line at the end + lines.splice(-1); + } + + // Chrome stacks contains some lines with (index 0) which apparently + // corresponds to some native functions (at least Promise.all). We need to + // ignore them because they will not correspond to a stackframe. + const skips = lines.filter((l) => l.includes("(index 0")).length; + const offset = lines.length - frames.length - skips; + let lineIndex = offset; + let frameIndex = 0; + while (frameIndex < frames.length) { + const line = lines[lineIndex]; + if (line.includes("(index 0)")) { + lineIndex++; + continue; + } + const frame = frames[frameIndex]; + const info = ` (${frame.fileName}:${frame.lineNumber})`; + lines[lineIndex] = line + info; + lineIndex++; + frameIndex++; + } + return lines.join("\n"); +} diff --git a/addons/web/static/src/legacy/legacy_promise_error_handler.js b/addons/web/static/src/legacy/legacy_promise_error_handler.js index 4ac8f89d717..0fc7cddfeda 100644 --- a/addons/web/static/src/legacy/legacy_promise_error_handler.js +++ b/addons/web/static/src/legacy/legacy_promise_error_handler.js @@ -4,8 +4,7 @@ import { registry } from "@web/core/registry"; /** * @typedef {import("../env").OdooEnv} OdooEnv - * @typedef {import("../core/errors/error_service").UncaughtError} UncaughError - * @typedef {(error: UncaughError) => boolean | void} ErrorHandler + * @typedef {import("../core/errors/error_service").UncaughtPromiseError} UncaughtPromiseError */ // ----------------------------------------------------------------------------- @@ -14,23 +13,24 @@ import { registry } from "@web/core/registry"; /** * @param {OdooEnv} env - * @returns {ErrorHandler} + * @param {Error} error + * @param {Error} originalError + * @returns {boolean} */ -function legacyRejectPromiseHandler(env) { - return (error) => { - if (error.name === "UncaughtPromiseError") { - const isLegitError = error.originalError && error.originalError instanceof Error; - const isLegacyRPC = error.originalError && error.originalError.legacy; - if (!isLegitError && !isLegacyRPC) { - // we consider that a code throwing something that is not an error is - // a case where it is meant as an asynchronous control flow (as legacy - // code is sadly doing). For now, we just want to consider this as a non - // error, so we prevent default it. - error.unhandledRejectionEvent.preventDefault(); - return true; - } +function legacyRejectPromiseHandler(env, error, originalError) { + if (error.name === "UncaughtPromiseError") { + const isLegitError = originalError && originalError instanceof Error; + const isLegacyRPC = originalError && originalError.legacy; + if (!isLegitError && !isLegacyRPC) { + // we consider that a code throwing something that is not an error is + // a case where it is meant as an asynchronous control flow (as legacy + // code is sadly doing). For now, we just want to consider this as a non + // error, so we prevent default it. + error.unhandledRejectionEvent.preventDefault(); + return true; } - }; + } + return false; } registry diff --git a/addons/web/static/src/legacy/legacy_rpc_error_handler.js b/addons/web/static/src/legacy/legacy_rpc_error_handler.js index 8ed20124d2d..65ab97be354 100644 --- a/addons/web/static/src/legacy/legacy_rpc_error_handler.js +++ b/addons/web/static/src/legacy/legacy_rpc_error_handler.js @@ -2,6 +2,7 @@ import { registry } from "@web/core/registry"; import { RPCErrorDialog } from "../core/errors/error_dialogs"; +import { RPCError } from "../core/network/rpc_service"; const errorDialogRegistry = registry.category("error_dialogs"); const errorHandlerRegistry = registry.category("error_handlers"); @@ -9,7 +10,6 @@ const errorHandlerRegistry = registry.category("error_handlers"); /** * @typedef {import("../env").OdooEnv} OdooEnv * @typedef {import("../core/errors/error_service").UncaughtError} UncaughError - * @typedef {(error: UncaughError) => boolean | void} ErrorHandler */ // ----------------------------------------------------------------------------- @@ -18,38 +18,43 @@ const errorHandlerRegistry = registry.category("error_handlers"); /** * @param {OdooEnv} env - * @returns {ErrorHandler} + * @param {Error} error + * @param {Error} originalError + * @returns {boolean} */ -function legacyRPCErrorHandler(env) { - return (uncaughtError) => { - let error = uncaughtError.originalError; - if (error && error.legacy && error.message && error.message.name === "RPC_ERROR") { - const event = error.event; - error = error.message; - uncaughtError.unhandledRejectionEvent.preventDefault(); - if (event.isDefaultPrevented()) { - // in theory, here, event was already handled - return true; - } - event.preventDefault(); - const exceptionName = error.exceptionName; - let ErrorComponent = error.Component; - if (!ErrorComponent && exceptionName && errorDialogRegistry.contains(exceptionName)) { - ErrorComponent = errorDialogRegistry.get(exceptionName); - } - - env.services.dialog.open(ErrorComponent || RPCErrorDialog, { - traceback: error.traceback || error.stack, - message: error.message, - name: error.name, - exceptionName: error.exceptionName, - data: error.data, - subType: error.subType, - code: error.code, - type: error.type, - }); +function legacyRPCErrorHandler(env, error, originalError) { + if ( + originalError && + originalError.legacy && + originalError.message && + originalError.message instanceof RPCError + ) { + const event = originalError.event; + originalError = originalError.message; + error.unhandledRejectionEvent.preventDefault(); + if (event.isDefaultPrevented()) { + // in theory, here, event was already handled return true; } - }; + event.preventDefault(); + const exceptionName = originalError.exceptionName; + let ErrorComponent = originalError.Component; + if (!ErrorComponent && exceptionName && errorDialogRegistry.contains(exceptionName)) { + ErrorComponent = errorDialogRegistry.get(exceptionName); + } + + env.services.dialog.open(ErrorComponent || RPCErrorDialog, { + traceback: originalError.traceback || originalError.stack, + message: originalError.message, + name: originalError.name, + exceptionName: originalError.exceptionName, + data: originalError.data, + subType: originalError.subType, + code: originalError.code, + type: originalError.type, + }); + return true; + } + return false; } errorHandlerRegistry.add("legacyRPCErrorHandler", legacyRPCErrorHandler, { sequence: 2 }); diff --git a/addons/web/static/tests/core/errors/error_service_tests.js b/addons/web/static/tests/core/errors/error_service_tests.js index 097131354bb..901b5126055 100644 --- a/addons/web/static/tests/core/errors/error_service_tests.js +++ b/addons/web/static/tests/core/errors/error_service_tests.js @@ -18,7 +18,7 @@ import { makeFakeNotificationService, makeFakeRPCService, } from "../../helpers/mock_services"; -import { nextTick, patchWithCleanup } from "../../helpers/utils"; +import { makeDeferred, nextTick, patchWithCleanup } from "../../helpers/utils"; const { Component, tags } = owl; const errorDialogRegistry = registry.category("error_dialogs"); @@ -84,7 +84,7 @@ QUnit.test("handle RPC_ERROR of type='server' and no associated dialog class", a serviceRegistry.add("dialog", makeFakeDialogService(open), { force: true }); await makeTestEnv(); const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null }); - unhandledRejectionCb(errorEvent); + await unhandledRejectionCb(errorEvent); }); QUnit.test( @@ -115,7 +115,7 @@ QUnit.test( await makeTestEnv(); errorDialogRegistry.add("strange_error", CustomDialog); const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null }); - unhandledRejectionCb(errorEvent); + await unhandledRejectionCb(errorEvent); } ); @@ -149,7 +149,7 @@ QUnit.test("handle CONNECTION_LOST_ERROR", async (assert) => { await makeTestEnv({ mockRPC }); const error = new ConnectionLostError(); const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null }); - unhandledRejectionCb(errorEvent); + await unhandledRejectionCb(errorEvent); await nextTick(); // wait for mocked RPCs assert.verifySteps([ "create (Connection lost. Trying to reconnect...)", @@ -163,8 +163,8 @@ QUnit.test("handle CONNECTION_LOST_ERROR", async (assert) => { }); QUnit.test("will let handlers from the registry handle errors first", async (assert) => { - errorHandlerRegistry.add("__test_handler__", (env) => (err) => { - assert.strictEqual(err.originalError, error); + errorHandlerRegistry.add("__test_handler__", (env, err, originalError) => { + assert.strictEqual(originalError, error); assert.strictEqual(env.someValue, 14); assert.step("in handler"); }); @@ -173,7 +173,7 @@ QUnit.test("will let handlers from the registry handle errors first", async (ass const error = new Error(); error.name = "boom"; const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null }); - unhandledRejectionCb(errorEvent); + await unhandledRejectionCb(errorEvent); assert.verifySteps(["in handler"]); }); @@ -186,8 +186,8 @@ QUnit.test("handle uncaught promise errors", async (assert) => { function open(dialogClass, props) { assert.strictEqual(dialogClass, ClientErrorDialog); assert.deepEqual(props, { - name: "TestError", - message: "This is an error test", + name: "UncaughtPromiseError > TestError", + message: "Uncaught Promise > This is an error test", traceback: error.stack, }); } @@ -195,7 +195,7 @@ QUnit.test("handle uncaught promise errors", async (assert) => { await makeTestEnv(); const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null }); - unhandledRejectionCb(errorEvent); + await unhandledRejectionCb(errorEvent); }); QUnit.test("handle uncaught client errors", async (assert) => { @@ -206,7 +206,8 @@ QUnit.test("handle uncaught client errors", async (assert) => { function open(dialogClass, props) { assert.strictEqual(dialogClass, ClientErrorDialog); - assert.strictEqual(props.message, "Uncaught Javascript Error"); + assert.strictEqual(props.name, "UncaughtClientError > TestError"); + assert.strictEqual(props.message, "Uncaught Javascript Error > This is an error test"); } serviceRegistry.add("dialog", makeFakeDialogService(open), { force: true }); await makeTestEnv(); @@ -217,7 +218,7 @@ QUnit.test("handle uncaught client errors", async (assert) => { lineno: 1, filename: "test", }); - errorCb(errorEvent); + await errorCb(errorEvent); }); QUnit.test("handle uncaught CORS errors", async (assert) => { @@ -235,5 +236,42 @@ QUnit.test("handle uncaught CORS errors", async (assert) => { // CORS error event has no colno, no lineno and no filename const errorEvent = new ErrorEvent("error", { error }); - errorCb(errorEvent); + await errorCb(errorEvent); +}); + +QUnit.test("check retry", async (assert) => { + assert.expect(3); + + const def = makeDeferred(); + + patchWithCleanup(browser, { + setTimeout(fn) { + def.then(fn); + }, + }); + + serviceRegistry.remove("dialog"); + const env = await makeTestEnv(); + + env.bus.on("ERROR_DISPATCHED", null, () => { + assert.step("ERROR_DISPATCHED"); + }); + + class TestError extends Error {} + const error = new TestError(); + error.message = "This is an error test"; + error.name = "TestError"; + + const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null }); + await unhandledRejectionCb(errorEvent); + + assert.verifySteps([]); + + serviceRegistry.add("dialog", dialogService); + await nextTick(); + + await def.resolve(); + assert.verifySteps(["ERROR_DISPATCHED"]); + + env.bus.off("ERROR_DISPATCHED", null); }); diff --git a/addons/web/static/tests/core/network/rpc_service_tests.js b/addons/web/static/tests/core/network/rpc_service_tests.js index 22c2df5f608..4f83fe35fcc 100644 --- a/addons/web/static/tests/core/network/rpc_service_tests.js +++ b/addons/web/static/tests/core/network/rpc_service_tests.js @@ -1,14 +1,14 @@ /** @odoo-module **/ import { browser } from "@web/core/browser/browser"; -import { rpcService } from "@web/core/network/rpc_service"; +import { ConnectionAbortedError, rpcService } from "@web/core/network/rpc_service"; import { notificationService } from "@web/core/notifications/notification_service"; import { registry } from "@web/core/registry"; import { useService } from "@web/core/service_hook"; import { patch, unpatch } from "@web/core/utils/patch"; import { makeTestEnv } from "../../helpers/mock_env"; import { makeMockXHR } from "../../helpers/mock_services"; -import { getFixture, makeDeferred, nextTick } from "../../helpers/utils"; +import { getFixture, makeDeferred, nextTick, patchWithCleanup } from "../../helpers/utils"; const { Component, mount, tags } = owl; const { xml } = tags; @@ -227,3 +227,14 @@ QUnit.test("check trigger RPC:REQUEST and RPC:RESPONSE for a rpc with an error", assert.verifySteps(["RPC:REQUEST", "RPC:RESPONSE"]); unpatch(browser, "mock.xhr"); }); + +QUnit.test("check connection aborted", async (assert) => { + const def = makeDeferred(); + let MockXHR = makeMockXHR({}, () => {}, def); + patchWithCleanup(browser, { XMLHttpRequest: MockXHR }, { pure: true }); + const env = await makeTestEnv({ serviceRegistry }); + + const connection = env.services.rpc(); + connection.abort(); + assert.rejects(connection, ConnectionAbortedError); +}); diff --git a/addons/web/static/tests/qunit.js b/addons/web/static/tests/qunit.js index 087526f17d9..55642f5858a 100644 --- a/addons/web/static/tests/qunit.js +++ b/addons/web/static/tests/qunit.js @@ -331,6 +331,11 @@ }); QUnit.begin(function () { + if (odoo.__DEBUG__.services["@web/core/errors/error_utils"]) { + const errorUtils = odoo.__DEBUG__.services["@web/core/errors/error_utils"]; + const { annotateTraceback } = errorUtils; + QUnit.annotateTraceback = annotateTraceback; + } const config = QUnit.config; if (config.failfast) { QUnit.testDone(function (details) {