fix: restore getter error details (#631)

* fix: restore getter error details

(#596) broke QUnit error messages so they now miss contextual error
info like page object path and selector.

This happened cause #596 relied on the `.toString()` method of the Error
to build the final error message. However, that's not how QUnit is
displaying the error message. It just uses the `Error.message`.

So let's build the final message in the `Error` constructor.

addon/src/-private/better-errors.js

@@ -22,47 +22,63 @@ export function throwContextualError(node, filters, e) {
 export function throwBetterError(node, key, error, { selector } = {}) {
   let message = error instanceof Error ? error.message : error.toString();
 
-  const err = new PageObjectError(key, node, selector, message);
+  const wrapperError = new PageObjectError(message, {
+    cause: {
+      message,
+      key,
+      node,
+      selector,
+    },
+  });
+
   if (error instanceof Error && 'stack' in error) {
-    err.stack = error.stack;
+    wrapperError.stack = error.stack;
   }
 
-  console.error(err.toString());
-  throw err;
+  console.error(wrapperError.toString());
+  throw wrapperError;
 }
 
 export class PageObjectError extends Error {
-  constructor(label, node, selector, ...args) {
-    super(...args);
+  constructor(message, options = {}, ...args) {
+    const { cause } = options;
+    const { node, key, selector } = cause || {};
+
+    const errorDescription = buildErrorDescription(node, key, selector);
 
-    this.label = label;
-    this.node = node;
-    this.selector = selector;
+    super(
+      [message, errorDescription].filter(Boolean).join('\n'),
+      options,
+      ...args
+    );
   }
+}
 
-  toString() {
-    let { message, label, node, selector } = this;
-    if (label) {
-      let path = buildPropertyNamesPath(label, node);
-      message = `${message}\n\nPageObject: '${path.join('.')}'`;
-    }
+function buildErrorDescription(node, key, selector) {
+  const lines = [];
 
-    if (typeof selector === 'string' && selector.trim().length > 0) {
-      message = `${message}\n  Selector: '${selector}'`;
-    }
+  const path = buildPropertyNamesPath(node);
+  if (key) {
+    path.push(key);
+  }
+  lines.push(`\nPageObject: '${path.join('.')}'`);
 
-    return `Error: ${message}`;
+  if (typeof selector === 'string' && selector.trim().length > 0) {
+    lines.push(`  Selector: '${selector}'`);
   }
+
+  return lines.join('\n');
 }
 
-function buildPropertyNamesPath(leafKey, node) {
-  let path = [leafKey];
+function buildPropertyNamesPath(node) {
+  let path = [];
 
   let current;
   for (current = node; current; current = Ceibo.parent(current)) {
     path.unshift(Ceibo.meta(current).key);
   }
 
+  // replace "root" with "page"
   path[0] = 'page';
 
   return path;

addon/src/macros/getter.js

@@ -49,8 +49,17 @@ export function getter(fn) {
         return fn.call(this, pageObjectKey);
       } catch (e) {
         if (e instanceof PageObjectError) {
-          if (!e.label) {
-            e.label = pageObjectKey;
+          if (!e.cause.key) {
+            // re-throw with a `pageObjectKey` to have a complete error message
+            const wrapperError = new PageObjectError(e.cause.message, {
+              cause: {
+                ...e.cause,
+                key: pageObjectKey,
+              },
+            });
+            wrapperError.stack = e.stack;
+
+            throw wrapperError;
           }
 
           throw e;

test-app/tests/integration/comma-separated-selector-test.ts

@@ -16,7 +16,7 @@ module('comma separated selectors', function (hooks) {
 
     assert.throws(
       () => page.isVisible,
-      new Error(
+      new RegExp(
         'Usage of comma separated selectors is not supported. Please make sure your selector targets a single selector.'
       )
     );
@@ -31,7 +31,7 @@ module('comma separated selectors', function (hooks) {
 
     assert.throws(
       () => page.text,
-      new Error(
+      new RegExp(
         'Usage of comma separated selectors is not supported. Please make sure your selector targets a single selector.'
       )
     );
@@ -50,7 +50,7 @@ module('comma separated selectors', function (hooks) {
 
     assert.throws(
       () => page.text,
-      new Error(
+      new RegExp(
         'Usage of comma separated selectors is not supported. Please make sure your selector targets a single selector.'
       )
     );

test-app/tests/unit/-private/properties/getter-test.ts

@@ -89,7 +89,7 @@ module('getter', function (hooks) {
 
     try {
       page.foo;
-      assert.true(false);
+      assert.false(true, 'should not succeed');
     } catch (e) {
       assert.strictEqual(
         e?.toString(),
@@ -109,6 +109,14 @@ PageObject: 'page.foo'`
       }),
     });
 
-    assert.throws(() => page.foo, /Selector: '.non-existing-scope'/);
+    try {
+      page.foo;
+      assert.false(true, 'should not succeed');
+    } catch (e: any) {
+      assert.strictEqual(
+        e.toString(),
+        `Error: Element not found.\n\nPageObject: 'page.foo'\n  Selector: '.non-existing-scope'`
+      );
+    }
   });
 });

test-app/tests/unit/extend/find-one-test.ts

@@ -40,13 +40,18 @@ module(`Extend | findOne`, function (hooks) {
   });
 
   test('throws error if 0 elements found', async function (assert) {
-    const page = create({});
+    const page = create({
+      child: {},
+    });
 
     await render(hbs`<span class="ipsum"></span>`);
 
     assert.throws(
-      () => findOne(page, '.unknown', {}),
-      /Error: Element not found./
+      () => findOne(page.child, '.unknown', {}),
+      new Error(`Element not found.
+
+PageObject: 'page.child'
+  Selector: '.unknown'`)
     );
   });