[FIX] components: fix cause left unset when thrown object is not Error

Previously, when wrapping errors in wrapError, if the error was not an
actual error object, we wouldn't set the cause property on the wrapping
error correctly. The "instanceof Error" check is simply there so that we
can know whether we can add the original errors message to the wrapping
error, but the line that sets the error's cause was mistakenly moved
into that condition.

This commit also fixes the wrapping error's message in the case of
non-Error objects, to avoid having "the following error occurred in
hookname:" with nothing after the colon which is confusing/misleading.

src/runtime/error_handling.ts

@@ -45,7 +45,10 @@ export function handleError(params: ErrorParams) {
   let { error } = params;
   // Wrap error if it wasn't wrapped by wrapError (ie when not in dev mode)
   if (!(error instanceof OwlError)) {
-    error = Object.assign(new OwlError("An error occured in the owl lifecycle"), { cause: error });
+    error = Object.assign(
+      new OwlError(`An error occured in the owl lifecycle (see this Error's "cause" property)`),
+      { cause: error }
+    );
   }
   const node = "node" in params ? params.node : params.fiber.node;
   const fiber = "fiber" in params ? params.fiber : node.fiber!;

src/runtime/lifecycle_hooks.ts

@@ -10,9 +10,11 @@ function wrapError(fn: (...args: any[]) => any, hookName: string) {
   const node = getCurrent();
   return (...args: any[]) => {
     const onError = (cause: any) => {
+      error.cause = cause;
       if (cause instanceof Error) {
-        error.cause = cause;
         error.message += `"${cause.message}"`;
+      } else {
+        error.message = `Something that is not an Error was thrown in ${hookName} (see this Error's "cause" property)`;
       }
       throw error;
     };

tests/components/__snapshots__/error_handling.test.ts.snap

@@ -101,7 +101,7 @@ exports[`basics simple catchError 2`] = `
 }"
 `;
 
-exports[`can catch errors Errors in owl lifecycle are wrapped in dev mode: async hook 1`] = `
+exports[`can catch errors Errors have the right cause 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
   let { text, createBlock, list, multi, html, toggler, comment } = bdom;
@@ -112,7 +112,7 @@ exports[`can catch errors Errors in owl lifecycle are wrapped in dev mode: async
 }"
 `;
 
-exports[`can catch errors Errors in owl lifecycle are wrapped in dev mode: sync hook 1`] = `
+exports[`can catch errors Errors in owl lifecycle are wrapped in dev mode: async hook 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
   let { text, createBlock, list, multi, html, toggler, comment } = bdom;
@@ -145,6 +145,28 @@ exports[`can catch errors Errors in owl lifecycle are wrapped outside dev mode:
 }"
 `;
 
+exports[`can catch errors Thrown values that are not errors are wrapped in dev mode 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(ctx['state'].value);
+  }
+}"
+`;
+
+exports[`can catch errors Thrown values that are not errors are wrapped outside dev mode 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(ctx['state'].value);
+  }
+}"
+`;
+
 exports[`can catch errors an error in onWillDestroy 1`] = `
 "function anonymous(app, bdom, helpers
 ) {

tests/components/error_handling.test.ts

@@ -511,7 +511,7 @@ describe("can catch errors", () => {
     );
   });
 
-  test("Errors in owl lifecycle are wrapped in dev mode: sync hook", async () => {
+  test("Errors have the right cause", async () => {
     const err = new Error("test error");
     class Root extends Component {
       static template = xml`<t t-esc="state.value"/>`;
@@ -574,7 +574,9 @@ describe("can catch errors", () => {
     } catch (error) {
       e = error as OwlError;
     }
-    expect(e!.message).toBe("An error occured in the owl lifecycle");
+    expect(e!.message).toBe(
+      `An error occured in the owl lifecycle (see this Error's "cause" property)`
+    );
     expect(e!.cause).toBe(err);
   });
 
@@ -597,10 +599,58 @@ describe("can catch errors", () => {
     } catch (error) {
       e = error as OwlError;
     }
-    expect(e!.message).toBe("An error occured in the owl lifecycle");
+    expect(e!.message).toBe(
+      `An error occured in the owl lifecycle (see this Error's "cause" property)`
+    );
     expect(e!.cause).toBe(err);
   });
 
+  test("Thrown values that are not errors are wrapped in dev mode", async () => {
+    class Root extends Component {
+      static template = xml`<t t-esc="state.value"/>`;
+      state = useState({ value: 1 });
+
+      setup() {
+        onMounted(() => {
+          throw "This is not an error";
+        });
+      }
+    }
+    let e: OwlError;
+    try {
+      await mount(Root, fixture, { test: true });
+    } catch (error) {
+      e = error as OwlError;
+    }
+    expect(e!.message).toBe(
+      `Something that is not an Error was thrown in onMounted (see this Error's "cause" property)`
+    );
+    expect(e!.cause).toBe("This is not an error");
+  });
+
+  test("Thrown values that are not errors are wrapped outside dev mode", async () => {
+    class Root extends Component {
+      static template = xml`<t t-esc="state.value"/>`;
+      state = useState({ value: 1 });
+
+      setup() {
+        onMounted(() => {
+          throw "This is not an error";
+        });
+      }
+    }
+    let e: OwlError;
+    try {
+      await mount(Root, fixture);
+    } catch (error) {
+      e = error as OwlError;
+    }
+    expect(e!.message).toBe(
+      `An error occured in the owl lifecycle (see this Error's "cause" property)`
+    );
+    expect(e!.cause).toBe("This is not an error");
+  });
+
   test("can catch an error in the initial call of a component render function (parent mounted)", async () => {
     class ErrorComponent extends Component {
       static template = xml`<div>hey<t t-esc="state.this.will.crash"/></div>`;