wip

src/component/fiber.ts

@@ -180,6 +180,7 @@ export class Fiber {
    */
   complete() {
     let component = this.component;
+    let fiber: Fiber = this;
     this.isCompleted = true;
     if (!this.target && !component.__owl__.isMounted) {
       return;
@@ -194,9 +195,10 @@ export class Fiber {
     this._walk(doWork);
     const patchLen = patchQueue.length;
 
+    try {
       // call willPatch hook on each fiber of patchQueue
       for (let i = 0; i < patchLen; i++) {
-      const fiber = patchQueue[i];
+        fiber = patchQueue[i];
         if (fiber.shouldPatch) {
           component = fiber.component;
           if (component.__owl__.willPatchCB) {
@@ -208,7 +210,7 @@ export class Fiber {
 
       // call __patch on each fiber of (reversed) patchQueue
       for (let i = patchLen - 1; i >= 0; i--) {
-      const fiber = patchQueue[i];
+        fiber = patchQueue[i];
         component = fiber.component;
         component.__patch(fiber.vnode!);
         if (!fiber.shouldPatch && (!fiber.target || i !== 0)) {
@@ -226,7 +228,7 @@ export class Fiber {
 
       // call patched/mounted hook on each fiber of (reversed) patchQueue
       for (let i = patchLen - 1; i >= 0; i--) {
-      const fiber = patchQueue[i];
+        fiber = patchQueue[i];
         component = fiber.component;
         if (fiber.shouldPatch && !this.target) {
           component.patched();
@@ -237,6 +239,15 @@ export class Fiber {
           component.__callMounted();
         }
       }
+    } catch (e) {
+      // if there is no current fiber on component, we are in the situation where
+      // components were patched to the DOM, but a mounted/patched hook threw an
+      // error.  In that case, we cannot manage the error at a lower level than
+      // the root fiber, since some components may not have been properly mounted
+      // patched yet.
+      const errorFiber = component.__owl__.currentFiber ? fiber : this;
+      errorFiber.handleError(e);
+    }
   }
 
   /**
@@ -274,7 +285,11 @@ export class Fiber {
     qweb.trigger("error", error);
 
     if (canCatch) {
+      // this.root.isCompleted = false
+      this.root.isCompleted = false;
+      // component.__owl__.currentFiber!.root.isCompleted = false;
       component.catchError!(error);
+
     } else {
       // the 3 next lines aim to mark the root fiber as being in error, and
       // to force it to end, without waiting for its children

src/component/scheduler.ts

@@ -62,10 +62,9 @@ export class Scheduler {
       }
       if (task.fiber.counter === 0) {
         if (!task.fiber.error) {
-          try {
           task.fiber.complete();
-          } catch (e) {
-            task.fiber.handleError(e);
+          if (!task.fiber.isCompleted) {
+            return true;
           }
         }
         task.callback();

tests/component/component.test.ts

@@ -4992,41 +4992,35 @@ describe("component error handling (catchError)", () => {
   test.skip("can catch an error in the mounted call", async () => {
     // we do not catch error in mounted anymore
     console.error = jest.fn();
-    env.qweb.addTemplates(`
-      <templates>
-        <div t-name="ErrorBoundary">
-            <t t-if="state.error">Error handled</t>
-            <t t-else="1"><t t-slot="default" /></t>
-        </div>
-        <div t-name="ErrorComponent">Some text</div>
-        <div t-name="App">
-            <ErrorBoundary><ErrorComponent /></ErrorBoundary>
-        </div>
-      </templates>`);
-    class ErrorComponent extends Widget {
+
+    class ErrorComponent extends Component<any,any> {
+      static template = xml`<div>Some text</div>`;
       mounted() {
         throw new Error("NOOOOO");
       }
     }
-    class ErrorBoundary extends Widget {
+    class ErrorBoundary extends Component<any,any> {
+      static template = xml`
+        <div>
+          <t t-if="state.error">Error handled</t>
+          <t t-else="1"><t t-slot="default" /></t>
+        </div>`;
       state = useState({ error: false });
 
       catchError() {
         this.state.error = true;
       }
     }
-    class App extends Widget {
+    class App extends Component<any,any> {
+      static template = xml`<div><ErrorBoundary><ErrorComponent /></ErrorBoundary></div>`;
       static components = { ErrorBoundary, ErrorComponent };
     }
     const app = new App();
     await app.mount(fixture);
-    await nextTick();
-    await nextTick();
-    await nextTick();
     expect(fixture.innerHTML).toBe("<div><div>Error handled</div></div>");
   });
 
-  test.skip("can catch an error in the willPatch call", async () => {
+  test("can catch an error in the willPatch call", async () => {
     // we do not catch error in willPatch anymore
     const consoleError = console.error;
     console.error = jest.fn();
@@ -5054,18 +5048,16 @@ describe("component error handling (catchError)", () => {
             <span><t t-esc="state.message"/></span>
           <ErrorBoundary><ErrorComponent message="state.message" /></ErrorBoundary>
         </div>`;
-      state = useState({ message: "abc" });
+      state = { message: "abc" };
       static components = { ErrorBoundary, ErrorComponent };
     }
     const app = new App();
     await app.mount(fixture);
     expect(fixture.innerHTML).toBe("<div><span>abc</span><div><div>abc</div></div></div>");
     app.state.message = "def";
-    await nextTick();
-    await nextTick();
-    await nextTick();
+    await app.render();
     expect(fixture.innerHTML).toBe("<div><span>def</span><div>Error handled</div></div>");
-    expect(console.error).toHaveBeenCalledTimes(1);
+    expect(console.error).toHaveBeenCalledTimes(0);
     console.error = consoleError;
   });