From dd6f103b1635868e03a7aa40eb7186861d22d377 Mon Sep 17 00:00:00 2001
From: Samuel Degueldre <sad@odoo.com>
Date: Mon, 28 Nov 2022 08:55:44 +0000
Subject: [PATCH] [FIX] web: make originalError the root of the error chain
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, when calling error handlers, we would give them a new error
created by the error service, as well as an `originalError` which is the
error that was caught by the error service. In some cases, this error is
part of an error chain, eg: errors that happen in the owl lifecycle are
wrapped in an `OwlError`, so that when an error occurs in a callback
passed to a lifecycle hook, the position where the hook was called to
register the offending callback is part of the stack trace.
In practice, most error handlers only care about the error that caused
the entire error chain (the root) when deciding whether they should
handle the error or not, eg: an RPCError should be handled by the rpc
error handler, regardless of whether it occured in an event handler or
in a component's willStart. It already reads the traceback to display on
the UncaughtError provided by the error service as this traceback
contains the entire error chain stack information and is annotated with
source maps by the error service.
This commit makes it so that the `originalError` passed to handlers is
now the root of the error chain, it also sets the `cause` of the
UncaughtError which is passed to handlers as the error that was caught
by the error service, so that if a handler needs to inspect the error
chain to know whether it can handle the error or not, it can still do
that.
related to OPW-3069633
closes odoo/odoo#106661
Signed-off-by: Michaël Mattiello <mcm@odoo.com>
---
.../static/src/core/errors/error_dialogs.js | 2 +-
.../static/src/core/errors/error_handlers.js | 78 +++----------------
.../static/src/core/errors/error_service.js | 36 +++++----
.../src/legacy/legacy_rpc_error_handler.js | 7 +-
.../tests/core/errors/error_service_tests.js | 58 +++++++++++++-
5 files changed, 89 insertions(+), 92 deletions(-)
diff --git a/addons/web/static/src/core/errors/error_dialogs.js b/addons/web/static/src/core/errors/error_dialogs.js
index 337362149492..d8ddd56b7df8 100644
--- a/addons/web/static/src/core/errors/error_dialogs.js
+++ b/addons/web/static/src/core/errors/error_dialogs.js
@@ -63,7 +63,7 @@ export class RPCErrorDialog extends ErrorDialog {
this.inferTitle();
this.traceback = this.props.traceback;
if (this.props.data && this.props.data.debug) {
- this.traceback = `${this.props.data.debug}`;
+ this.traceback = `${this.props.data.debug}\nThe above server error caused the following client error:\n${this.traceback}`;
}
}
inferTitle() {
diff --git a/addons/web/static/src/core/errors/error_handlers.js b/addons/web/static/src/core/errors/error_handlers.js
index 3a35daec50a2..8b686f7d4a71 100644
--- a/addons/web/static/src/core/errors/error_handlers.js
+++ b/addons/web/static/src/core/errors/error_handlers.js
@@ -20,48 +20,6 @@ const errorHandlerRegistry = registry.category("error_handlers");
const errorDialogRegistry = registry.category("error_dialogs");
const errorNotificationRegistry = registry.category("error_notifications");
-// -----------------------------------------------------------------------------
-// CORS errors
-// -----------------------------------------------------------------------------
-
-/**
- * @param {OdooEnv} env
- * @param {UncaughError} error
- * @returns {boolean}
- */
-function corsErrorHandler(env, error) {
- if (error instanceof UncaughtCorsError) {
- env.services.dialog.add(NetworkErrorDialog, {
- traceback: error.traceback,
- message: error.message,
- name: error.name,
- });
- return true;
- }
-}
-errorHandlerRegistry.add("corsErrorHandler", corsErrorHandler, { sequence: 95 });
-
-// -----------------------------------------------------------------------------
-// Client errors
-// -----------------------------------------------------------------------------
-
-/**
- * @param {OdooEnv} env
- * @param {UncaughError} error
- * @returns {boolean}
- */
-function clientErrorHandler(env, error) {
- if (error instanceof UncaughtClientError) {
- env.services.dialog.add(ClientErrorDialog, {
- traceback: error.traceback,
- message: error.message,
- name: error.name,
- });
- return true;
- }
-}
-errorHandlerRegistry.add("clientErrorHandler", clientErrorHandler, { sequence: 96 });
-
// -----------------------------------------------------------------------------
// RPC errors
// -----------------------------------------------------------------------------
@@ -171,41 +129,27 @@ export function lostConnectionHandler(env, error, originalError) {
}
errorHandlerRegistry.add("lostConnectionHandler", lostConnectionHandler, { sequence: 98 });
-// -----------------------------------------------------------------------------
-// Empty rejection errors
-// -----------------------------------------------------------------------------
-
-/**
- * @param {OdooEnv} env
- * @param {UncaughError} error
- * @returns {boolean}
- */
-function emptyRejectionErrorHandler(env, error) {
- if (!(error instanceof UncaughtPromiseError)) {
- return false;
- }
- env.services.dialog.add(ClientErrorDialog, {
- traceback: error.traceback,
- message: error.message,
- name: error.name,
- });
- return true;
-}
-errorHandlerRegistry.add("emptyRejectionErrorHandler", emptyRejectionErrorHandler, {
- sequence: 99,
-});
-
// -----------------------------------------------------------------------------
// Default handler
// -----------------------------------------------------------------------------
+const defaultDialogs = new Map([
+ [UncaughtClientError, ClientErrorDialog],
+ [UncaughtPromiseError, ClientErrorDialog],
+ [UncaughtCorsError, NetworkErrorDialog],
+]);
+
/**
+ * Handles the errors based on the very general error categories emitted by the
+ * error service. Notice how we do not look at the original error at all.
+ *
* @param {OdooEnv} env
* @param {UncaughError} error
* @returns {boolean}
*/
function defaultHandler(env, error) {
- env.services.dialog.add(ErrorDialog, {
+ const DialogComponent = defaultDialogs.get(error.constructor) || ErrorDialog;
+ env.services.dialog.add(DialogComponent, {
traceback: error.traceback,
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 563ab3ad8e70..ca83d9438fd6 100644
--- a/addons/web/static/src/core/errors/error_service.js
+++ b/addons/web/static/src/core/errors/error_service.js
@@ -42,7 +42,11 @@ export class UncaughtCorsError extends UncaughtError {
export const errorService = {
start(env) {
- function handleError(error, originalError, retry = true) {
+ function handleError(uncaughtError, retry = true) {
+ let originalError = uncaughtError;
+ while (originalError && "cause" in originalError) {
+ originalError = originalError.cause;
+ }
const services = env.services;
if (!services.dialog || !services.notification || !services.rpc) {
// here, the environment is not ready to provide feedback to the user.
@@ -50,13 +54,13 @@ export const errorService = {
// recover.
if (retry) {
browser.setTimeout(() => {
- handleError(error, originalError, false);
+ handleError(uncaughtError, false);
}, 1000);
}
return;
}
for (const handler of registry.category("error_handlers").getAll()) {
- if (handler(env, error, originalError)) {
+ if (handler(env, uncaughtError, originalError)) {
break;
}
}
@@ -67,19 +71,19 @@ export const errorService = {
) {
// Log the full traceback instead of letting the browser log the incomplete one
originalError.errorEvent.preventDefault();
- console.error(error.traceback);
+ console.error(uncaughtError.traceback);
}
}
browser.addEventListener("error", async (ev) => {
- const { colno, error: originalError, filename, lineno, message } = ev;
+ const { colno, error, filename, lineno, message } = ev;
const errorsToIgnore = [
// Ignore some unnecessary "ResizeObserver loop limit exceeded" error in Firefox.
"ResizeObserver loop completed with undelivered notifications.",
// ignore Chrome video internal error: https://crbug.com/809574
"ResizeObserver loop limit exceeded",
];
- if (!originalError && errorsToIgnore.includes(message)) {
+ if (!error && errorsToIgnore.includes(message)) {
return;
}
let uncaughtError;
@@ -93,25 +97,27 @@ export const errorService = {
);
} else {
uncaughtError = new UncaughtClientError();
- if (originalError instanceof Error) {
- originalError.errorEvent = ev;
+ if (error instanceof Error) {
+ error.errorEvent = ev;
const annotated = env.debug && env.debug.includes("assets");
- await completeUncaughtError(uncaughtError, originalError, annotated);
+ await completeUncaughtError(uncaughtError, error, annotated);
}
}
- handleError(uncaughtError, originalError);
+ uncaughtError.cause = error;
+ handleError(uncaughtError);
});
browser.addEventListener("unhandledrejection", async (ev) => {
- const originalError = ev.reason;
+ const error = ev.reason;
const uncaughtError = new UncaughtPromiseError();
uncaughtError.unhandledRejectionEvent = ev;
- if (originalError instanceof Error) {
- originalError.errorEvent = ev;
+ if (error instanceof Error) {
+ error.errorEvent = ev;
const annotated = env.debug && env.debug.includes("assets");
- await completeUncaughtError(uncaughtError, originalError, annotated);
+ await completeUncaughtError(uncaughtError, error, annotated);
}
- handleError(uncaughtError, originalError);
+ uncaughtError.cause = error;
+ handleError(uncaughtError);
});
},
};
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 63903671536f..9321da2fa898 100644
--- a/addons/web/static/src/legacy/legacy_rpc_error_handler.js
+++ b/addons/web/static/src/legacy/legacy_rpc_error_handler.js
@@ -4,8 +4,6 @@ import { registry } from "@web/core/registry";
import { ConnectionLostError, RPCError } from "../core/network/rpc_service";
import { lostConnectionHandler, rpcErrorHandler } from "@web/core/errors/error_handlers";
-import { OwlError } from "@odoo/owl";
-
const errorHandlerRegistry = registry.category("error_handlers");
/**
@@ -19,14 +17,11 @@ const errorHandlerRegistry = registry.category("error_handlers");
/**
* @param {OdooEnv} env
- * @param {Error} error
+ * @param {UncaughError} error
* @param {Error} originalError
* @returns {boolean}
*/
function legacyRPCErrorHandler(env, error, originalError) {
- if (originalError instanceof OwlError) {
- originalError = originalError.cause;
- }
if (
originalError &&
originalError.legacy &&
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 12858fc3913b..6ad816d08d59 100644
--- a/addons/web/static/tests/core/errors/error_service_tests.js
+++ b/addons/web/static/tests/core/errors/error_service_tests.js
@@ -7,7 +7,7 @@ import {
RPCErrorDialog,
NetworkErrorDialog,
} from "@web/core/errors/error_dialogs";
-import { errorService } from "@web/core/errors/error_service";
+import { errorService, UncaughtPromiseError } from "@web/core/errors/error_service";
import { ConnectionLostError, RPCError } from "@web/core/network/rpc_service";
import { notificationService } from "@web/core/notifications/notification_service";
import { registry } from "@web/core/registry";
@@ -20,9 +20,9 @@ import {
makeFakeNotificationService,
makeFakeRPCService,
} from "../../helpers/mock_services";
-import { makeDeferred, nextTick, patchWithCleanup } from "../../helpers/utils";
+import { getFixture, makeDeferred, mount, nextTick, patchWithCleanup } from "../../helpers/utils";
-import { Component, xml } from "@odoo/owl";
+import { Component, xml, onError, OwlError, onWillStart } from "@odoo/owl";
const errorDialogRegistry = registry.category("error_dialogs");
const errorHandlerRegistry = registry.category("error_handlers");
const serviceRegistry = registry.category("services");
@@ -242,6 +242,58 @@ QUnit.test("will let handlers from the registry handle errors first", async (ass
assert.verifySteps(["in handler"]);
});
+QUnit.test("originalError is the root cause of the error chain", async (assert) => {
+ errorHandlerRegistry.add("__test_handler__", (env, err, originalError) => {
+ assert.ok(err instanceof UncaughtPromiseError); // Wrapped by error service
+ assert.ok(err.cause instanceof OwlError); // Wrapped by owl
+ assert.strictEqual(err.cause.cause, originalError); // original error
+ assert.step("in handler");
+ });
+ const testEnv = await makeTestEnv();
+ testEnv.someValue = 14;
+ const error = new Error();
+ error.name = "boom";
+
+ class ErrHandler extends Component {
+ setup() {
+ onError(async (err) => {
+ await unhandledRejectionCb(
+ new PromiseRejectionEvent("error", {
+ reason: err,
+ promise: null,
+ cancelable: true,
+ })
+ );
+ prom.resolve();
+ });
+ }
+ }
+ ErrHandler.template = xml`<t t-component="props.comp"/>`;
+ class ThrowInSetup extends Component {
+ setup() {
+ throw error;
+ }
+ }
+ ThrowInSetup.template = xml``;
+ let prom = makeDeferred();
+ mount(ErrHandler, getFixture(), { props: { comp: ThrowInSetup } });
+ await prom;
+ assert.verifySteps(["in handler"]);
+
+ class ThrowInWillStart extends Component {
+ setup() {
+ onWillStart(() => {
+ throw error;
+ });
+ }
+ }
+ ThrowInWillStart.template = xml``;
+ prom = makeDeferred();
+ mount(ErrHandler, getFixture(), { props: { comp: ThrowInWillStart } });
+ await prom;
+ assert.verifySteps(["in handler"]);
+});
+
QUnit.test("handle uncaught promise errors", async (assert) => {
class TestError extends Error {}
const error = new TestError();
--
GitLab