From f7f16bd85daba6ef10f2eb96e5f5fafd2c1f6ab1 Mon Sep 17 00:00:00 2001 From: Samuel Degueldre Date: Mon, 19 Dec 2022 09:19:16 +0000 Subject: [PATCH] [FIX] web: correctly log full traceback for uncaught errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, we added some handling so that errors that were not defaultPrevented would have their traceback logged by the error service instead of the default behaviour of the browser, because not all browsers correctly log error chains (errors with causes). This did not actually work because the errorEvent was written on the object at the top of the error chain and checked on the error at the bottom of it. This commit fixes that by just looking at the event on the uncaught error. closes odoo/odoo#108463 X-original-commit: cefba026ad7fe453da840a948e25d29d4d1837e9 Signed-off-by: Géry Debongnie Signed-off-by: Samuel Degueldre --- .../tests/base_automation_error_dialog.js | 30 ++++--- .../static/src/core/errors/error_service.js | 10 +-- .../tests/core/errors/error_service_tests.js | 82 +++++++++++++++++++ 3 files changed, 106 insertions(+), 16 deletions(-) diff --git a/addons/base_automation/static/tests/base_automation_error_dialog.js b/addons/base_automation/static/tests/base_automation_error_dialog.js index 869d2c0259a..d0a41e570ee 100644 --- a/addons/base_automation/static/tests/base_automation_error_dialog.js +++ b/addons/base_automation/static/tests/base_automation_error_dialog.js @@ -87,11 +87,16 @@ QUnit.module("base_automation", {}, function () { const { Component: Container, props } = registry.category("main_components").get("DialogContainer"); await mount(Container, target, { env, props }); - const errorEvent = new PromiseRejectionEvent("error", { reason: { - message: error, - legacy: true, - event: $.Event(), - }, promise: null }); + const errorEvent = new PromiseRejectionEvent("error", { + reason: { + message: error, + legacy: true, + event: $.Event(), + }, + promise: null, + cancelable: true, + bubbles: true, + }); await unhandledRejectionCb(errorEvent); await nextTick(); assert.containsOnce(target, '.modal .fa-clipboard'); @@ -115,11 +120,16 @@ QUnit.module("base_automation", {}, function () { const { Component: Container, props } = registry.category("main_components").get("DialogContainer"); await mount(Container, target, { env, props }); - const errorEvent = new PromiseRejectionEvent("error", { reason: { - message: error, - legacy: true, - event: $.Event(), - }, promise: null }); + const errorEvent = new PromiseRejectionEvent("error", { + reason: { + message: error, + legacy: true, + event: $.Event(), + }, + promise: null, + cancelable: true, + bubbles: true, + }); await unhandledRejectionCb(errorEvent); await nextTick(); assert.containsOnce(target, '.modal .fa-clipboard'); diff --git a/addons/web/static/src/core/errors/error_service.js b/addons/web/static/src/core/errors/error_service.js index ca83d9438fd..519db81130b 100644 --- a/addons/web/static/src/core/errors/error_service.js +++ b/addons/web/static/src/core/errors/error_service.js @@ -64,13 +64,9 @@ export const errorService = { break; } } - if ( - originalError instanceof Error && - originalError.errorEvent && - !originalError.errorEvent.defaultPrevented - ) { + if (uncaughtError.event && !uncaughtError.event.defaultPrevented) { // Log the full traceback instead of letting the browser log the incomplete one - originalError.errorEvent.preventDefault(); + uncaughtError.event.preventDefault(); console.error(uncaughtError.traceback); } } @@ -97,6 +93,7 @@ export const errorService = { ); } else { uncaughtError = new UncaughtClientError(); + uncaughtError.event = ev; if (error instanceof Error) { error.errorEvent = ev; const annotated = env.debug && env.debug.includes("assets"); @@ -111,6 +108,7 @@ export const errorService = { const error = ev.reason; const uncaughtError = new UncaughtPromiseError(); uncaughtError.unhandledRejectionEvent = ev; + uncaughtError.event = ev; if (error instanceof Error) { error.errorEvent = ev; const annotated = env.debug && env.debug.includes("assets"); 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 3688e6e0efd..7eb48fccb89 100644 --- a/addons/web/static/tests/core/errors/error_service_tests.js +++ b/addons/web/static/tests/core/errors/error_service_tests.js @@ -419,3 +419,85 @@ QUnit.test("lazy loaded handlers", async (assert) => { await unhandledRejectionCb(errorEvent); assert.verifySteps(["in handler"]); }); + +// The following test(s) do not want the preventDefault to be done automatically. +QUnit.module("Error Service", { + beforeEach() { + serviceRegistry.add("error", errorService); + serviceRegistry.add("dialog", dialogService); + serviceRegistry.add("notification", notificationService); + serviceRegistry.add("rpc", makeFakeRPCService()); + serviceRegistry.add("localization", makeFakeLocalizationService()); + serviceRegistry.add("ui", uiService); + const windowAddEventListener = browser.addEventListener; + browser.addEventListener = (type, cb) => { + if (type === "unhandledrejection") { + unhandledRejectionCb = cb; + } else if (type === "error") { + errorCb = cb; + } + }; + registerCleanup(() => { + browser.addEventListener = windowAddEventListener; + }); + }, +}); + +QUnit.test("logs the traceback of the full error chain for unhandledrejection", async (assert) => { + assert.expect(2); + const regexParts = [ + /^.*This is a wrapper error/, + /Caused by:.*This is a second wrapper error/, + /Caused by:.*This is the original error/, + ]; + const errorRegex = new RegExp(regexParts.map((re) => re.source).join(/[\s\S]*/.source)); + patchWithCleanup(console, { + error(errorMessage) { + assert.ok(errorRegex.test(errorMessage)); + }, + }); + + const error = new Error("This is a wrapper error"); + error.cause = new Error("This is a second wrapper error"); + error.cause.cause = new Error("This is the original error"); + + // start the services + await makeTestEnv(); + const errorEvent = new PromiseRejectionEvent("unhandledrejection", { + reason: error, + promise: null, + cancelable: true, + }); + await unhandledRejectionCb(errorEvent); + assert.ok(errorEvent.defaultPrevented); +}); + +QUnit.test("logs the traceback of the full error chain for uncaughterror", async (assert) => { + assert.expect(2); + const regexParts = [ + /^.*This is a wrapper error/, + /Caused by:.*This is a second wrapper error/, + /Caused by:.*This is the original error/, + ]; + const errorRegex = new RegExp(regexParts.map((re) => re.source).join(/[\s\S]*/.source)); + patchWithCleanup(console, { + error(errorMessage) { + assert.ok(errorRegex.test(errorMessage)); + }, + }); + + const error = new Error("This is a wrapper error"); + error.cause = new Error("This is a second wrapper error"); + error.cause.cause = new Error("This is the original error"); + + // start the services + await makeTestEnv(); + const errorEvent = new Event("error", { + promise: null, + cancelable: true, + }); + errorEvent.error = error; + errorEvent.filename = "dummy_file.js"; // needed to not be treated as a CORS error + await errorCb(errorEvent); + assert.ok(errorEvent.defaultPrevented); +});