From 23e93f9341e0beb24712f0f48ba6777123e669af Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Wed, 28 Sep 2022 13:19:26 +0000 Subject: [PATCH] [FIX] web: Mutex: do not fork rejected promise Have a many2one field in a form view, and try to quick create a record. If the name_create returns an error (e.g. ValidationError in the model), the many2one field opens a form in a dialog (aka the slow create). It thus properly handle the error. However, before this commit, the error returned by the RPC was also displayed in its own dialog (behind the form dialog). This was due to a mistake in the Mutex, which forks the promise returned by the registered action. When that promise was rejected, the forked one was thus rejected as well, and it wasn't caught. That later promise is only used to unlock the mutex, so we can safely catch it. closes odoo/odoo#101549 X-original-commit: fe9e044b9e03ac7fff5d314e6e207921a15c8965 Signed-off-by: Aaron Bohy (aab) --- .../static/src/legacy/js/core/concurrency.js | 2 +- .../tests/core/utils/concurrency_tests.js | 15 ++++++++++++++ .../tests/legacy/core/concurrency_tests.js | 16 +++++++++++++++ .../views/fields/many2one_field_tests.js | 20 ++++++++++++++++--- 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/addons/web/static/src/legacy/js/core/concurrency.js b/addons/web/static/src/legacy/js/core/concurrency.js index 716abafa919..840df367aff 100644 --- a/addons/web/static/src/legacy/js/core/concurrency.js +++ b/addons/web/static/src/legacy/js/core/concurrency.js @@ -202,7 +202,7 @@ return { } return returnedResult; }; - Promise.resolve(result).then(always).guardedCatch(always); + Promise.resolve(result).then(always).catch(always); }); }); return this.lock.then(function () { diff --git a/addons/web/static/tests/core/utils/concurrency_tests.js b/addons/web/static/tests/core/utils/concurrency_tests.js index b8bbc888188..7b110510b34 100644 --- a/addons/web/static/tests/core/utils/concurrency_tests.js +++ b/addons/web/static/tests/core/utils/concurrency_tests.js @@ -120,6 +120,21 @@ QUnit.module("utils", () => { assert.verifySteps(["mutex unlocked (2)", "ok [2]"]); }); + QUnit.test("Mutex: error and getUnlockedDef", async function (assert) { + const mutex = new Mutex(); + const action = async () => { + await Promise.resolve(); + throw new Error("boom"); + }; + mutex.exec(action).catch(() => assert.step("prom rejected")); + await nextTick(); + assert.verifySteps(["prom rejected"]); + + mutex.getUnlockedDef().then(() => assert.step("mutex unlocked")); + await nextTick(); + assert.verifySteps(["mutex unlocked"]); + }); + QUnit.test("KeepLast: basic use", async function (assert) { assert.expect(3); diff --git a/addons/web/static/tests/legacy/core/concurrency_tests.js b/addons/web/static/tests/legacy/core/concurrency_tests.js index 1e195fde67d..e88ae4b6eba 100644 --- a/addons/web/static/tests/legacy/core/concurrency_tests.js +++ b/addons/web/static/tests/legacy/core/concurrency_tests.js @@ -124,6 +124,22 @@ QUnit.module('core', {}, function () { assert.verifySteps(['ok prom2', 'mutex unlocked (2)']); }); + QUnit.test('mutex: error and getUnlockedDef', async function (assert) { + const mutex = new concurrency.Mutex(); + mutex.exec(async () => { + await Promise.resolve(); + throw new Error("boom"); + }).catch(() => assert.step("prom rejected")); + await testUtils.nextTick(); + assert.verifySteps(['prom rejected']); + + mutex.getUnlockedDef().then(function () { + assert.step('mutex unlocked'); + }); + await testUtils.nextMicrotaskTick(); + assert.verifySteps(['mutex unlocked']); + }); + QUnit.test('DropPrevious: basic usecase', async function (assert) { assert.expect(4); diff --git a/addons/web/static/tests/views/fields/many2one_field_tests.js b/addons/web/static/tests/views/fields/many2one_field_tests.js index 13b1bd742d7..75cb96140a3 100644 --- a/addons/web/static/tests/views/fields/many2one_field_tests.js +++ b/addons/web/static/tests/views/fields/many2one_field_tests.js @@ -1,10 +1,13 @@ /** @odoo-module **/ import { browser } from "@web/core/browser/browser"; +import { errorService } from "@web/core/errors/error_service"; +import { RPCError } from "@web/core/network/rpc_service"; import { registry } from "@web/core/registry"; import { session } from "@web/session"; import { Field } from "@web/views/fields/field"; import { Record } from "@web/views/record"; +import { registerCleanup } from "@web/../tests/helpers/cleanup"; import { addRow, click, @@ -2835,7 +2838,17 @@ QUnit.module("Fields", (hooks) => { }); QUnit.test("failing quick create on a many2one", async function (assert) { - assert.expect(4); + assert.expect(5); + + registry.category("services").add("error", errorService); + + // remove the override in qunit.js that swallows unhandledrejection errors + // s.t. we let the error service handle them + const originalOnUnhandledRejection = window.onunhandledrejection; + window.onunhandledrejection = () => {}; + registerCleanup(() => { + window.onunhandledrejection = originalOnUnhandledRejection; + }); serverData.views = { "product,false,form": '
', @@ -2848,7 +2861,7 @@ QUnit.module("Fields", (hooks) => { arch: '
', mockRPC(route, { args, method }) { if (method === "name_create") { - return Promise.reject(); + throw new RPCError("Something went wrong"); } if (method === "create") { assert.deepEqual(args[0], { name: "xyz" }); @@ -2858,7 +2871,8 @@ QUnit.module("Fields", (hooks) => { await editInput(target, ".o_field_widget[name='product_id'] input", "abcd"); await click(target.querySelector(".o_field_widget[name='product_id'] .dropdown-item")); - + await nextTick(); // wait for the error service to ensure that there's no error dialog + assert.containsNone(target, ".o_form_error_dialog"); assert.containsOnce(target, ".modal .o_form_view"); assert.strictEqual( target.querySelector(".modal .o_field_widget[name='name'] input").value,