diff --git a/src/component/component.ts b/src/component/component.ts index b1ef1462..e13c1b2d 100644 --- a/src/component/component.ts +++ b/src/component/component.ts @@ -266,7 +266,7 @@ export class Component { * 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 { } else { this.__render(fiber); } - return new Promise(resolve => { - scheduler.addFiber(fiber, () => { + return new Promise((resolve, reject) => { + 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 { } const fiber = new Fiber(null, this, this.props, undefined, undefined, force); this.__render(fiber); - return new Promise(resolve => { - scheduler.addFiber(fiber.root, () => { + return new Promise((resolve, reject) => { + scheduler.addFiber(fiber.root, err => { + if (err) { + reject(err); + return; + } if (__owl__.isMounted && fiber === fiber.root) { fiber.patchComponents(); } diff --git a/src/component/fiber.ts b/src/component/fiber.ts index 182c5dee..7923ba12 100644 --- a/src/component/fiber.ts +++ b/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, 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(); } } diff --git a/src/component/scheduler.ts b/src/component/scheduler.ts index 23c5f8fe..87511af6 100644 --- a/src/component/scheduler.ts +++ b/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; diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts index d2593403..8e090872 100644 --- a/tests/component/component.test.ts +++ b/tests/component/component.test.ts @@ -1090,12 +1090,15 @@ describe("composition", () => { static components = { SomeWidget: Widget }; } const parent = new Parent(env); - await parent.mount(fixture); - - expect(console.error).toBeCalledTimes(1); - expect((console.error).mock.calls[0][0].message).toMatch( - 'Cannot find the definition of component "SomeMispelledWidget"' - ); + let error; + try { + await parent.mount(fixture); + } catch (e) { + 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); - - expect(console.error).toBeCalledTimes(1); - expect((console.error).mock.calls[0][0].message).toMatch( - 'Observed state cannot be changed here! (key: "coffee", val: "2")' - ); + let error; + try { + await parent.mount(fixture); + } catch (e) { + 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(` - -
hey -
-
- -
-
`); - class ErrorComponent extends Widget {} + + class ErrorComponent extends Widget { + static template = xml`
hey
`; + } + class App extends Widget { - state = useState({ flag: false }); + static template = xml`
`; 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("
Error handled
"); }); + + 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 { + static template = xml`
`; + } + + 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 { + static template = xml`
`; + } + class App extends Component { + static template = xml`
`; + 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 { + static template = xml`
`; + flag = false; + } + + const app = new App(env); + await app.mount(fixture); + expect(fixture.innerHTML).toBe("
"); + 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", () => {