componentDidCatch - preventing the app from breaking when a rendering problem occurs

[NullVoxPopuli]

Some history first:

componentDidCatch (from react), is a new~ish feature (as of v2 (16)). It is super handy for catching rendering errors in components. Since react is all components, it'd be silly to have componentDidCatch in every component, so what I ended up doing, was making my own Route component (extending from react-router -- very similar to ember's Route, but less structured, no data handling), where an error catcher component wrapped the sub-tree of components.

This provided the following benefits:

• when my data fetching threw an error, such as 401 or 403, I could have a pre-defined shared error-rendering component for a consistent error look and feel throughout the app, and it would be contextual within the sub-page / route -- this is similar to how our error template from routes works today (however, see An alternative to Controllers #499 for using components for error rendering)
• (mainly what this pre-RFC is about) when any component in the sub-tree errored, either because of a malformed template or due to a typical javascript runtime error, such as calling a method on undefined, or accessing a property on undefined, the error message would be caught by the nearest ancestor's componentDidCatch, so that we may render an appropriate and meaningful error message to not confuse our users and get support requests saying "it's broken, pls send help".

So, my question to the community and core team members:

• what do you think of extending the current route-error catching functionality to include rendering errors?
• what sort of details need to be known for implementation? (I haven't explored that part of the code myself yet)

For reference: examples of errors not caught by the existing error handling mechanisms, and what the consequences are: https://codesandbox.io/s/error-examples-ydped

[webark]

any rendering error should be caught at compile time. For runtime errors, there is the error action on a route that catches the thrown errors from at least the route and controller. It might catch component runtime errors too.

[NullVoxPopuli]

@webark a rendering error can't be caught at compile time if there is a conditional render.
I'll make some examples

[lifeart]

should we have it on component level?
Just wondering, if modifier have sense on it?

<div {{on-render-error this.onError}}>
  ....
</div>

{{#each this.items as |item|}}
   <MyListItem {{on-render-error (fn  this.catchListItemRenderError item)}} />
{{/each}}

[NullVoxPopuli]

I think something like that could be a nice thing to provide, and maybe the default route template is wrapped in a tagless version of that.

Ignore the rest of this comment, I made the mistake of using a personal ideal API, which uses code from an unmerged RFC, #499.

Ultimately, I think an API like:

import Route from '@ember/routing/route';

import FooComponent from './components/Foo';
import ErrorComponent from 'my-app/components/generic-error';
import LoadingComponent from 'my-app/components/spinner';

export default class extends Route {
  static render = FooComponent;
  static loading = LoadingComponent;
  static error = ErrorComponent;

  async model() {
    const contacts = await this.store.findAll('contact');

    return { contacts };
  }
}

where if an error happens anywhere in the tree of LoadingComponent or FooComponent, ErrorComponent is renedered.
and if the ErrorComponent errors, maybe we catch at the parent route.

[rtablada]

I’d favor this being a modifier rather than adding api surface to routes. It gives you the flexibility of catching errors in confined areas or segments and IMO that’s a lot more power with less side effects and complexity.

[NullVoxPopuli]

I’d favor this being a modifier rather than adding api surface to routes.

I def don't want to do that. I'd prefer 0 api surface changes. Just adding behavior to the existing api surface (my sample showed code proposed in RFC #499)

[lifeart]

I think we need more declarative way for root components, like cerebral pipelines - https://cerebraljs.com/docs/api/sequence.html

export default class extends Route {
  static states = {
    loading: FooLoading,
    error: FooLoadingError,
    pending: FooPendingUserAction,
    success: FooListItems
  }
  private currentState = 'pending';
  
  @task
  model(transition) {
    try {
      const data = yield fetch('data.json')
      this.router.currentRoute.switchTo('succes', transition, data);
    } catch (e) {
      transition.abort();
      this.router.currentRoute('pending', new Transition('....') , { error: e });
    }
  }
}

[NullVoxPopuli]

I think that's out of scope for this proposed RFC.

taking from this page: https://guides.emberjs.com/release/routing/loading-and-error-substates/#toc_error-substates

I'm mostly just proposing using these existing error-handling techniques for catching rendering errors

[lifeart]

In this case, we need to pair transition and glimmer-vm error emitter, including ability to safely close render context and get minal working dom.

[lifeart]

let's imagine template like this:

  <div> | ctx1
    <div>  | ctx2
      <ul>   | ctx3
        {{#each this.items as |item index|}}  | ctx3+index 
          <MyLi @data={{item}} /> 
        {{/each}}
      </ul>
    </div>
  <div>

and error in last each iteration.

given:

rendering loop started:

  mv ctx1
  create element div
  mv ctx2
  create element div
  mv ctx3
  create element ul
  mv loop context
  mv ctx3 + 1
  -> render MyLi...
  mv ctx3 + 2
  -> render MyLi...
  mv ctx3 + 3
  -> render MyLi...
  
  Error!!!!


  here we need to find closest `route` context, remove all rendered nodes for this context and it's children,
  switch context to `error` and rerender error component.
  
  as result we unable to get exact render error position, due to new transition

[lifeart]

in ^ this case modifier error has more sense, because we can localize scope of error

[NullVoxPopuli]

couldn't the error modifier be implicitly implemented for the route template?

[NullVoxPopuli]

Examples of errors not caught by the existing error handling mechanisms, and what the consequences are: https://codesandbox.io/s/error-examples-ydped

[rtablada]

From a teaching and mental model, thinking about this I had some more thoughts.

Since this is a rendering error handler where do we work with rendering: templates. Routes are mostly independent of the rendering cycle. Also because of existing paradigms and different patterns teams may have UI and behavior defined in different locations (Routes, Controllers, Components).

In a sense it can be thought that routes are loading/async handlers that populate component args (or currently the state properties on controllers for teams that are controller heavy). Moving render error handling to routes muddles the purpose of routes in my mind because what is a data loader now starts messing more and more with UI/rendering rather than being somewhat
of a pure collection of data loading functions.

---

@NullVoxPopuli you do make a good point on how to pick the error component for a given route though, this gets a bit trickier. On one hand giving a 1-1 mapping for the existing x-error routable templates is useful. But, to give an alternative perspective on that, maybe it could be the purpose of the route to send in args when catching errors? For example

Route {
  async model() {
    try {
      let value = await somethingAsync();
      
      return { value };
    } catch(e) {
      return { error: e };
    }
  }
}

Then in the component/template:

{{#if @error}}
  <SomeErrorHandlingComponent @error={{@error}} />
{{else}}
  {{!-- Your usual stuff here --}}
{{/if}}

With this approach rendering errors can be handled using modifiers which keeps UI and rendering all a concern of components.

[NullVoxPopuli]

I get what you're saying. I guess I want some form of error handling by default. In the codesandbox I made, there are situations in which you can break the entire app, or get the outlet repeated over and over.

While, I'm all for having the ability to manually handle the error, I think what we have today needs some new defaults.

I like what you said about the Route staying away from the rendering layer, and being more something that passes args to the rendering layer.

In that case, the route could have a default onRenderError arg that is passed to the rendering context (this rendering context being an implicit layer above the route template), so that we can provide improved DX when accidents happen