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,48 +195,58 @@ export class Fiber {
     this._walk(doWork);
     const patchLen = patchQueue.length;
 
-    // call willPatch hook on each fiber of patchQueue
-    for (let i = 0; i < patchLen; i++) {
-      const fiber = patchQueue[i];
-      if (fiber.shouldPatch) {
+    try {
+      // call willPatch hook on each fiber of patchQueue
+      for (let i = 0; i < patchLen; i++) {
+        fiber = patchQueue[i];
+        if (fiber.shouldPatch) {
+          component = fiber.component;
+          if (component.__owl__.willPatchCB) {
+            component.__owl__.willPatchCB();
+          }
+          component.willPatch();
+        }
+      }
+
+      // call __patch on each fiber of (reversed) patchQueue
+      for (let i = patchLen - 1; i >= 0; i--) {
+        fiber = patchQueue[i];
         component = fiber.component;
-        if (component.__owl__.willPatchCB) {
-          component.__owl__.willPatchCB();
+        component.__patch(fiber.vnode!);
+        if (!fiber.shouldPatch && (!fiber.target || i !== 0)) {
+          component.__owl__.pvnode!.elm = component.__owl__.vnode!.elm;
         }
-        component.willPatch();
+        component.__owl__.currentFiber = null;
       }
-    }
 
-    // call __patch on each fiber of (reversed) patchQueue
-    for (let i = patchLen - 1; i >= 0; i--) {
-      const fiber = patchQueue[i];
-      component = fiber.component;
-      component.__patch(fiber.vnode!);
-      if (!fiber.shouldPatch && (!fiber.target || i !== 0)) {
-        component.__owl__.pvnode!.elm = component.__owl__.vnode!.elm;
+      // insert into the DOM (mount case)
+      let inDOM = false;
+      if (this.target) {
+        this.target.appendChild(this.component.el!);
+        inDOM = document.body.contains(this.target);
       }
-      component.__owl__.currentFiber = null;
-    }
 
-    // insert into the DOM (mount case)
-    let inDOM = false;
-    if (this.target) {
-      this.target.appendChild(this.component.el!);
-      inDOM = document.body.contains(this.target);
-    }
-
-    // call patched/mounted hook on each fiber of (reversed) patchQueue
-    for (let i = patchLen - 1; i >= 0; i--) {
-      const fiber = patchQueue[i];
-      component = fiber.component;
-      if (fiber.shouldPatch && !this.target) {
-        component.patched();
-        if (component.__owl__.patchedCB) {
-          component.__owl__.patchedCB();
+      // call patched/mounted hook on each fiber of (reversed) patchQueue
+      for (let i = patchLen - 1; i >= 0; i--) {
+        fiber = patchQueue[i];
+        component = fiber.component;
+        if (fiber.shouldPatch && !this.target) {
+          component.patched();
+          if (component.__owl__.patchedCB) {
+            component.__owl__.patchedCB();
+          }
+        } else if (this.target ? inDOM : true) {
+          component.__callMounted();
         }
-      } else if (this.target ? inDOM : true) {
-        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);
+          task.fiber.complete();
+          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;
   });