Master error mounted ged

src/runtime/fibers.ts

@@ -30,6 +30,13 @@ export function makeRootFiber(node: ComponentNode): Fiber {
       fibersInError.delete(current);
       fibersInError.delete(root);
       current.appliedToDom = false;
+      if (current instanceof RootFiber) {
+        // it is possible that this fiber is a fiber that crashed while being
+        // mounted, so the mounted list is possibly corrupted. We restore it to
+        // its normal initial state (which is empty list or a list with a mount
+        // fiber.
+        current.mounted = current instanceof MountFiber ? [current] : [];
+      }
     }
     return current;
   }
@@ -152,6 +159,7 @@ export class RootFiber extends Fiber {
     const node = this.node;
     this.locked = true;
     let current: Fiber | undefined = undefined;
+    let mountedFibers = this.mounted;
     try {
       // Step 1: calling all willPatch lifecycle hooks
       for (current of this.willPatch) {
@@ -173,7 +181,6 @@ export class RootFiber extends Fiber {
       this.locked = false;
 
       // Step 4: calling all mounted lifecycle hooks
-      let mountedFibers = this.mounted;
       while ((current = mountedFibers.pop())) {
         current = current;
         if (current.appliedToDom) {
@@ -194,6 +201,15 @@ export class RootFiber extends Fiber {
         }
       }
     } catch (e) {
+      // if mountedFibers is not empty, this means that a crash occured while
+      // calling the mounted hooks of some component. So, there may still be
+      // some component that have been mounted, but for which the mounted hooks
+      // have not been called. Here, we remove the willUnmount hooks for these
+      // specific component to prevent a worse situation (willUnmount being
+      // called even though mounted has not been called)
+      for (let fiber of mountedFibers) {
+        fiber.node.willUnmount = [];
+      }
       this.locked = false;
       node.app.handleError({ fiber: current || this, error: e });
     }

tests/components/__snapshots__/error_handling.test.ts.snap

@@ -1191,6 +1191,135 @@ exports[`can catch errors error in mounted on a component with a sibling (proper
 }"
 `;
 
+exports[`can catch errors error in onMounted, graceful recovery 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(null, false, false, false, []);
+
+  return function template(ctx, node, key = \\"\\") {
+    const Comp1 = ctx['component'];
+    return toggler(Comp1, comp1({}, (Comp1).name + key + \`__1\`, node, this, Comp1));
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Child\`, true, false, false, []);
+  const comp2 = app.createComponent(\`Boom\`, true, false, false, []);
+
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(\`parent\`);
+    const b3 = comp1({}, key + \`__1\`, node, this, null);
+    const b4 = comp2({}, key + \`__2\`, node, this, null);
+    return multi([b2, b3, b4]);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery 3`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`abc\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery 4`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`boom\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery 5`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`def\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(null, false, false, false, []);
+
+  return function template(ctx, node, key = \\"\\") {
+    let b2, b3;
+    b2 = text(\`R\`);
+    if (ctx['state'].gogogo) {
+      const Comp1 = ctx['component'];
+      b3 = toggler(Comp1, comp1({}, (Comp1).name + key + \`__1\`, node, this, Comp1));
+    }
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 3`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Child\`, true, false, false, []);
+  const comp2 = app.createComponent(\`Boom\`, true, false, false, []);
+
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(\`parent\`);
+    const b3 = comp1({}, key + \`__1\`, node, this, null);
+    const b4 = comp2({}, key + \`__2\`, node, this, null);
+    return multi([b2, b3, b4]);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 4`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`abc\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 5`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`boom\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in onMounted, graceful recovery, variation 6`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`def\`);
+  }
+}"
+`;
+
 exports[`can catch errors onError in class inheritance is called if rethrown 1`] = `
 "function anonymous(app, bdom, helpers
 ) {

tests/components/error_handling.test.ts

@@ -1678,4 +1678,198 @@ describe("can catch errors", () => {
     `);
     expect(fixture.innerHTML).toBe("2");
   });
+
+  test("error in onMounted, graceful recovery", async () => {
+    class Child extends Component {
+      static template = xml`abc`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class OtherChild extends Component {
+      static template = xml`def`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Boom extends Component {
+      static template = xml`boom`;
+      setup() {
+        useLogLifecycle();
+        onMounted(() => {
+          throw new Error("boom");
+        });
+      }
+    }
+
+    class Parent extends Component {
+      static template = xml`parent<Child/><Boom/>`;
+      static components = { Child, Boom };
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Root extends Component {
+      static template = xml`<t t-component="component"/>`;
+
+      component: any = Parent;
+      setup() {
+        useLogLifecycle();
+        onError(() => {
+          logStep("error");
+          this.component = OtherChild;
+          this.render();
+        });
+      }
+    }
+
+    await mount(Root, fixture);
+    expect(fixture.innerHTML).toBe("def");
+
+    expect(steps.splice(0)).toMatchInlineSnapshot(`
+      Array [
+        "Root:setup",
+        "Root:willStart",
+        "Root:willRender",
+        "Parent:setup",
+        "Parent:willStart",
+        "Root:rendered",
+        "Parent:willRender",
+        "Child:setup",
+        "Child:willStart",
+        "Boom:setup",
+        "Boom:willStart",
+        "Parent:rendered",
+        "Child:willRender",
+        "Child:rendered",
+        "Boom:willRender",
+        "Boom:rendered",
+        "Boom:mounted",
+        "error",
+        "Root:willRender",
+        "OtherChild:setup",
+        "OtherChild:willStart",
+        "Root:rendered",
+        "OtherChild:willRender",
+        "OtherChild:rendered",
+        "OtherChild:mounted",
+        "Root:mounted",
+      ]
+    `);
+  });
+
+  test("error in onMounted, graceful recovery, variation", async () => {
+    class Child extends Component {
+      static template = xml`abc`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class OtherChild extends Component {
+      static template = xml`def`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Boom extends Component {
+      static template = xml`boom`;
+      setup() {
+        useLogLifecycle();
+        onMounted(() => {
+          throw new Error("boom");
+        });
+      }
+    }
+
+    class Parent extends Component {
+      static template = xml`parent<Child/><Boom/>`;
+      static components = { Child, Boom };
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Root extends Component {
+      static template = xml`R<t t-if="state.gogogo" t-component="component"/>`;
+
+      component: any = Parent;
+      state = useState({ gogogo: false });
+
+      setup() {
+        useLogLifecycle();
+        onError(() => {
+          logStep("error");
+          this.component = OtherChild;
+          this.render();
+        });
+      }
+    }
+
+    const root = await mount(Root, fixture);
+    expect(fixture.innerHTML).toBe("R");
+
+    // standard mounting process
+    expect(steps.splice(0)).toMatchInlineSnapshot(`
+      Array [
+        "Root:setup",
+        "Root:willStart",
+        "Root:willRender",
+        "Root:rendered",
+        "Root:mounted",
+      ]
+    `);
+
+    root.state.gogogo = true;
+    await nextTick();
+
+    expect(fixture.innerHTML).toBe("Rparentabcboom");
+    // rerender, root creates sub components, it crashes, tries to recover
+    expect(steps.splice(0)).toMatchInlineSnapshot(`
+      Array [
+        "Root:willRender",
+        "Parent:setup",
+        "Parent:willStart",
+        "Root:rendered",
+        "Parent:willRender",
+        "Child:setup",
+        "Child:willStart",
+        "Boom:setup",
+        "Boom:willStart",
+        "Parent:rendered",
+        "Child:willRender",
+        "Child:rendered",
+        "Boom:willRender",
+        "Boom:rendered",
+        "Root:willPatch",
+        "Boom:mounted",
+        "error",
+        "Root:willRender",
+        "OtherChild:setup",
+        "OtherChild:willStart",
+        "Root:rendered",
+      ]
+    `);
+
+    await nextTick();
+    expect(fixture.innerHTML).toBe("Rdef");
+
+    expect(steps.splice(0)).toMatchInlineSnapshot(`
+      Array [
+        "OtherChild:willRender",
+        "OtherChild:rendered",
+        "Root:willPatch",
+        "Child:willDestroy",
+        "Boom:willUnmount",
+        "Boom:willDestroy",
+        "Parent:willDestroy",
+        "OtherChild:mounted",
+        "Root:patched",
+      ]
+    `);
+  });
 });