From b59024f6ba8a9c1b86355176882ab8afb41811d4 Mon Sep 17 00:00:00 2001 From: "Michael Mattiello (mcm)" Date: Thu, 27 May 2021 08:45:02 +0000 Subject: [PATCH] [IMP] web: use the error class to handle errors Before this commit, error handlers checked the error name to know if they should be executed. Now, they check the instance type of the error to be more accurate on which error an handler deals with. This commit also changes `emptyRejectionErrorHandler` sequence number because it had more priority than `rpcErrorHandler` and `lostConnectionHandler`. --- .../static/src/core/errors/error_handlers.js | 76 ++++++++++-------- .../static/src/core/errors/error_service.js | 6 +- .../static/src/core/network/rpc_service.js | 18 ++--- .../tests/core/errors/error_service_tests.js | 79 +++++++++++++++++-- 4 files changed, 122 insertions(+), 57 deletions(-) diff --git a/addons/web/static/src/core/errors/error_handlers.js b/addons/web/static/src/core/errors/error_handlers.js index 197a0831294..cc871d39aae 100644 --- a/addons/web/static/src/core/errors/error_handlers.js +++ b/addons/web/static/src/core/errors/error_handlers.js @@ -1,6 +1,7 @@ /** @odoo-module **/ import { browser } from "../browser/browser"; +import { ConnectionLostError, RPCError } from "../network/rpc_service"; import { registry } from "../registry"; import { ClientErrorDialog, @@ -8,6 +9,7 @@ import { NetworkErrorDialog, RPCErrorDialog, } from "./error_dialogs"; +import { UncaughtClientError, UncaughtCorsError, UncaughtPromiseError } from "./error_service"; /** * @typedef {import("../../env").OdooEnv} OdooEnv @@ -28,7 +30,7 @@ const errorDialogRegistry = registry.category("error_dialogs"); */ function corsErrorHandler(env) { return (error) => { - if (error.name === "UNKNOWN_CORS_ERROR") { + if (error instanceof UncaughtCorsError) { env.services.dialog.open(NetworkErrorDialog, { traceback: error.traceback || error.stack, message: error.message, @@ -50,7 +52,7 @@ errorHandlerRegistry.add("corsErrorHandler", corsErrorHandler, { sequence: 95 }) */ function clientErrorHandler(env) { return (error) => { - if (error.name === "UNCAUGHT_CLIENT_ERROR") { + if (error instanceof UncaughtClientError) { env.services.dialog.open(ClientErrorDialog, { traceback: error.traceback || error.stack, message: error.message, @@ -62,29 +64,6 @@ function clientErrorHandler(env) { } errorHandlerRegistry.add("clientErrorHandler", clientErrorHandler, { sequence: 96 }); -// ----------------------------------------------------------------------------- -// Empty rejection errors -// ----------------------------------------------------------------------------- - -/** - * @param {OdooEnv} env - * @returns {ErrorHandler} - */ -function emptyRejectionErrorHandler(env) { - return (error) => { - if (error.name === "UNCAUGHT_EMPTY_REJECTION_ERROR") { - env.services.dialog.open(ClientErrorDialog, { - message: error.message, - name: error.name, - }); - return true; - } - }; -} -errorHandlerRegistry.add("emptyRejectionErrorHandler", emptyRejectionErrorHandler, { - sequence: 97, -}); - // ----------------------------------------------------------------------------- // RPC errors // ----------------------------------------------------------------------------- @@ -95,8 +74,11 @@ errorHandlerRegistry.add("emptyRejectionErrorHandler", emptyRejectionErrorHandle */ function rpcErrorHandler(env) { return (uncaughtError) => { + if (!(uncaughtError instanceof UncaughtPromiseError)) { + return; + } const error = uncaughtError.originalError; - if (error && error.name === "RPC_ERROR") { + 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. @@ -106,13 +88,13 @@ function rpcErrorHandler(env) { // error is here a RPCError uncaughtError.unhandledRejectionEvent.preventDefault(); const exceptionName = error.exceptionName; - let ErrorComponent = error.Component; - if (!ErrorComponent && exceptionName && errorDialogRegistry.contains(exceptionName)) { + let ErrorComponent = null; + if (exceptionName && errorDialogRegistry.contains(exceptionName)) { ErrorComponent = errorDialogRegistry.get(exceptionName); } env.services.dialog.open(ErrorComponent || RPCErrorDialog, { - traceback: error.traceback || error.stack, + traceback: error.stack, message: error.message, name: error.name, exceptionName: error.exceptionName, @@ -125,7 +107,7 @@ function rpcErrorHandler(env) { } }; } -errorHandlerRegistry.add("rpcErrorHandler", rpcErrorHandler, { sequence: 98 }); +errorHandlerRegistry.add("rpcErrorHandler", rpcErrorHandler, { sequence: 97 }); // ----------------------------------------------------------------------------- // Lost connection errors @@ -139,7 +121,7 @@ function lostConnectionHandler(env) { let connectionLostNotifId; return (uncaughtError) => { const error = uncaughtError.originalError; - if (error && error.name === "CONNECTION_LOST_ERROR") { + if (error instanceof ConnectionLostError) { if (connectionLostNotifId) { // notification already displayed (can occur if there were several // concurrent rpcs when the connection was lost) @@ -163,7 +145,7 @@ function lostConnectionHandler(env) { } ); }) - .catch((e) => { + .catch(() => { // exponential backoff, with some jitter delay = delay * 1.5 + 500 * Math.random(); browser.setTimeout(checkConnection, delay); @@ -173,7 +155,32 @@ function lostConnectionHandler(env) { } }; } -errorHandlerRegistry.add("lostConnectionHandler", lostConnectionHandler, { sequence: 99 }); +errorHandlerRegistry.add("lostConnectionHandler", lostConnectionHandler, { sequence: 98 }); + +// ----------------------------------------------------------------------------- +// Empty rejection errors +// ----------------------------------------------------------------------------- + +/** + * @param {OdooEnv} env + * @returns {ErrorHandler} + */ +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; + } + }; +} +errorHandlerRegistry.add("emptyRejectionErrorHandler", emptyRejectionErrorHandler, { + sequence: 99, +}); // ----------------------------------------------------------------------------- // Default handler @@ -185,8 +192,7 @@ errorHandlerRegistry.add("lostConnectionHandler", lostConnectionHandler, { seque */ function defaultHandler(env) { return (error) => { - const DialogComponent = error.Component || ErrorDialog; - env.services.dialog.open(DialogComponent, { + env.services.dialog.open(ErrorDialog, { traceback: error.traceback || error.stack, message: error.message, name: error.name, diff --git a/addons/web/static/src/core/errors/error_service.js b/addons/web/static/src/core/errors/error_service.js index 184f5aa201b..1a353198148 100644 --- a/addons/web/static/src/core/errors/error_service.js +++ b/addons/web/static/src/core/errors/error_service.js @@ -22,20 +22,20 @@ export class UncaughtError extends Error { } } -class UncaughtClientError extends UncaughtError { +export class UncaughtClientError extends UncaughtError { constructor(message = _lt("Uncaught Javascript Error")) { super(message, "UncaughtClientError"); } } -class UncaughtPromiseError extends UncaughtError { +export class UncaughtPromiseError extends UncaughtError { constructor(message = _lt("Uncaught Promise")) { super(message, "UncaughtPromiseError"); this.unhandledRejectionEvent = null; } } -class UncaughtCorsError extends UncaughtError { +export class UncaughtCorsError extends UncaughtError { constructor(message = _lt("Uncaught CORS Error")) { super(message, "UncaughtCorsError"); } diff --git a/addons/web/static/src/core/network/rpc_service.js b/addons/web/static/src/core/network/rpc_service.js index 62ba9e53905..38d9c25e60b 100644 --- a/addons/web/static/src/core/network/rpc_service.js +++ b/addons/web/static/src/core/network/rpc_service.js @@ -11,22 +11,16 @@ export class RPCError extends Error { super(...arguments); this.name = "RPC_ERROR"; this.type = "server"; + this.code = null; + this.data = null; + this.exceptionName = null; + this.subType = null; } } -export class ConnectionLostError extends Error { - constructor() { - super(...arguments); - this.name = "CONNECTION_LOST_ERROR"; - } -} +export class ConnectionLostError extends Error {} -export class ConnectionAbortedError extends Error { - constructor() { - super(...arguments); - this.name = "CONNECTION_ABORTED_ERROR"; - } -} +export class ConnectionAbortedError extends Error {} // ----------------------------------------------------------------------------- // Main RPC method 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 547e9f3d712..097131354bb 100644 --- a/addons/web/static/tests/core/errors/error_service_tests.js +++ b/addons/web/static/tests/core/errors/error_service_tests.js @@ -2,7 +2,11 @@ import { browser } from "@web/core/browser/browser"; import { dialogService } from "@web/core/dialog/dialog_service"; -import { RPCErrorDialog } from "@web/core/errors/error_dialogs"; +import { + ClientErrorDialog, + RPCErrorDialog, + NetworkErrorDialog, +} from "@web/core/errors/error_dialogs"; import { errorService } from "@web/core/errors/error_service"; import { ConnectionLostError, RPCError } from "@web/core/network/rpc_service"; import { notificationService } from "@web/core/notifications/notification_service"; @@ -30,7 +34,7 @@ function makeFakeDialogService(open) { }; } -let errorCb; // unused ? +let errorCb; let unhandledRejectionCb; QUnit.module("Error Service", { @@ -73,7 +77,7 @@ QUnit.test("handle RPC_ERROR of type='server' and no associated dialog class", a }, subType: "strange_error", message: "Some strange error occured", - exceptionName: undefined, + exceptionName: null, traceback: error.stack, }); } @@ -93,17 +97,17 @@ QUnit.test( const error = new RPCError(); error.code = 701; error.message = "Some strange error occured"; - error.Component = CustomDialog; + error.exceptionName = "strange_error"; function open(dialogClass, props) { assert.strictEqual(dialogClass, CustomDialog); assert.deepEqual(props, { name: "RPC_ERROR", type: "server", code: 701, - data: undefined, - subType: undefined, + data: null, + subType: null, message: "Some strange error occured", - exceptionName: undefined, + exceptionName: "strange_error", traceback: error.stack, }); } @@ -172,3 +176,64 @@ QUnit.test("will let handlers from the registry handle errors first", async (ass unhandledRejectionCb(errorEvent); assert.verifySteps(["in handler"]); }); + +QUnit.test("handle uncaught promise errors", async (assert) => { + class TestError extends Error {} + const error = new TestError(); + error.message = "This is an error test"; + error.name = "TestError"; + + function open(dialogClass, props) { + assert.strictEqual(dialogClass, ClientErrorDialog); + assert.deepEqual(props, { + name: "TestError", + message: "This is an error test", + traceback: error.stack, + }); + } + serviceRegistry.add("dialog", makeFakeDialogService(open), { force: true }); + await makeTestEnv(); + + const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null }); + unhandledRejectionCb(errorEvent); +}); + +QUnit.test("handle uncaught client errors", async (assert) => { + class TestError extends Error {} + const error = new TestError(); + error.message = "This is an error test"; + error.name = "TestError"; + + function open(dialogClass, props) { + assert.strictEqual(dialogClass, ClientErrorDialog); + assert.strictEqual(props.message, "Uncaught Javascript Error"); + } + serviceRegistry.add("dialog", makeFakeDialogService(open), { force: true }); + await makeTestEnv(); + + const errorEvent = new ErrorEvent("error", { + error, + colno: 1, + lineno: 1, + filename: "test", + }); + errorCb(errorEvent); +}); + +QUnit.test("handle uncaught CORS errors", async (assert) => { + class TestError extends Error {} + const error = new TestError(); + error.message = "This is a cors error"; + error.name = "CORS error"; + + function open(dialogClass, props) { + assert.strictEqual(dialogClass, NetworkErrorDialog); + assert.strictEqual(props.message, "Uncaught CORS Error"); + } + serviceRegistry.add("dialog", makeFakeDialogService(open), { force: true }); + await makeTestEnv(); + + // CORS error event has no colno, no lineno and no filename + const errorEvent = new ErrorEvent("error", { error }); + errorCb(errorEvent); +});