Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: don't re-throw errors caught by boundaries (in development) #19613

Closed
OliverJAsh opened this issue Aug 14, 2020 · 4 comments
Closed

Comments

@OliverJAsh
Copy link

OliverJAsh commented Aug 14, 2020

We are using error boundaries to catch and handle errors inside of a React render. As for errors which occur outside of a React render (i.e. uncaught errors that happen in a timeout), we need to register an uncaught error handler (window.addEventListener('error', …)) to handle these separately (otherwise the user will not be alerted).

Unfortunately we can only rely on this uncaught error event handler in production, not development. This is because—in development—errors caught by error boundaries will still bubble up to the uncaught error handler. (Unfortunately it's not documented but it's explained here: #12897 (comment). PR to add this to the docs: reactjs/react.dev#2168.)

This means that (in development) the uncaught error handler will be called for errors which are actually caught by error boundaries.

Intuitively I expect boundaries to work like try...catch—if an error is caught, it should never bubble up to the uncaught error handler. Also, I do not expect such a big difference in the way errors are handled in development versus production.

For the same reason, when using the Fast Refresh webpack plugin in development, the error overlay still appears even when an error is caught by an error boundary, because they are using an uncaught error handler to detect when to show the overlay. I would not expect this to happen because the error overlay is traditionally for uncaught errors.

Why does React need to treat errors differently in development? Is there a reason it needs to do this?

Whatever the reason, I think it's quite important for errors to behave consistently in development and production. When we're working on the user experience around errors in development, it's not obvious that the behaviour of errors could be significantly different in production. Furthermore, it's difficult to test error boundaries in development if an uncaught error handler has also been registered. For example, if Fast Refresh is used, the error overlay will appear over the top of the error boundary.

For this reason, I'm keen to explore if there is a different approach we can take here, in order to make the behaviour consistent between development and production. Ideally error boundaries would just behave similarly to try...catch.

Failing that, is there any other way we can differentiate between "error caught by boundary" and "uncaught error"—which works in production and development?

To demonstrate the problem, here is a reduced test case which:

  • throws an error both inside and outside of React render
  • registers a uncaught error handler and logs events
  • uses an error boundary and logs calls to componentDidCatch and getDerivedStateFromError

From the log screenshots I've included at the bottom, we can see that in development the uncaught error handler is called for the error which is in-fact caught by an error boundary.

src/index.jsx:

import * as React from 'react';
import * as ReactDOM from 'react-dom';

window.addEventListener('error', (event) => {
    console.log('window.addEventListener error', event);
});

class ErrorBoundary extends React.Component {
    state = { hasError: false };

    static getDerivedStateFromError(error) {
        console.log('getDerivedStateFromError', { error });

        return { hasError: true };
    }

    componentDidCatch(error, errorInfo) {
        console.log('componentDidCatch', { error, errorInfo });
    }

    render() {
        return this.state.hasError === false && this.props.children;
    }
}

const Boom = () => {
    console.log('Boom');

    throw new Error('Error inside React render');

    return null;
};

const App = () => {
    console.log('App render');

    return (
        <ErrorBoundary>
            <Boom />
        </ErrorBoundary>
    );
};

const rootEl = document.getElementById('root');

ReactDOM.render(<App />, rootEl);

setTimeout(() => {
    throw new Error('Error outside of React render');
}, 5000);

Development

image

Production

image


Related: https://stackoverflow.com/questions/57197936/componentdidcatch-window-addeventlistenererror-cb-dont-behave-as-expecte/57200935

@OliverJAsh OliverJAsh added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Aug 14, 2020
@OliverJAsh OliverJAsh changed the title How to reliably detect *uncaught* errors in development *and* production? How to reliably detect *uncaught* errors in production *and* development? Aug 14, 2020
@proamg9
Copy link

proamg9 commented Aug 18, 2020

Isn't this "by design", intentionally, by the react dev team, not giving too much info to the public user?

@OliverJAsh OliverJAsh changed the title How to reliably detect *uncaught* errors in production *and* development? Suggestion: don't re-throw errors caught by boundaries (in development) Nov 14, 2020
@kmurgic
Copy link

kmurgic commented Nov 25, 2020

This looks like it might be a duplicate of #10474.

@eps1lon
Copy link
Collaborator

eps1lon commented Dec 2, 2020

This looks like it might be a duplicate of #10474.

I agree. Closing in favor of #10474. @OliverJAsh if you think #10474 is not talking about the same problem could you explain what the difference between these two issues is?

@eps1lon eps1lon closed this as completed Dec 2, 2020
@eps1lon eps1lon added Resolution: Duplicate and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Dec 2, 2020
@OliverJAsh
Copy link
Author

OliverJAsh commented Sep 5, 2023

The behaviour has changed slightly after upgrading to React 18 + createRoot. I've pushed a new commit to the reduced test case.

When a component throws, it seems that React will now try to re-render that component meaning it may throw again. This means that—in development—the error will be logged multiple times by the global event handler.

Development

image

Production

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants