[IMP] component: propagate error to mount and render

src/component/component.ts

@@ -266,7 +266,7 @@ export class Component<T extends Env, Props extends {}> {
    * It needs to be implemented by a component that is designed to handle the
    * error properly.
    */
-  catchError?(error?: Error):void;
+  catchError?(error?: Error): void;
 
   //--------------------------------------------------------------------------
   // Public
@@ -298,8 +298,12 @@ export class Component<T extends Env, Props extends {}> {
     } else {
       this.__render(fiber);
     }
-    return new Promise(resolve => {
+    return new Promise((resolve, reject) => {
-      scheduler.addFiber(fiber, () => {
+      scheduler.addFiber(fiber, err => {
+        if (err) {
+          reject(err);
+          return;
+        }
         if (!__owl__.isDestroyed) {
           this.__patch(fiber.vnode);
           target.appendChild(this.el!);
@@ -342,8 +346,12 @@ export class Component<T extends Env, Props extends {}> {
     }
     const fiber = new Fiber(null, this, this.props, undefined, undefined, force);
     this.__render(fiber);
-    return new Promise(resolve => {
+    return new Promise((resolve, reject) => {
-      scheduler.addFiber(fiber.root, () => {
+      scheduler.addFiber(fiber.root, err => {
+        if (err) {
+          reject(err);
+          return;
+        }
         if (__owl__.isMounted && fiber === fiber.root) {
           fiber.patchComponents();
         }

src/component/fiber.ts

@@ -53,6 +53,8 @@ export class Fiber {
   sibling: Fiber | null = null;
   parent: Fiber | null = null;
 
+  error?: Error;
+
   constructor(parent: Fiber | null, component: Component<any, any>, props, scope, vars, force) {
     this.force = force;
     this.scope = scope;
@@ -212,14 +214,15 @@ export class Fiber {
       root = component;
       component = component.__owl__.parent!;
     }
-    console.error(error);
     qweb.trigger("error", error);
 
     if (canCatch) {
       setTimeout(() => {
+        console.error(error);
         component.catchError!(error);
       });
     } else {
+      this.root.error = error;
       root.destroy();
     }
   }

src/component/scheduler.ts

@@ -13,7 +13,7 @@ import { Fiber } from "./fiber";
 
 interface Task {
   fiber: Fiber;
-  callback: () => void;
+  callback: (err?: Error) => void;
 }
 
 export class Scheduler {
@@ -45,7 +45,7 @@ export class Scheduler {
         return false;
       }
       if (task.fiber.counter === 0) {
-        task.callback();
+        task.callback(task.fiber.error);
         return false;
       }
       return true;

tests/component/component.test.ts

@@ -1090,12 +1090,15 @@ describe("composition", () => {
       static components = { SomeWidget: Widget };
     }
     const parent = new Parent(env);
-    await parent.mount(fixture);
+    let error;
-
+    try {
-    expect(console.error).toBeCalledTimes(1);
+      await parent.mount(fixture);
-    expect((<any>console.error).mock.calls[0][0].message).toMatch(
+    } catch (e) {
-      'Cannot find the definition of component "SomeMispelledWidget"'
+      error = e;
-    );
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe('Cannot find the definition of component "SomeMispelledWidget"');
+    expect(console.error).toBeCalledTimes(0);
     console.error = consoleError;
   });
 
@@ -3466,12 +3469,15 @@ describe("widget and observable state", () => {
       static components = { Child };
     }
     const parent = new Parent(env);
-    await parent.mount(fixture);
+    let error;
-
+    try {
-    expect(console.error).toBeCalledTimes(1);
+      await parent.mount(fixture);
-    expect((<any>console.error).mock.calls[0][0].message).toMatch(
+    } catch (e) {
-      'Observed state cannot be changed here! (key: "coffee", val: "2")'
+      error = e;
-    );
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe('Observed state cannot be changed here! (key: "coffee", val: "2")');
+    expect(console.error).toBeCalledTimes(0);
     console.error = consoleError;
   });
 });
@@ -4359,22 +4365,27 @@ describe("component error handling (catchError)", () => {
   });
 
   test("no component catching error lead to full app destruction", async () => {
+      expect.assertions(6)
     const handler = jest.fn();
     env.qweb.on("error", null, handler);
     const consoleError = console.error;
     console.error = jest.fn();
-    env.qweb.addTemplates(`
+
-      <templates>
+    class ErrorComponent extends Widget {
-        <div t-name="ErrorComponent">hey<t t-esc="props.flag and state.this.will.crash"/>
+      static template = xml`<div>hey<t t-esc="props.flag and state.this.will.crash"/></div>`;
-        </div>
+    }
-        <div t-name="App">
+
-            <ErrorComponent flag="state.flag"/>
-        </div>
-      </templates>`);
-    class ErrorComponent extends Widget {}
     class App extends Widget {
-      state = useState({ flag: false });
+      static template = xml`<div><ErrorComponent flag="state.flag"/></div>`;
       static components = { ErrorComponent };
+      state = useState({ flag: false });
+      async render() {
+        try {
+          await super.render();
+        } catch (e) {
+          expect(e.message).toBe("Cannot read property 'this' of undefined");
+        }
+      }
     }
     const app = new App(env);
     await app.mount(fixture);
@@ -4385,7 +4396,7 @@ describe("component error handling (catchError)", () => {
     await nextTick();
     expect(fixture.innerHTML).toBe("");
 
-    expect(console.error).toBeCalledTimes(1);
+    expect(console.error).toBeCalledTimes(0);
     console.error = consoleError;
     expect(app.__owl__.isDestroyed).toBe(true);
     expect(handler).toBeCalledTimes(1);
@@ -4586,6 +4597,80 @@ describe("component error handling (catchError)", () => {
     await nextTick();
     expect(fixture.innerHTML).toBe("<div><div>Error handled</div></div>");
   });
+
+  test("a rendering error will reject the mount promise", async () => {
+    const consoleError = console.error;
+    console.error = jest.fn(() => {});
+    // we do not catch error in willPatch anymore
+    class App extends Component<any, any> {
+      static template = xml`<div><t t-esc="this.will.crash"/></div>`;
+    }
+
+    const app = new App(env);
+    let error;
+    try {
+      await app.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe("Cannot read property 'crash' of undefined");
+
+    expect(console.error).toBeCalledTimes(0);
+    console.error = consoleError;
+  });
+
+  test("a rendering error in a sub component will reject the mount promise", async () => {
+    const consoleError = console.error;
+    console.error = jest.fn(() => {});
+    // we do not catch error in willPatch anymore
+    class Child extends Component<any, any> {
+      static template = xml`<div><t t-esc="this.will.crash"/></div>`;
+    }
+    class App extends Component<any, any> {
+      static template = xml`<div><Child/></div>`;
+      static components = { Child };
+    }
+
+    const app = new App(env);
+    let error;
+    try {
+      await app.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe("Cannot read property 'crash' of undefined");
+
+    expect(console.error).toBeCalledTimes(0);
+    console.error = consoleError;
+  });
+
+  test("a rendering error will reject the render promise", async () => {
+    const consoleError = console.error;
+    console.error = jest.fn(() => {});
+    // we do not catch error in willPatch anymore
+    class App extends Component<any, any> {
+      static template = xml`<div><t t-if="flag" t-esc="this.will.crash"/></div>`;
+      flag = false;
+    }
+
+    const app = new App(env);
+    await app.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div></div>");
+    app.flag = true;
+    let error;
+    try {
+      await app.render();
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe("Cannot read property 'crash' of undefined");
+
+    expect(console.error).toBeCalledTimes(0);
+    console.error = consoleError;
+  });
 });
 
 describe("top level sub widgets", () => {