Uncought exceptions in certain lifecycle methods are handled poorly

[brainlessbadger]

Description

Currently, in case of uncaught exception from render, updated and possibly some other methods, LitElement will

a) display stale UI until next update
b) "swallow" the exception from the console on next update (I guess one of the methods wraps previous update promise in an empty catch...?)

Those two combined can make some exceptions on quickly updating elements exceptionally hard to notice early, occasionally making the development more painful then it should be.

Acceptance criteria

I think that at very least LitElement should make sure that uncaught exceptions stay clearly visible in console at all times, but maybe considering more comprehensive approach to failed updates would also be in place? E.g. in analogous situation React will prefer killing the app from displaying stale UI to the user (not suggesting this in particular, just for contrast).

[sorvell]

a) display stale UI until next update

This should generally not be the case. The error handling we do is primarily intended to make sure that the update cycle cannot be disrupted by an exception.

b) "swallow" the exception from the console on next update

This is unintentional, and we'll work on fixing it. We may elect to either use console.error or throw the error asynchronously, again, to try to minimize disruption from the error.

I also want to point out that to the next major version of LitElement, we're adding a dev/prod mode switch, and we'll consider making dev mode not do any error handling to surface errors more clearly during development.

[brainlessbadger]

Thanks for response.

b) "swallow" the exception from the console on next update

This is unintentional, and we'll work on fixing it. We may elect to either use console.error or throw the error asynchronously, again, to try to minimize > disruption from the error.

This makes me happy and was the most important part really.

a) display stale UI until next update

This should generally not be the case. The error handling we do is primarily intended to make sure that the update cycle cannot be disrupted by an exception.

Let me provide an example, because I don't think I was clear about this one. A component with this code:

render() {
    if (this.counter % 3 == 0) {
        throw new Error();
    }

    return html`
        ${this.counter}
    `;
}

connectedCallback() {
    super.connectedCallback();
    this.counter = 0;
    setInterval(() => {
        this.counter++;
    }, 500);
}

When this.counter == 3, the render method will fail, but the component doesn't do anything about it, and will keep displaying UI described by previous render call, when counter was equal 2, which is what I meant by "stale UI". For me risk of stale UI is a big problem.

I'm currently attempting to solve this in "user-land" by subclassing LitElement and dispatching events that can be handled to restore (or "nuke") the faulty piece of UI, i.e.

render() {
    try {
        return this.renderSafely();
    } catch (ex) {
        this.dispatchEvent(new CustomEvent('unhandled-exception', {detail: ex, composed: true, bubbles: true}))

        return null;
    }
}

but I find user-land solution sub-optimal because I have to rename all life cycle methods, and (more importantly) can't do that to components where I have no control of the code.

This is less important and I'm not sure if LitElement itself should do anything about such exceptions, or if so - what exactly, but I thought it was worth mentioning since I was already creating an issue.

[sorvell]

After evaluating the current code, we've determined that squelching errors is important to ensure our contract that an error in one update cycle should not prevent the next update cycle from proceeding normally. However, we are going to make sure to re-fire any caught errors so devs have a chance to observe them. The plan is to do this asynchronously. This way you can implement window.onunhandledrejection to see the error.

If it's important to your use case to see any errors synchronously, you can dispatch an event as you've done. One thing I'd like to point out about this is that you can do this once in performUpdate rather than needing to override individual lifecycle methods. Something like this:

performUpdate() {
    try {
        return super.performUpdate();
    } catch (ex) {
        this.dispatchEvent(new CustomEvent('unhandled-exception', {detail: ex, composed: true, bubbles: true}))
        return null;
    }
}

Hope that helps.

[sorvell]

Closing via lit/lit#1372.