Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify template error message once error causes are better-supported. #169

Open
theengineear opened this issue Feb 13, 2024 · 2 comments

Comments

@theengineear
Copy link
Collaborator

See #128 for the original motivation here. Just opening this ticket to simplify some of our error logging to keep things as tidy as possible.

Current:

  render() {
    const { template, properties, renderRoot, render } = XElement.#hosts.get(this);
    const result = template(properties, this);
    try {
      render(renderRoot, result);
    } catch (error) {
      const pathString = XElement.#toPathString(this);
      const message = `${error.message} — Invalid template for "${this.constructor.name}" at path "${pathString}"`;
      throw new Error(message, { cause: error });
    }
  }

Proposed:

  render() {
    const { template, properties, renderRoot, render } = XElement.#hosts.get(this);
    const result = template(properties, this);
    try {
      render(renderRoot, result);
    } catch (error) {
      const pathString = XElement.#toPathString(this);
      const message = `Invalid template for "${this.constructor.name}" at path "${pathString}"`;
      throw new Error(message, { cause: error });
    }
  }

☝️ — See the difference? We ought to not have to literally repeat / embed the original error in our own messaging.

@theengineear
Copy link
Collaborator Author

Note that this is currently supported in Firefox. Here’s a link to the Chrome bug.

Screenshot 2024-02-13 at 1 15 37 PM

@theengineear
Copy link
Collaborator Author

This seems to work in all browsers now. BUT — we may want to consider that the cause there may or may not itself have an error associated. I.e., an error cause could be the original error, or it could be any compound object… I’m actually surprised / disappointed to learn this. It feels like a bad abstraction to me 🤷

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant