[FIX] move error handling out of fiber, fix complicated mounted issues

src/app/app.ts

@@ -1,3 +1,4 @@
+import { onError, onMounted } from "../component/lifecycle_hooks";
 import { Component } from "../component/component";
 import { ComponentNode } from "../component/component_node";
 import { MountOptions } from "../component/fibers";
@@ -64,8 +65,16 @@ export class App<T extends typeof Component = any> extends TemplateSet {
       throw new Error("Cannot mount a component on a detached dom node");
     }
     const node = new ComponentNode(this.Root, this.props, this);
+    const promise: any = new Promise((resolve, reject) => {
+      onMounted(() => resolve(node.component));
+      onError((e) => {
+        reject(e);
+        throw e;
+      });
+    });
     this.root = node;
-    return node.mountComponent(target, options);
+    node.mountComponent(target, options);
+    return promise;
   }
 
   destroy() {

src/component/component_node.ts

@@ -112,11 +112,10 @@ export class ComponentNode<T extends typeof Component = typeof Component>
     this.component.setup();
   }
 
-  mountComponent(target: any, options?: MountOptions): Promise<InstanceType<T>> {
+  mountComponent(target: any, options?: MountOptions) {
     const fiber = new MountFiber(this, target, options);
     this.app.scheduler.addFiber(fiber);
     this.initiateRender(fiber);
-    return fiber.promise.then(() => this.component);
   }
 
   async initiateRender(fiber: Fiber | MountFiber) {
@@ -227,6 +226,9 @@ export class ComponentNode<T extends typeof Component = typeof Component>
    * a mounted hook failed and was handled.
    */
   updateDom() {
+    if (!this.fiber) {
+      return;
+    }
     if (this.bdom === this.fiber!.bdom) {
       // If the error was handled by some child component, we need to find it to
       // apply its change

src/component/error_handling.ts

@@ -20,23 +20,22 @@ function _handleError(node: ComponentNode | null, error: any, isFirstRound = fal
       fiber.root.counter--;
     }
 
-    let propagate = true;
+    let stopped = false;
     for (const h of errorHandlers) {
       try {
         h(error);
-        propagate = false;
+        stopped = true;
+        break;
       } catch (e) {
         error = e;
       }
     }
 
-    if (propagate) {
-      return _handleError(node.parent, error);
+    if (stopped) {
+      return true;
     }
-    return true;
-  } else {
-    return _handleError(node.parent, error);
   }
+  return _handleError(node.parent, error);
 }
 
 type ErrorParams = { error: any } & ({ node: ComponentNode } | { fiber: Fiber });

src/component/fibers.ts

@@ -151,18 +151,11 @@ export interface MountOptions {
 export class MountFiber extends RootFiber {
   target: HTMLElement;
   position: Position;
-  resolve: any;
-  promise: Promise<any>;
-  reject: any;
 
   constructor(node: ComponentNode, target: HTMLElement, options: MountOptions = {}) {
     super(node, null);
     this.target = target;
     this.position = options.position || "last-child";
-    this.promise = new Promise((resolve, reject) => {
-      this.resolve = resolve;
-      this.reject = reject;
-    });
   }
   complete() {
     let current: Fiber | undefined = this;
@@ -195,10 +188,7 @@ export class MountFiber extends RootFiber {
       }
       node.fiber = null;
     } catch (e) {
-      if (!handleError({ fiber: current as Fiber, error: e })) {
-        this.reject(e);
-      }
+      handleError({ fiber: current as Fiber, error: e });
     }
-    this.resolve();
   }
 }

src/component/scheduler.ts

@@ -1,5 +1,5 @@
-import { Fiber, MountFiber, RootFiber } from "./fibers";
 import { fibersInError } from "./error_handling";
+import { Fiber, RootFiber } from "./fibers";
 import { STATUS } from "./status";
 
 // -----------------------------------------------------------------------------
@@ -44,9 +44,6 @@ export class Scheduler {
       const hasError = fibersInError.has(fiber);
       if (hasError && fiber.counter !== 0) {
         this.tasks.delete(fiber);
-        if (fiber instanceof MountFiber) {
-          fiber.reject(fibersInError.get(fiber));
-        }
         return;
       }
       if (fiber.node.status === STATUS.DESTROYED) {

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

@@ -739,6 +739,72 @@ exports[`can catch errors catchError in catchError 3`] = `
 }"
 `;
 
+exports[`can catch errors error in mounted on a component with a sibling (properly mounted) 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber, safeOutput } = helpers;
+  
+  let block1 = createBlock(\`<div>Some text</div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    return block1();
+  }
+}"
+`;
+
+exports[`can catch errors error in mounted on a component with a sibling (properly mounted) 2`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber, safeOutput } = helpers;
+  
+  let block1 = createBlock(\`<div><block-child-0/><block-child-1/><block-child-1/></div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2,b3;
+    if (ctx['state'].error) {
+      b2 = text(\`Error handled\`);
+    } else {
+      b3 = callSlot(ctx, node, key, 'default', false, {});
+    }
+    return block1([], [b2, b3]);
+  }
+}"
+`;
+
+exports[`can catch errors error in mounted on a component with a sibling (properly mounted) 3`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber, safeOutput } = helpers;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`OK\`);
+  }
+}"
+`;
+
+exports[`can catch errors error in mounted on a component with a sibling (properly mounted) 4`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber, safeOutput } = helpers;
+  
+  let block1 = createBlock(\`<div><block-child-0/><block-child-1/></div>\`);
+  
+  function slot2(ctx, node, key = \\"\\") {
+    return component(\`ErrorComponent\`, {}, key + \`__3\`, node, ctx);
+  }
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2 = component(\`OK\`, {}, key + \`__1\`, node, ctx);
+    let b4 = component(\`ErrorBoundary\`, {slots: {'default': {__render: slot2, __ctx: ctx}}}, key + \`__4\`, node, ctx);
+    return block1([], [b2, b4]);
+  }
+}"
+`;
+
 exports[`errors and promises a rendering error in a sub component will reject the mount promise 1`] = `
 "function anonymous(bdom, helpers
 ) {

tests/components/error_handling.test.ts

@@ -585,7 +585,6 @@ describe("can catch errors", () => {
       }
     }
     await mount(Root, fixture);
-    await nextTick();
     expect(fixture.innerHTML).toBe("<div><div>Error handled</div></div>");
     expect([
       "Root:setup",
@@ -634,7 +633,6 @@ describe("can catch errors", () => {
       }
     }
     await mount(Root, fixture);
-    await nextTick();
     expect(fixture.innerHTML).toBe("<div>Error handled</div>");
     expect([
       "Root:setup",
@@ -694,7 +692,6 @@ describe("can catch errors", () => {
       }
     }
     await mount(A, fixture);
-    await nextTick();
     expect(fixture.innerHTML).toBe("<div><div>Error handled</div></div>");
     expect([
       "A:setup",
@@ -723,6 +720,75 @@ describe("can catch errors", () => {
     ]).toBeLogged();
   });
 
+  test("error in mounted on a component with a sibling (properly mounted)", async () => {
+    class ErrorComponent extends Component {
+      static template = xml`<div>Some text</div>`;
+      setup() {
+        useLogLifecycle();
+        onMounted(() => {
+          logStep("boom");
+          throw new Error("NOOOOO");
+        });
+      }
+    }
+    class ErrorBoundary extends Component {
+      static template = xml`<div>
+       <t t-if="state.error">Error handled</t>
+       <t t-else=""><t t-slot="default" /></t>
+      </div>`;
+      state = useState({ error: false });
+
+      setup() {
+        useLogLifecycle();
+        onError(() => (this.state.error = true));
+      }
+    }
+    class OK extends Component {
+      static template = xml`OK`;
+      setup() {
+        useLogLifecycle();
+      }
+    }
+
+    class Root extends Component {
+      static template = xml`<div>
+        <OK/>
+        <ErrorBoundary><ErrorComponent /></ErrorBoundary>
+      </div>`;
+      static components = { ErrorBoundary, ErrorComponent, OK };
+      setup() {
+        useLogLifecycle();
+      }
+    }
+    await mount(Root, fixture);
+    expect(fixture.innerHTML).toBe("<div>OK<div>Error handled</div></div>");
+    expect([
+      "Root:setup",
+      "Root:willStart",
+      "Root:willRender",
+      "OK:setup",
+      "OK:willStart",
+      "ErrorBoundary:setup",
+      "ErrorBoundary:willStart",
+      "Root:rendered",
+      "OK:willRender",
+      "OK:rendered",
+      "ErrorBoundary:willRender",
+      "ErrorComponent:setup",
+      "ErrorComponent:willStart",
+      "ErrorBoundary:rendered",
+      "ErrorComponent:willRender",
+      "ErrorComponent:rendered",
+      "ErrorComponent:mounted",
+      "boom",
+      "ErrorBoundary:willRender",
+      "ErrorBoundary:rendered",
+      "ErrorBoundary:mounted",
+      "OK:mounted",
+      "Root:mounted",
+    ]).toBeLogged();
+  });
+
   test("can catch an error in the willPatch call", async () => {
     class ErrorComponent extends Component {
       static template = xml`<div><t t-esc="props.message"/></div>`;