Exceptions in update lifecycle are not handled

[blikblum]

Description

If an exception is throw inside updated method, the next property changes do not updates the element

This occurs because, after the error is throw, the check if (!this._hasRequestedUpdate) in _invalidate never evaluates to true (the update Promise is never resolved)

Live Demo

https://codepen.io/blikblum/pen/zmEGXL?editors=1001

Steps to Reproduce

1. Throw an exception in updated method
2. Change an observed property

Expected Results

It should continue to update on property changes.

Actual Results

It stops to update on property changes

Browsers Affected

• Chrome
• Firefox
• Edge
• Safari 11
• Safari 10
• IE 11

Versions

• lit-element: v0.6.2
• webcomponents: latest

[sorvell]

I think implementing this would requite catching the error via try... catch. We'd consider this, but we need to ensure that there's no performance regression.

[blikblum]

If there's performance regression i think it can leave as is and document, so users can catch the errors in specific cases

[justinfagnani]

I think we need to fix this. Not handling exceptions causes other user-visible problems, like Promises that never resolve. That violates Promise contracts in a pretty bad way.

Here's a simple example:

import { LitElement, html } from 'lit-element';

class TestElement extends LitElement {
  render() {
    throw new Error('Test');
    return html`<p>Hello!</p>`;
  }
}
customElements.define('test-element', TestElement);

(async () => {
  let el = new SimpleGreeting();
  try {
    await el.requestUpdate();
    console.log('Resolved');
  } catch (e) {
    console.log('Rejected', e);
  }
})();

[justinfagnani]

Copying in a response from me to an internal user who hit something like this issue when throwing in render():

I added this to the next release milestone. I said I think we should fix, but as we discuss the fix it it's possible that instead of rejecting the requestUpdate() / updateComplete Promise, we'll resolve it. Given that we may have multiple, basically uncoordinated, points waiting on an update, I want to consider what they could actually do in response to an exception in rendering. If it's usually nothing, rejecting may unnecessarily complicate using this API - code that definitely wants to know when rendering has completed will need to try/catch and await.

I think you should possible reconsider throwing an exception in render(). Aside from propagating an error to waiting code, all it does is abandon the render and preserve the last state, which might not be reflective of anything good at all. It's probably better to decide exactly what you want to display in errors states and just display that. Consider that no native elements throw in rendering, because rendering is basically unobservable as a step to user code.

If the goal here is to communicate a failure to other code, I'd like to understand more how the other code will respond. In a synchronous model like React, render exceptions are usually used to just show a nice stack trace over the app in development. We could target something like that instead by firing an error event.

[justinfagnani]

cc @sorvell