Errors in render() persist even after removing component

[angus-c]

I reproduced this with a simple example (source code below)
<App> is the parent component, <Profile> is the child

1. Clicking the button in Profile sets state.error to true which successfully renders the error page.
2. If instead I cause a TypeError in Profile (a plausible runtime occurrence if an object property is null), the error page does not show, even though state.error is still set to true.
   e.g. change to <div class={style.profile.x.y}> in Profile
3. If I repeat 2) but set default state.error to true (so that Profile is never rendered) the error page successfully renders.

The difference between 2) and 3) suggests there might be an issue replacing a component that rendered with an error.

app.js

import { h, Component } from 'preact';
import Profile from './profile';
export default class App extends Component {

  constructor(props) {
    super(props);
    this.state = {
      error: true
    };

    window.onerror = () => {
      this.setState({
        error: false
      });
    };
  }

  throwError = e => {
    throw new Error('error generated');
  };

  render() {
    console.log('this.state.error', this.state.error);
    return (
      <div id="app">
        {this.state.error ?
          <div>{'error, please refresh'}</div> :
          <Profile throwError={this.throwError}/>
        }
      </div>
    );
  }
}

profile.js

import { h, Component } from 'preact';
import style from './style';

export default class Profile extends Component {
  render({throwError}) {
    return (
      <div class={style.profile}>
        <h1>Profile.</h1>
        <p>This is the profile page</p>
        <button onclick={throwError}>{'Throw Error'}</button>
      </div>
    );
  }
}

[developit]

An exception encountered during rendering (essentially, within render() or a PFC) halts rendering but leaves pending renders queued. The next setState() call will attempt to re-render the broken components.

Philosophically, what could Preact do here? The child component cannot be rendered, since its render throws and thus has no return value. However, skipping its render would be a silent error.

FWIW I've actually used this behavior to implement error boundaries. Basically, you install a VNode hook that automatically wraps all component prototype methods in error softening via console.error(). A naive version looks something like this:

import { options } from 'preact';

let old = options.vnode;
options.vnode = vnode => {
  if (typeof vnode.nodeName==='function') {
    wrapComponent(vnode.nodeName);
  }
  if (old) old(vnode);
};

function wrapComponent(Component) {
  let proto = Component.prototype;
  if (!proto || !proto.render || Component.__safe) return;
  Component.__safe = true;   // only wrap components once
  for (let i in proto) if (i!=='constructor' && typeof proto[i]==='function') {
    proto[i] = wrapMethod(proto[i]);
  }
}

const wrapMethod = fn => function() {
  try {
    return old.apply(this, arguments);
  } catch(e) { console.error(e); }
};

Interestingly, because the behavior is entirely customizable via that hook, you could install a development error handler that intentionally blows up or console.error()'s, but then switch to a production error handler that sequesters logging and attempts to continue rendering if possible to avoid white-screening.

That brings me to the crux of the issue though: if a component errors, I'm not sure that there is a great way to recover or roll back. Any way you go about it, an error is going to produce incorrect UI state that was not expected.

Just my thoughts, I had been working on something like this over the past little while so it's fresh in my brain. Happy you raised the issue here, maybe we can come up with a great open-source solution (either within preact or as a library).

[angus-c]

Thanks for the quick reply!

Yeah I was wondering if this came down to React philosophy. React-aside, render/PFCs are just functions, and from that perspective it's surprising that previous exceptions should influence subsequent invocations (or in this case non-invocations).

Is the real issue here VDOM? i.e. because the previous render failed, subsequent renders have no valid DOM baseline to compare to? Thus failing renders must be queued for re-render until a valid DOM is restored. If that's the case (and I might well be barking up the wrong tree, so humor me here), it feels like VDOM could instead use the last non-failing render as its DOM baseline. .

Thanks for the VNode hook tip. This might do the trick (if the perf of the extra function calls isn't an issue). I'd settle for wrapping Component.prototype.render and when exceptions are caught returning null and firing an error action. (The motivation is to catch store changes which might cause TypeErrors to be thrown in long running browser sessions).

[developit]

@angus-c you might find it interesting to know - Next.js uses an approach very similar to my suggestion, and we did some benchmarking to determine if there'd be a performance issue due to the try/catch. The wrapper function gets deopted as you might expect, but interestingly, since there's basically nothing else happening in that wrapper function, the original function within the safe() wrapper is actually still optimized (it's unaffected). So the net result is pretty really negligible difference, I think it was around 2% slower.

VDOM certainly doesn't help here - the key issue is that VDOM Components let you delegate instantiation and invocation into the framework (preact, react, etc) - that also then sortof forces you into delegating error handling to it as well.

I do think there's something in your thinking around queueing failed renders and waiting for the next valid render. Maybe that would be as simple as wrapping each internal renderComponent() call in a try/catch within Preact.

[angus-c]

We should check performance impacts of wrapping component's render method (see #531 (comment))

[developit]

Yup. I did some basic tests when Next.js added this to certain components.

[angus-c]

Cool, looks promising. Apologies, I meant to add this comment to my internal ticket, not Github :)

[angus-c]

Wrote a wrapper based on yours (and then conditionally don't render child components when in error state). It seems to be working well but feedback welcome.

Thanks again for your help!

import {options} from 'preact';
import window from 'global/window';

export default function handleRenderErrors() {

  const originalVnode = options.vnode;
  options.vnode = vnode => {
    if (typeof vnode.nodeName === 'function') {
      wrapComponent(vnode.nodeName);
    }
    if (originalVnode) {
      originalVnode(vnode);
    }
  };

  function wrapComponent(Component) {
    if (Component.prototype && Component.prototype.render && !Component.__safe) {
      Component.__safe = true;   // only wrap components once
      const originalRender = Component.prototype.render;
      Component.prototype.render = function render(...args) {
        try {
          return originalRender.apply(this, args);
        } catch (e) {
          window.onerror(`Error rendering component ${this.constructor.name}`, null, null, null, e, true);
          return null;
        }
      };
    }
  }
}

[rajeshsegu]

@developit wrapping component does not seem to work for functional components without any render method. We are planning to solve this by wrapping the functional component with a simple render component. What do you think about this approach ? Is there a better alternative ?

export default function handleRenderErrors() {

  const originalVnode = options.vnode;
  options.vnode = vnode => {
    if (typeof vnode.nodeName === 'function') {
      // Once wrapped, it would not be a functional component anymore
      if (isFunctionalComponent(vnode.nodeName)) {
        vnode.nodeName = wrapFunctionalComponent(vnode.nodeName);
      }

      wrapComponent(vnode.nodeName);
    }
    if (originalVnode) {
      originalVnode(vnode);
    }
  };

function wrapFunctionalComponent(FnComponent) {
  class WrapFunctionalComponent extends Component {
    render() {
      return h(FnComponent, {
        ...this.props,
      });
    }
  }
  WrapFunctionalComponent.displayName = `WrapFunctionalComponent(${getDisplayName(FnComponent)})`;
  if (WrapFunctionalComponent.defaultProps) {
    WrapFunctionalComponent.defaultProps = FnComponent.defaultProps;
  } else if (typeof FnComponent.getDefaultProps === 'function') {
    WrapFunctionalComponent.defaultProps = FnComponent.getDefaultProps();
  }
  return WrapFunctionalComponent;
}

function wrapComponent(Component) {
    // As in above comment
  }

export function isFunctionalComponent(Component) {
  return Component && ('function' === typeof Component) && !(Component.prototype && Component.prototype.render);
}

function getDisplayName(WrappedComponent) {
  return WrappedComponent.displayName || WrappedComponent.name || 'Component';
}

[developit]

I just wrap functional components in their own wrapper - you might find swapping functional components into classful ones produces odd side effects (importantly, it changes the constructor).

Something like this:

export default function handleRenderErrors() {
  const originalVnode = options.vnode;
  options.vnode = vnode => {
    if (typeof vnode.nodeName === 'function') {
      if (isFunctionalComponent(vnode.nodeName)) {
        vnode.nodeName = wrapFunctionalComponent(vnode.nodeName);
      }
      else {
        wrapComponent(vnode.nodeName);
      }
    }
    if (originalVnode) originalVnode(vnode);
  };
}

function wrapFunctionalComponent(FnComponent) {
    // only generate safe wrapper once
    if (FnComponent.__safe) return FnComponent.__safe;

    function Wrapper(props, context) {
      try { return FnComponent.call(this, props, context); }
      catch (err) { console.error(err); }
    }
    Wrapper.displayName = FnComponent.displayName;

    return Component.__safe = Wrapper;
}

function wrapComponent(Component) {
    // As in above comment
}

export function isFunctionalComponent(Component) {
  return Component && ('function' === typeof Component) && !(Component.prototype && Component.prototype.render);
}

function getDisplayName(WrappedComponent) {
  return WrappedComponent.displayName || WrappedComponent.name || 'Component';
}

[gaearon]

That brings me to the crux of the issue though: if a component errors, I'm not sure that there is a great way to recover or roll back. Any way you go about it, an error is going to produce incorrect UI state that was not expected.

I don’t agree.

That’s why in React we use the component tree as a way to “route” the error to the closest “boundary”. It’s like a try / catch: errors bubble up until they find the closest boundary, which can then render an error UI instead. This keeps the UI consistent.

We use this extensively at Facebook to guard individual high-risk widgets as opposed to taking down an entire page or rendering a UI with “holes” like in the approaches mentioned above.

You can learn more about error boundaries in React from this blog post.