From 0f8c859d5f90511b90845ef3ee81430cc4fb9832 Mon Sep 17 00:00:00 2001
From: "Lucas Perais (lpe)" <lpe@odoo.com>
Date: Mon, 20 Dec 2021 16:27:09 +0100
Subject: [PATCH] [FIX] component: error_handling when an error is rethrown

---
 src/component/error_handling.ts               |  7 +-
 .../__snapshots__/error_handling.test.ts.snap | 75 +++++++++++++++++++
 tests/components/error_handling.test.ts       | 70 +++++++++++++++++
 3 files changed, 148 insertions(+), 4 deletions(-)

diff --git a/src/component/error_handling.ts b/src/component/error_handling.ts
index 9774b2f7..d61a6d8c 100644
--- a/src/component/error_handling.ts
+++ b/src/component/error_handling.ts
@@ -16,10 +16,6 @@ function _handleError(node: ComponentNode | null, error: any, isFirstRound = fal
 
   const errorHandlers = nodeErrorHandlers.get(node);
   if (errorHandlers) {
-    if (isFirstRound && fiber) {
-      fiber.root.counter--;
-    }
-
     let stopped = false;
     // execute in the opposite order
     for (let i = errorHandlers.length - 1; i >= 0; i--) {
@@ -33,6 +29,9 @@ function _handleError(node: ComponentNode | null, error: any, isFirstRound = fal
     }
 
     if (stopped) {
+      if (isFirstRound && fiber) {
+        fiber.root.counter--;
+      }
       return true;
     }
   }
diff --git a/tests/components/__snapshots__/error_handling.test.ts.snap b/tests/components/__snapshots__/error_handling.test.ts.snap
index ddf47998..c1e86958 100644
--- a/tests/components/__snapshots__/error_handling.test.ts.snap
+++ b/tests/components/__snapshots__/error_handling.test.ts.snap
@@ -705,6 +705,81 @@ exports[`can catch errors catchError in catchError 3`] = `
 }"
 `;
 
+exports[`can catch errors catching error, rethrow, render parent  -- a main component loop implementation 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { prepareList, capture, withKey } = helpers;
+  
+  function slot3(ctx, node, key = \\"\\") {
+    let Comp5 = ctx['cp'].Comp;
+    return toggler(Comp5, component(Comp5, {}, key + \`__4\`, node, ctx));
+  }
+  
+  return function template(ctx, node, key = \\"\\") {
+    ctx = Object.create(ctx);
+    const [k_block1, v_block1, l_block1, c_block1] = prepareList(Object.values(ctx['state'].cps));
+    for (let i1 = 0; i1 < l_block1; i1++) {
+      ctx[\`cp\`] = v_block1[i1];
+      let key1 = ctx['cp'].id;
+      const v1 = ctx['cp'];
+      const ctx2 = capture(ctx);
+      c_block1[i1] = withKey(component(\`ErrorHandler\`, {onError: ()=>this.cleanUp(v1.id),slots: {'default': {__render: slot3, __ctx: ctx2}}}, key + \`__6__\${key1}\`, node, ctx), key1);
+    }
+    return list(c_block1);
+  }
+}"
+`;
+
+exports[`can catch errors catching error, rethrow, render parent  -- a main component loop implementation 2`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { callSlot } = helpers;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return callSlot(ctx, node, key, 'default', false, {});
+  }
+}"
+`;
+
+exports[`can catch errors catching error, rethrow, render parent  -- a main component loop implementation 3`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return component(\`ErrorComponent\`, {}, key + \`__1\`, node, ctx);
+  }
+}"
+`;
+
+exports[`can catch errors catching error, rethrow, render parent  -- a main component loop implementation 4`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  let block1 = createBlock(\`<div/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    return block1();
+  }
+}"
+`;
+
+exports[`can catch errors catching error, rethrow, render parent  -- a main component loop implementation 5`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  let block1 = createBlock(\`<div>Sibling</div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    return block1();
+  }
+}"
+`;
+
 exports[`can catch errors error in mounted on a component with a sibling (properly mounted) 1`] = `
 "function anonymous(bdom, helpers
 ) {
diff --git a/tests/components/error_handling.test.ts b/tests/components/error_handling.test.ts
index 62df4ae4..70d3fde0 100644
--- a/tests/components/error_handling.test.ts
+++ b/tests/components/error_handling.test.ts
@@ -13,6 +13,7 @@ import {
   logStep,
   makeTestFixture,
   nextTick,
+  nextMicroTick,
   snapshotEverything,
   useLogLifecycle,
 } from "../helpers";
@@ -963,4 +964,73 @@ describe("can catch errors", () => {
     expect(fixture.innerHTML).toBe("<div>Abstract</div>");
     expect(mockConsoleWarn).toBeCalledTimes(0);
   });
+
+  test("catching error, rethrow, render parent  -- a main component loop implementation", async () => {
+    let parentState: any;
+
+    class ErrorComponent extends Component {
+      static template = xml`<div />`;
+      setup() {
+        throw new Error("My Error");
+      }
+    }
+
+    class Child extends Component {
+      static template = xml`<ErrorComponent />`;
+      static components = { ErrorComponent };
+      setup() {
+        onError((error) => {
+          throw error;
+        });
+      }
+    }
+
+    class Sibling extends Component {
+      static template = xml`<div>Sibling</div>`;
+    }
+
+    class ErrorHandler extends Component {
+      static template = xml`<t t-slot="default" />`;
+      setup() {
+        onError(() => {
+          this.props.onError();
+          Promise.resolve().then(() => {
+            parentState.cps[2] = {
+              id: 2,
+              Comp: Sibling,
+            };
+          });
+        });
+      }
+    }
+
+    class Parent extends Component {
+      static template = xml`
+        <t t-foreach="Object.values(state.cps)" t-as="cp" t-key="cp.id">
+          <ErrorHandler onError="() => this.cleanUp(cp.id)">
+              <t t-component="cp.Comp" />
+            </ErrorHandler>
+        </t>`;
+
+      static components = { ErrorHandler };
+      state: any = useState({
+        cps: {},
+      });
+
+      setup() {
+        parentState = this.state;
+      }
+
+      cleanUp(id: number) {
+        delete this.state.cps[id];
+      }
+    }
+
+    await mount(Parent, fixture);
+    parentState.cps[1] = { id: 1, Comp: Child };
+    await nextMicroTick();
+    expect(fixture.innerHTML).toBe("");
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div>Sibling</div>");
+  });
 });