From 27b4eece66275d898fb01fe2c20968d7963c8118 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Wed, 20 Nov 2019 13:19:00 +0100
Subject: [PATCH] [FIX] component: display correct error in some case

It could happen that a component would crash. But then, the __render
code tried to copy the class properties into the vnode, which does not
exist, creating a new error.

So, the solution is to process the classObj only in the successful part
of the render call, which then will not crash, so the normal error
handling will occur.
---
 src/component/component.ts        | 14 +++++++-------
 tests/component/component.test.ts | 21 +++++++++++++++++++++
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/src/component/component.ts b/src/component/component.ts
index c72879b6..c31daaeb 100644
--- a/src/component/component.ts
+++ b/src/component/component.ts
@@ -597,6 +597,13 @@ export class Component<T extends Env, Props extends {}> {
         throw new Error(`Rendering '${this.constructor.name}' did not return anything`);
       }
       fiber.vnode = vnode;
+      // we apply here the class information described on the component by the
+      // template (so, something like <MyComponent class="..."/>) to the actual
+      // root vnode
+      if (__owl__.classObj) {
+        const data = vnode.data!;
+        data.class = Object.assign(data.class || {}, __owl__.classObj);
+      }
     } catch (e) {
       error = e;
     }
@@ -604,13 +611,6 @@ export class Component<T extends Env, Props extends {}> {
       __owl__.observer.allowMutations = true;
     }
 
-    // we apply here the class information described on the component by the
-    // template (so, something like <MyComponent class="..."/>) to the actual
-    // root vnode
-    if (__owl__.classObj) {
-      const data = fiber.vnode!.data!;
-      data.class = Object.assign(data.class || {}, __owl__.classObj);
-    }
     fiber.root.counter--;
     fiber.isRendered = true;
     if (error) {
diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts
index 507ef5cf..9cd94647 100644
--- a/tests/component/component.test.ts
+++ b/tests/component/component.test.ts
@@ -1718,6 +1718,27 @@ describe("class and style attributes with t-component", () => {
 
     expect(fixture.innerHTML).toBe(`<div><div style="font-size: 30px;"></div></div>`);
   });
+
+  test("error in subcomponent with class", async () => {
+    class Child extends Widget {
+      static template = xml`<div t-esc="this.will.crash"/>`;
+    }
+    class ParentWidget extends Widget {
+      static template = xml`<div><Child class="a"/></div>`;
+      static components = { Child };
+    }
+    const widget = new ParentWidget();
+    let error;
+    try {
+      await widget.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe("Cannot read property 'crash' of undefined");
+    expect(fixture.innerHTML).toBe("");
+  });
+
 });
 
 describe("other directives with t-component", () => {