From 65d2fa48b752a044d4ab0793df70e8b19f622263 Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Mon, 2 May 2022 10:29:57 +0000
Subject: [PATCH] [FIX] web: action service: no crash if can't stringify action

When a doAction is done, the action service stringifies the given
action and writes it in the session storage, s.t. it can be
restored on F5 even if it's a dynamic action (i.e. not in DB).

However, the stringify operation may crash (e.g. if there is a
cycle in the action description). As we do not control what is
given to doAction, it can happen, and we have to properly handle
it.

This commit simply catches the error, and there's nothing more to
do in this case.

Part-of: odoo/odoo#90271
---
 .../src/webclient/actions/action_service.js   | 11 +++-
 .../webclient/actions/window_action_tests.js  | 52 ++++++++++++-------
 2 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/addons/web/static/src/webclient/actions/action_service.js b/addons/web/static/src/webclient/actions/action_service.js
index 18807d89fa6..459fb0b69d1 100644
--- a/addons/web/static/src/webclient/actions/action_service.js
+++ b/addons/web/static/src/webclient/actions/action_service.js
@@ -185,7 +185,11 @@ function makeActionManager(env) {
      * with a unique jsId.
      */
     function _preprocessAction(action, context = {}) {
-        action._originalAction = JSON.stringify(action);
+        try {
+            action._originalAction = JSON.stringify(action);
+        } catch (_e) {
+            // do nothing, the action might simply not be serializable
+        }
         action.context = makeContext([context, action.context], env.services.user.context);
         if (action.domain) {
             const domain = action.domain || [];
@@ -669,7 +673,10 @@ function makeActionManager(env) {
                     controllerStack = nextStack; // the controller is mounted, commit the new stack
                     pushState(controller);
                     this.titleService.setParts({ action: controller.displayName });
-                    browser.sessionStorage.setItem("current_action", action._originalAction);
+                    browser.sessionStorage.setItem(
+                        "current_action",
+                        action._originalAction || "{}"
+                    );
                 }
                 resolve();
                 env.bus.trigger("ACTION_MANAGER:UI-UPDATED", _getActionMode(action));
diff --git a/addons/web/static/tests/webclient/actions/window_action_tests.js b/addons/web/static/tests/webclient/actions/window_action_tests.js
index a786baa9890..e07427eb5e8 100644
--- a/addons/web/static/tests/webclient/actions/window_action_tests.js
+++ b/addons/web/static/tests/webclient/actions/window_action_tests.js
@@ -640,7 +640,7 @@ QUnit.module("ActionManager", (hooks) => {
         await testUtils.dom.click($(target).find(".o_form_view button:contains(Execute action)"));
         await legacyExtraNextTick();
         assert.containsN(target, ".o_control_panel .breadcrumb li", 3);
-        var $previousBreadcrumb = $(target).find(".o_control_panel .breadcrumb li.active").prev();
+        let $previousBreadcrumb = $(target).find(".o_control_panel .breadcrumb li.active").prev();
         assert.strictEqual(
             $previousBreadcrumb.attr("accesskey"),
             "b",
@@ -649,7 +649,7 @@ QUnit.module("ActionManager", (hooks) => {
         await testUtils.dom.click($previousBreadcrumb);
         await legacyExtraNextTick();
         assert.containsN(target, ".o_control_panel .breadcrumb li", 2);
-        var $previousBreadcrumb = $(target).find(".o_control_panel .breadcrumb li.active").prev();
+        $previousBreadcrumb = $(target).find(".o_control_panel .breadcrumb li.active").prev();
         assert.strictEqual(
             $previousBreadcrumb.attr("accesskey"),
             "b",
@@ -1605,23 +1605,37 @@ QUnit.module("ActionManager", (hooks) => {
         }
     );
 
-    QUnit.test("current act_window action is stored in session_storage", async function (assert) {
-        assert.expect(1);
-        const expectedAction = serverData.actions[3];
-        patchWithCleanup(browser, {
-            sessionStorage: Object.assign(Object.create(sessionStorage), {
-                setItem(k, value) {
-                    assert.deepEqual(
-                        JSON.parse(value),
-                        expectedAction,
-                        "should store the executed action in the sessionStorage"
-                    );
-                },
-            }),
-        });
-        const webClient = await createWebClient({ serverData });
-        await doAction(webClient, 3);
-    });
+    QUnit.test(
+        "current act_window action is stored in session_storage if possible",
+        async function (assert) {
+            let expectedAction;
+            patchWithCleanup(browser, {
+                sessionStorage: Object.assign(Object.create(sessionStorage), {
+                    setItem(k, value) {
+                        assert.deepEqual(JSON.parse(value), expectedAction);
+                    },
+                }),
+            });
+            const webClient = await createWebClient({ serverData });
+
+            // execute an action that can be stringified -> should be stored
+            expectedAction = serverData.actions[3];
+            await doAction(webClient, 3);
+            assert.containsOnce(target, ".o_list_view");
+
+            // execute an action that can't be stringified -> should not crash
+            expectedAction = {};
+            const x = {};
+            x.y = x;
+            await doAction(webClient, {
+                type: "ir.actions.act_window",
+                res_model: "partner",
+                views: [[false, "kanban"]],
+                flags: { x },
+            });
+            assert.containsOnce(target, ".o_kanban_view");
+        }
+    );
 
     QUnit.test("destroy action with lazy loaded controller", async function (assert) {
         assert.expect(6);