From 52d0526ddd2e9ad3f70209ddc6166f294f6706f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Tue, 11 Jan 2022 09:39:26 +0100
Subject: [PATCH] [FIX] portal: properly handle errors

Before this commit, Portal overrode the _render function for its
component node, which means it bypassed the error handling mechanism
that was implemented in that method.  It could have been fixed by
duplicating the error handling code as well, but a better solution in my
opinion is to simply override the renderFn function.  This is closer to
the actual intent of the portal implementation: wrap the result of the
rendering in a VPortal vnode.
---
 src/portal.ts                                | 14 +++----
 tests/misc/__snapshots__/portal.test.ts.snap | 39 ++++++++++++++++++++
 tests/misc/portal.test.ts                    | 37 +++++++++++++++++++
 3 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/src/portal.ts b/src/portal.ts
index 68f654e9..7085b482 100644
--- a/src/portal.ts
+++ b/src/portal.ts
@@ -1,7 +1,6 @@
-import type { ComponentNode } from "./component/component_node";
-import { Component } from "./component/component";
 import { xml } from "./app/template_set";
 import { BDom, text, VNode } from "./blockdom";
+import { Component } from "./component/component";
 
 const VText: any = text("").constructor;
 
@@ -60,12 +59,9 @@ export class Portal extends Component {
     slots: true,
   };
 
-  constructor(props: any, env: any, node: ComponentNode) {
-    super(props, env, node);
-    node._render = function (fiber: any) {
-      const bdom = new VPortal(props.target, this.renderFn());
-      fiber.bdom = bdom;
-      fiber.root.counter--;
-    };
+  setup() {
+    const node = this.__owl__;
+    const renderFn = node.renderFn;
+    node.renderFn = () => new VPortal(this.props.target, renderFn());
   }
 }
diff --git a/tests/misc/__snapshots__/portal.test.ts.snap b/tests/misc/__snapshots__/portal.test.ts.snap
index c5b09eee..36119058 100644
--- a/tests/misc/__snapshots__/portal.test.ts.snap
+++ b/tests/misc/__snapshots__/portal.test.ts.snap
@@ -409,6 +409,45 @@ exports[`Portal portal's parent's env is not polluted 2`] = `
 }"
 `;
 
+exports[`Portal simple catchError with portal 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  let block1 = createBlock(\`<div><block-child-0/><block-child-1/></div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2,b3;
+    if (ctx['error']) {
+      b2 = text(\`Error\`);
+    } else {
+      b3 = component(\`Boom\`, {}, key + \`__1\`, node, ctx);
+    }
+    return block1([], [b2, b3]);
+  }
+}"
+`;
+
+exports[`Portal simple catchError with portal 2`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  let block1 = createBlock(\`<div><span>1</span><block-child-0/></div>\`);
+  let block2 = createBlock(\`<p><block-text-0/></p>\`);
+  
+  function slot1(ctx, node, key = \\"\\") {
+    let txt1 = ctx['a'].b.c;
+    return block2([txt1]);
+  }
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b3 = component(\`Portal\`, {target: '#outside',slots: {'default': {__render: slot1, __ctx: ctx}}}, key + \`__1\`, node, ctx);
+    return block1([], [b3]);
+  }
+}"
+`;
+
 exports[`Portal with target in template (after portal) 1`] = `
 "function anonymous(bdom, helpers
 ) {
diff --git a/tests/misc/portal.test.ts b/tests/misc/portal.test.ts
index 2168681f..9baf12f3 100644
--- a/tests/misc/portal.test.ts
+++ b/tests/misc/portal.test.ts
@@ -68,6 +68,43 @@ describe("Portal", () => {
     expect(fixture.innerHTML).toBe('<div id="outside"><p>2</p></div><div><span>1</span></div>');
   });
 
+  test("simple catchError with portal", async () => {
+    class Boom extends Component {
+      static components = { Portal };
+      static template = xml`
+          <div>
+            <span>1</span>
+            <Portal target="'#outside'">
+              <p><t t-esc="a.b.c"/></p>
+            </Portal>
+          </div>`;
+    }
+
+    class Parent extends Component {
+      static template = xml`
+        <div>
+          <t t-if="error">Error</t>
+          <t t-else="">
+            <Boom />
+          </t>
+        </div>`;
+      static components = { Boom };
+
+      error: any = false;
+
+      setup() {
+        onError((err) => {
+          this.error = err;
+          this.render();
+        });
+      }
+    }
+    addOutsideDiv(fixture);
+
+    await mount(Parent, fixture);
+    expect(fixture.innerHTML).toBe('<div id="outside"></div><div>Error</div>');
+  });
+
   test("basic use of portal in dev mode", async () => {
     class Parent extends Component {
       static components = { Portal };