[FIX] web: action_service: better error handling

on a legacy form view, do an action in target new
The view in target new should, during its onchange, trigger a ValidationError

Before this commit, the dialog_container was stuck in an error loop.
This was because the received error was never an instance of Error, rather it was
a legacy error event. So it was never transmitted upstream were the ErrorHandler could remove
the dialog in error from is dialog array.

After this commit, the ValidationError mesage is displayed.

This commit additionally fixes another issue:
An action in target main that failed to render erased the whole DOM. This is not what we want,
at least for now. Imagine a list view, open a record, this record fails to load. We still want to be
on the list view, not in a blank state.

addons/web/static/src/webclient/actions/action_container.js

@@ -20,21 +20,6 @@ export class ActionContainer extends Component {
         this.env.bus.off("ACTION_MANAGER:UPDATE", this);
         super.destroy();
     }
-
-    catchError() {
-        this.info = {};
-        this.render();
-        // we trigger here the 'MENUS-APP-CHANGED' event to make sure the navbar
-        // is properly displayed/updated.
-        this.env.bus.trigger("MENUS:APP-CHANGED");
-
-        // we do not rethrow the error here, because the controller component
-        // already caught the error, and rejected the doAction promise. That
-        // doAction promise will then trigger an unhandledRejection error, which
-        // will be displayed by the error service (unless the promise is handled
-        // by some caller code, but then in that case, it is its own responsibility
-        // to handle the error properly.
-    }
 }
 ActionContainer.components = { ActionDialog };
 ActionContainer.template = tags.xml`

addons/web/static/src/webclient/actions/action_service.js

@@ -458,11 +458,15 @@ function makeActionManager(env) {
                 }
             }
             catchError(error) {
-                // The above component should truely handle the error
                 reject(error);
-                // Re-throw in case it is a programming error
-                if (error && error.name) {
+                cleanDomFromBootstrap();
+                if (action.target === "new") {
+                    // get the dialog service to close the dialog.
                     throw error;
+                } else {
+                    const lastCt = controllerStack[controllerStack.length - 1];
+                    const info = lastCt ? lastCt.__info__ : {};
+                    env.bus.trigger("ACTION_MANAGER:UPDATE", info);
                 }
             }
             mounted() {
@@ -582,11 +586,13 @@ function makeActionManager(env) {
         const nextStack = controllerStack.slice(0, index).concat(controllerArray);
         controller.props.breadcrumbs = _getBreadcrumbs(nextStack.slice(0, nextStack.length - 1));
         const closingProm = _executeCloseAction();
-        env.bus.trigger("ACTION_MANAGER:UPDATE", {
+
+        controller.__info__ = {
             id: ++id,
             Component: ControllerComponent,
             componentProps: controller.props,
-        });
+        };
+        env.bus.trigger("ACTION_MANAGER:UPDATE", controller.__info__);
         return Promise.all([currentActionProm, closingProm]).then((r) => r[0]);
     }
 

addons/web/static/tests/webclient/actions/error_handling_tests.js

@@ -24,12 +24,13 @@ QUnit.module("ActionManager", (hooks) => {
         const webClient = await createWebClient({ serverData });
         assert.strictEqual(webClient.el.querySelector(".o_action_manager").innerHTML, "");
         await doAction(webClient, "1");
-        assert.ok(webClient.el.querySelector(".o_action_manager").innerHTML !== "");
+        const contents = webClient.el.querySelector(".o_action_manager").innerHTML;
+        assert.ok(contents !== "");
         try {
             await doAction(webClient, "Boom");
         } catch (e) {
             assert.ok(e instanceof TypeError);
         }
-        assert.strictEqual(webClient.el.querySelector(".o_action_manager").innerHTML, "");
+        assert.strictEqual(webClient.el.querySelector(".o_action_manager").innerHTML, contents);
     });
 });

addons/web/static/tests/webclient/actions/load_state_tests.js

@@ -8,8 +8,7 @@ import core from "web.core";
 import AbstractAction from "web.AbstractAction";
 import { registerCleanup } from "../../helpers/cleanup";
 import { makeTestEnv } from "../../helpers/mock_env";
-import { makeFakeRouterService } from "../../helpers/mock_services";
-import { getFixture, legacyExtraNextTick, patchWithCleanup } from "../../helpers/utils";
+import { click, getFixture, legacyExtraNextTick, patchWithCleanup } from "../../helpers/utils";
 import {
     createWebClient,
     doAction,
@@ -17,13 +16,13 @@ import {
     loadState,
     setupWebClientServiceRegistry,
 } from "./../helpers";
+import { errorService } from "@web/core/errors/error_service";
 
 const { Component, mount, tags } = owl;
 
 let serverData;
 
 const actionRegistry = registry.category("actions");
-const serviceRegistry = registry.category("services");
 
 QUnit.module("ActionManager", (hooks) => {
     hooks.beforeEach(() => {
@@ -927,4 +926,44 @@ QUnit.module("ActionManager", (hooks) => {
             );
         }
     );
+
+    QUnit.test("initial action crashes", async (assert) => {
+        assert.expect(7);
+
+        const handler = (ev) => {
+            // need to preventDefault to remove error from console (so python test pass)
+            ev.preventDefault();
+        };
+        window.addEventListener("unhandledrejection", handler);
+        registerCleanup(() => window.removeEventListener("unhandledrejection", handler));
+
+        patchWithCleanup(QUnit, {
+            onUnhandledRejection: () => {},
+        });
+
+        browser.location.hash = "#action=__test__client__action__&menu_id=1";
+        const ClientAction = registry.category("actions").get("__test__client__action__");
+        class Override extends ClientAction {
+            setup() {
+                super.setup();
+                assert.step("clientAction setup");
+                throw new Error("my error");
+            }
+        }
+        registry.category("actions").add("__test__client__action__", Override, { force: true });
+
+        registry.category("services").add("error", errorService);
+
+        const webClient = await createWebClient({ serverData });
+        assert.verifySteps(["clientAction setup"]);
+        await click(webClient.el, "nav .o_navbar_apps_menu .o_dropdown_toggler ");
+        assert.containsN(webClient, ".o_dropdown_item.o_app", 3);
+        assert.containsNone(webClient, ".o_menu_brand");
+        assert.containsOnce(webClient, ".o_dialog_error");
+        assert.strictEqual(webClient.el.querySelector(".o_action_manager").innerHTML, "");
+        assert.deepEqual(webClient.env.services.router.current.hash, {
+            action: "__test__client__action__",
+            menu_id: 1,
+        });
+    });
 });

addons/web/static/tests/webclient/actions/window_action_tests.js

@@ -10,8 +10,18 @@ import FormView from "web.FormView";
 import ListController from "web.ListController";
 import testUtils from "web.test_utils";
 import legacyViewRegistry from "web.view_registry";
-import { click, legacyExtraNextTick, nextTick, patchWithCleanup } from "../../helpers/utils";
+import {
+    click,
+    legacyExtraNextTick,
+    makeDeferred,
+    nextTick,
+    patchWithCleanup,
+} from "../../helpers/utils";
 import { createWebClient, doAction, getActionManagerServerData, loadState } from "./../helpers";
+import { errorService } from "../../../src/core/errors/error_service";
+import { RPCError } from "@web/core/network/rpc_service";
+import { registerCleanup } from "../../helpers/cleanup";
+import { WarningDialog } from "@web/core/errors/error_dialogs";
 
 let serverData;
 const serviceRegistry = registry.category("services");
@@ -2294,4 +2304,73 @@ QUnit.module("ActionManager", (hooks) => {
             });
         }
     );
+
+    QUnit.test("window action in target new fails (onchange)", async (assert) => {
+        assert.expect(3);
+
+        /*
+         * By-pass QUnit's and test's error handling because the error service needs to be active
+         */
+        const handler = (ev) => {
+            // need to preventDefault to remove error from console (so python test pass)
+            ev.preventDefault();
+        };
+        window.addEventListener("unhandledrejection", handler);
+        registerCleanup(() => window.removeEventListener("unhandledrejection", handler));
+
+        patchWithCleanup(QUnit, {
+            onUnhandledRejection: () => {},
+        });
+
+        const originOnunhandledrejection = window.onunhandledrejection;
+        window.onunhandledrejection = () => {};
+        registerCleanup(() => {
+            window.onunhandledrejection = originOnunhandledrejection;
+        });
+        /*
+         * End By pass error handling
+         */
+
+        const warningOpened = makeDeferred();
+        class WarningDialogWait extends WarningDialog {
+            setup() {
+                super.setup();
+                owl.hooks.onMounted(() => warningOpened.resolve());
+            }
+        }
+
+        serviceRegistry.add("error", errorService);
+        registry
+            .category("error_dialogs")
+            .add("odoo.exceptions.ValidationError", WarningDialogWait);
+
+        const mockRPC = (route, args) => {
+            if (args.method === "onchange" && args.model === "partner") {
+                const error = new RPCError();
+                error.exceptionName = "odoo.exceptions.ValidationError";
+                error.code = 200;
+                return Promise.reject(error);
+            }
+        };
+
+        serverData.views["partner,666,form"] = /*xml*/ `
+            <form>
+                <header>
+                    <button name="5" type="action"/>
+                </header>
+                <field name="display_name"/>
+            </form>`;
+
+        const webClient = await createWebClient({ serverData, mockRPC });
+        await doAction(webClient, 24);
+        await click(webClient.el, ".o_form_view button");
+
+        await warningOpened;
+        assert.containsOnce(webClient, ".modal");
+        assert.containsOnce(webClient, ".modal .o_dialog_warning");
+        assert.strictEqual(
+            webClient.el.querySelector(".modal .modal-title").textContent,
+            "Validation Error"
+        );
+    });
 });