[ErrorBoundary] New ErrorBoundary Component

[dimitropoulos]

This ticket is to track ideas/rationale for material-ui including an ErrorBoundary component.

I would like to contribute this component to material-ui, as described below.

• I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

ErrorBoundaries are a React 16+ specific feature that uses the componentDidCatch API for handling uncaught errors without unmounting the whole React component tree.

This ticket exists to request feedback and ideas for a material-ui-provided ErrorBoundary component.

Examples 🌈

The react-ui-roundup tracks ErrorBoundaries, and it appears that there are just 3 implementations:

For convenience, I've linked to the actual code for these three here:

• Elastic's code: This one is the simplest (and, best looking, in my opinion). No props, just catches the error as you might expect.
• AntD's code: This is implemented by adapting the Alert component (I do not think we should do this in this project, btw). It therefore accepts a title and description prop (both strings), but not for passing in a custom fallback component, which is a bummer.
• Carbon Design's code: This one, although the storybook example they have is a bit unfortunate (it doesn't even show an error or callstack..???), it's actually the most flexible of the bunch. It allows passing in a fallback component via a fallback prop. The only downside here is that the actual error and componentStack are not accessible because they are not passed in to the fallback component.

So basically, I'm suggesting a mashup of the above 3, the looks of Eui, the simple string api of AntD, the flexible fallback (if you really need or want it), and two more little additions (they're related).

I have done a few of these ErrorBoundary components for various projects in the past and one thing I ALWAYS end up needing is a little section at the bottom of the callstack where I can tell the user something like:

Oops! Something went wrong. Please contact support at 1-800-867-5309

Or (better) sometimes it's appropriate to have a button that the user can click to contact support that will then open up something like Intercom, will start an email with some useful information pre-populated, or will send an error report dialog with something like getSentry.

Speaking of which, mature codebases will often be tracking errors of this sort with something like getSentry, and the onCatch prop exists as a simple callback that will allow the user to trigger some analytics (or other change) if an error state is to occur.

I propose the following API:

import { ReactNode, ErrorInfo } from 'react';

interface ErrorBoundaryProps {
  /**
   * This inserts a section below the call stack (or fallback) but within the ErrorBoundary that you can use to provide the users with more info (such as a support phone number to call) or
   */
  actions?: ReactNode;
  
  /**
   * Can be overridden by providing a `fallback` prop.
   */
  description?: string | ((componentStack: string) => string);

  /**
   * forces the ErrorBoundary to render when debug is `true`
   */
  debug?: boolean;

  /**
   * This matches the arguments of `componentDidCatch`.
   * If provided, overrides `title` and `description` props by occupying the space where the title and description are.
  */
  renderFallback?: (error: Error, errorInfo: ErrorInfo) => ReactNode;

  /**
   * This matches the arguments of `componentDidCatch`.
   * This callback is useful for enabling sending error information to analytics.
   */
  onCatch?: (error: Error, errorInfo: ErrorInfo) => void;

  /**
   * Provide a string to override, or a function that receives the name of the error
   */
  title?: string | ((errorTitle: string) => string);
}

const ErrorBoundary: FC<ErrorBoundary> = props => { ... }

As far as styling goes, I think Elastic's looks best. Perhaps we can just play around with material-ui internals and quickly find something suitable that works in both light and dark themes.

Motivation 🔦

this was mentioned by @oliviertassinari #19744 (comment) in a comment about improving user experience.

[oliviertassinari]

@dimitropoulos Thanks for opening this issue! :) If I understand the motivation correctly, the component is about improving the user experience in production? It sounds great.

On the execution of the idea side of things. A few thoughts, at first sight, but happy to be proven otherwise:

1. We shouldn't display the error stack by default but still allow developers to handle it.
2. Will need some UI for displaying the error, the Alert sounds like a relatively good component we have available for this, we could also consider a minimalist default design. Curious to benchmark what Google's products are doing.
3. The concept of context Carbon has could be replaced with our theme.props.MuiErrorBoundary feature. Only one context to manage, same power => simpler.
4. Great comment on Sentry/Bugsnap, etc. I think that we will need to mention the big point of care in the documentation: https://docs.sentry.io/platforms/javascript/react/#error-boundaries. Missing these errors can be extremely frustrating.
5. We should disable the component if process.env.NODE_ENV !== 'production', with a debug flag to force it in dev mode for working on it. If we run the component in development, we risk breaking the nice DX of react-error-overlay (a dependency used in Next.js, CRA, and Gatsby)?

I propose the following API:

• I guess there is also a children prop? :)
• Maybe I would rename fallback -> renderFallback. Also, the description doesn't seem to match.
• What about a debug boolean flag?

[dimitropoulos]

1. We shouldn't display the error stack by default but still allow developers to handle it.

Could you please say a little more as to why? In the above implementation setting description prop to be always empty solves this aim description=""

2. Will need some UI for displaying the error, the Alert sounds like a relatively good component we have available for this, we could also consider a minimalist default design. Curious to benchmark what Google's products are doing.

I share that curiosity. What I meant is that this is (from a DOM structure perspective) relatively simple component and I think that tying it to the Alert component will only complicate matters. If you feel strongly, by all means, I am happy to give using Alert a try! :)

3. The concept of context Carbon has could be replaced with our theme.props.MuiErrorBoundary feature. Only one context to manage, same power => simpler.

I don't have a lot of background on what you're talking about: could you please point me in the right direction?

4. Great comment on Sentry/Bugsnap, etc. I think that we will need to mention the big point of care in the documentation: https://docs.sentry.io/platforms/javascript/react/#error-boundaries. Missing these errors can be extremely frustrating.

yep! this could be one of the big innovations by material-ui's implementation, and, after all, it'd only be like 4 lines of code!

5. We should disable the component if process.env.NODE_ENV !== 'production', with a debug flag to force it in dev mode for working on it. If we run the component in development, we risk breaking the nice DX of react-error-overlay (a dependency used in Next.js, CRA, and Gatsby)?

hmmmmm.... yeah... I see what you mean. I see it both ways (and, I think you can too). Sometimes when you're developing you want to just forge ahead past a few (temporary) errors. Other times, though, you really really want to see the app exactly as the users will see it (but without losing all the other benefits of things that exist in dev mode like better hot reloading and build times, etc.). I can agree that we can start out with a debug prop (I'm assuming that's what you meant by "debug flag"

I guess there is also a children prop? :)

For sure, yeah, Component<ErrorBoundaryProps> would be the instantiated API. I updated the description so that that's clear.

Maybe I would rename fallback -> renderFallback. Also, the description doesn't seem to match.

Good call. I lifted the terminology from the Carbon. This popular error boundary component uses FallbackComponent (capital F??). I think renderFallback is an improvement, but maybe fallbackComponent is better? I'm fine with either: what do you think?

What about a debug boolean flag?

See above, but I've added it to the description

[oliviertassinari]

Could you please say a little more as to why?

As a end-user (not as a developer), I don't want to see the stack trace of an error, it's noise.
I'm interested in 1. getting the feedback that there is an error (not always the case on products I'm using, even for Uber). 2. having actionable information regarding what to do next.

I think renderFallback is an improvement, but maybe fallbackComponent is better?

We could accept a React component. The usage of ReactNode made me think that you were more interested in a React element. Which one would be best? (the answer will resolve the naming question).

No objection with the other comments.

[dimitropoulos]

As a end-user (not as a developer), I don't want to see the stack trace of an error, it's noise.

Ok. I see what you mean. I think it depends on the audience but you've brought me to reconsider my position and upon reconsideration I now strongly agree with you. Most audiences (end users) will see a stack trace as an active act of aggression, hahaha. The default should be to hide the componetStack.

The usage of ReactNode

Just to put this in front of us while we're talking:

    type ReactText = string | number;
    type ReactChild = ReactElement | ReactText;

    interface ReactNodeArray extends Array<ReactNode> {}
    type ReactFragment = {} | ReactNodeArray;
    type ReactNode = ReactChild | ReactFragment | ReactPortal | boolean | null | undefined;

I picked ReactNode rather than ReactElement because it includes ReactElement (via ReactChild) but also includes ReactText. This means that someone can do renderFallback="foo" and it'll work just the same as a ReactFragment or a ReactNodeArray.

[dimitropoulos]

Let me know, though, if you think you have enough interest for me to start on a proof of concept. I'm trying to plan out my next few nights/weekends (which is when I could work on this) and I'd love to pencil this in if it's of interest to you!

[oliviertassinari]

@dimitropoulos From my perspective, using Component in the name of the prop implies React.ElementType under the hood.

I personally think that it's worth exploring, I don't know about what the rest of the team think :). I think that it would be ideal to identify exactly how it will be used in practice.