From 4b3dc07b52e57f2b513d52ea1429690660ca9c57 Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Fri, 9 Jul 2021 11:36:35 +0000
Subject: [PATCH] [FIX] web: better handle error in actions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before this commit, if there was a crash in a view or client
action during an update (i.e. the action is already in the DOM,
but an update triggers a re-rendering), the error was caught by
the action service, it wasn't displayed to the user, and the
action was re-rendered again (which could obviously result in the
same error being thrown again and again).

With this commit, we properly show the error, and we do not try
to re-render the action.

closes odoo/odoo#73507

Signed-off-by: Géry Debongnie (ged) <ged@openerp.com>
---
 .../src/webclient/actions/action_service.js   | 18 +++++++-
 .../webclient/actions/client_action_tests.js  |  1 -
 .../webclient/actions/error_handling_tests.js | 45 ++++++++++++++++++-
 3 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/addons/web/static/src/webclient/actions/action_service.js b/addons/web/static/src/webclient/actions/action_service.js
index 5641569760e..b2048498b49 100644
--- a/addons/web/static/src/webclient/actions/action_service.js
+++ b/addons/web/static/src/webclient/actions/action_service.js
@@ -472,7 +472,23 @@ function makeActionManager(env) {
                     throw error;
                 } else {
                     const lastCt = controllerStack[controllerStack.length - 1];
-                    const info = lastCt ? lastCt.__info__ : {};
+                    let info = {};
+                    if (lastCt) {
+                        if (lastCt.jsId === controller.jsId) {
+                            // the error occurred on the controller which is
+                            // already in the DOM, so simply show the error
+                            Promise.resolve().then(() => {
+                                throw error;
+                            });
+                            return;
+                        } else {
+                            info = lastCt.__info__;
+                            // the error occurred while rendering a new controller,
+                            // so go back to the last non faulty controller
+                            // (the error will be shown anyway as the promise
+                            // has been rejected)
+                        }
+                    }
                     env.bus.trigger("ACTION_MANAGER:UPDATE", info);
                 }
             }
diff --git a/addons/web/static/tests/webclient/actions/client_action_tests.js b/addons/web/static/tests/webclient/actions/client_action_tests.js
index 8eba3d7e52a..464114c0887 100644
--- a/addons/web/static/tests/webclient/actions/client_action_tests.js
+++ b/addons/web/static/tests/webclient/actions/client_action_tests.js
@@ -12,7 +12,6 @@ import { createWebClient, doAction, getActionManagerServerData } from "./../help
 const { Component, tags } = owl;
 
 let serverData;
-const mainComponentRegistry = registry.category("main_components");
 const actionRegistry = registry.category("actions");
 
 QUnit.module("ActionManager", (hooks) => {
diff --git a/addons/web/static/tests/webclient/actions/error_handling_tests.js b/addons/web/static/tests/webclient/actions/error_handling_tests.js
index a156c48ba8a..fdea5e97213 100644
--- a/addons/web/static/tests/webclient/actions/error_handling_tests.js
+++ b/addons/web/static/tests/webclient/actions/error_handling_tests.js
@@ -1,7 +1,11 @@
 /** @odoo-module **/
 
+import { browser } from "@web/core/browser/browser";
 import { registry } from "@web/core/registry";
 import { createWebClient, doAction, getActionManagerServerData } from "./../helpers";
+import { registerCleanup } from "../../helpers/cleanup";
+import { click, patchWithCleanup } from "../../helpers/utils";
+import { errorService } from "@web/core/errors/error_service";
 
 const { Component, tags } = owl;
 
@@ -15,7 +19,7 @@ QUnit.module("ActionManager", (hooks) => {
 
     QUnit.module("Error handling");
 
-    QUnit.test("error in a client action", async function (assert) {
+    QUnit.test("error in a client action (at rendering)", async function (assert) {
         assert.expect(4);
         class Boom extends Component {}
         Boom.template = tags.xml`<div><t t-esc="a.b.c"/></div>`;
@@ -33,4 +37,43 @@ QUnit.module("ActionManager", (hooks) => {
         }
         assert.strictEqual(webClient.el.querySelector(".o_action_manager").innerHTML, contents);
     });
+
+    QUnit.test("error in a client action (after the first rendering)", async function (assert) {
+        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: () => {},
+        });
+
+        registry.category("services").add("error", errorService);
+
+        class Boom extends Component {
+            setup() {
+                this.boom = false;
+            }
+            onClick() {
+                this.boom = true;
+                this.render();
+            }
+        }
+        Boom.template = tags.xml`
+            <div>
+                <t t-if="boom" t-esc="a.b.c"/>
+                <button t-else="" class="my_button" t-on-click="onClick">Click Me</button>
+            </div>`;
+        actionRegistry.add("Boom", Boom);
+
+        const webClient = await createWebClient({ serverData });
+        await doAction(webClient, "Boom");
+        assert.containsOnce(webClient, ".my_button");
+
+        await click(document.querySelector(".my_button"));
+        assert.containsOnce(webClient, ".my_button");
+        assert.containsOnce(webClient, ".o_dialog_error");
+    });
 });