-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Error boundaries: Recover from errors thrown in render
#2461
Comments
There are quite a few places where you can't throw exceptions without React ending up in an invalid state. If you can't trust your developers then the only solution I know of is to put a try/catch in each render. |
Ah thanks :) I mean it's not a matter of trust it's to prevent using 3rd-party React component and whatnot. Do we plan to have more support for error handling? |
@longlho Not my area, but I believe the two biggest blockers are; many browsers ruin the error/call-stack when rethrowing and try-catch incurs performance penalities and these are hot paths. |
Yeah, you may hear conversation about "error boundaries" which means making a way to handle this sort of thing more gracefully and something we want to find a good solution to. I don't think we have a tracking issue for it though so let's use this one. |
render
functionrender
Ah thanks guys :) this does sound like a big issue but definitely important. Given that react is stateful wouldn't it require some sort of sandbox for rendering to make sure everything's cool, then apply the new state I guess? Is there any active effort on this front? |
#3313 has a really nice unit test that we should probably start using once the error boundaries issue is fixed. |
👍 |
1 similar comment
+1 |
+1 This makes debugging React apps much more difficult than it has to be! |
+1 It's very scary that one child component error can take an entire component tree down. It would be awesome to put try/catch statements at some key components in that tree. |
I am working on an isomorphic app with many components, and we lean on very messy third-party APIs. So there is a relatively high risk that a developer can write a new component that makes bad assumptions about the data it receives (since we render server side, this destroys the request and the user cannot get any part of the page) We decided to take an aggressive approach to the problem by wrapping React.createElement in our app and replacing the lifecycle methods with our own error handling. For ES6 we were thinking of doing the same thing by extending the Component class Even though this leaves the app in an invalid state we thought it was preferable to rendering nothing for the user. Here is a link to the code I am working on: Is there some reason why I really should not do this? |
@skiano If that's working for you, no reason to stop. Our eventual solution should be a little more flexible than this and more efficient but will be similar in spirit. |
+1 |
Any idea what the time-frame might be for an official solution? Just curious. :) In the mean-time I will probably just monkey-patch everything. |
If a component's render function throws an error, React cannot recover. This causes subsequent hot reloads to fail. Now we catch these errors during hot reloading so things are moar better.
+1 |
+1 |
1 similar comment
👍 |
@spicyj Should we label this as |
I'm willing to but it doesn't require much discussion, it just needs to get done. |
I'd be glad to help out with this. It's a pain point for my team. |
Hi, first of all the test case in the first post works only within certain app context. Here's the working test case http://jsfiddle.net/XeeD/pLqphjm4/. There is an already mounted component, that will throw in We thought about a few approaches by catching it here in
However we are not sure about the correct way how to handle this. Feedback? Working on PR atm. |
I've not voted on it myself, but I wouldn't use it since it changes which method all the derived classes need to override to render, so special-case knowledge about how error handling in the base class works propagates to affect all React components. The current workaround I'm using is to wrap the render method in the base class in the constructor by calling
Very much looking forward to v16 with proper error boundaries! |
We've used the HOWEVER, the More details here: https://engineering.classdojo.com/blog/2016/12/10/catching-react-errors/ |
Small update on this: latest alphas of React ( The recommended way to use them is to create a component like this: class ErrorBoundary extends React.Component {
state = { error: null };
unstable_handleError(error, info) {
this.setState({ error });
// Note: you can send error and info to analytics if you want too
}
render() {
if (this.state.error) {
// You can render anything you want here,
// but note that if you try to render the original children
// and they throw again, the error boundary will no longer catch that.
return <h1>Something went wrong: {this.state.error.toString()}.</h1>;
}
return this.props.children;
}
} Then you can put it anywhere: <ErrorBoundary>
<MyApp />
</ErrorBoundary> or to protect specific components: <Layout
left={<Sidebar />}
right={<ErrorBoundary><RouteHandler /></ErrorBoundary>}
/> Note that The exact API may change in 16. |
@slorber If error boundaries don't work in latest alpha for some reason please create a complete reproducing example. |
I'm imagining something like that: ReactDOM.safeRender(
<App />,
document.getElementById('root'),
(err, componentInstance) => {
// If component fails then report the incident and render nothing
reportError(err, componentInstance)
return null
}
) ...or at least on a class level, so that we could use inheritance. class SafeComponent extends Component {
catchRender (err) {
reportError(err, this)
return null
}
}
class Whatever extends SafeComponent {
render () {
throw 'whoops'
return <div>A small cosmetic detail</div>
}
} |
There is no support for doing this at individual component level (randomly disappearing individual components isn't a great user experience). You are, however, free to wrap all your components with an error boundary, or to create a base class (although we generally don't recommend that).
Why a whole app/page? You can divide your page into individual sections and wrap each of those with error boundary. The granularity is up to you. |
One question I have is with hmr, how to reset that error. I guess we'd need
to store the boundaries that saved errors in the react proxy logic. Or
perhaps a way to bubble up when a child has force updated. Anyway just
thinking out loud, excited to give it a try.
…On Mon, Jun 26, 2017 at 8:31 AM Dan Abramov ***@***.***> wrote:
There is no support for doing this at individual component level (randomly
disappearing individual components isn't a great user experience). You are,
however, free to wrap all your components with an error boundary, or to
create a base class (although we generally don't recommend that).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2461 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAvRKJviviZS6uF4bMjiWHOLHugL2Baks5sH87pgaJpZM4C2nLz>
.
|
That warrants a separate discussion (it's not immediately clear to me how hot reloading should work with error handling). We might be looking at hot reloading some time this year, and this is a good time to discuss the ideal experience. |
I ended up implementing export default function ErrorBoundary(WrappedComponent) {
class ErrorBoundaryComponent extends React.Component {
state = { error: null };
unstable_handleError(error, info) {
console.error(error, info);
this.setState({ error });
// Note: you can send error and info to analytics if you want too
}
render() {
if (this.state.error) {
return null;
}
return <WrappedComponent {...this.props} />;
}
};
return ErrorBoundaryComponent;
} This solution is considerably better than extending a base component because it can wrap functional components function FunctionalComponent(props) {
...
}
export default ErrorBoundary(FunctionalComponent); and catch errors that occur in the component's other wrappers, such as a selector in redux's export default ErrorBoundary(connect(mapStateToProps, mapDispatchToProps)(SomeComponent)); |
@joshclowater What is the benefit as opposed to using a regular component? <ErrorBoundary>
<Something />
</ErrorBoundary> This gives you more freedom with granularity. |
@gaearon Personal preference I guess. I find it slightly cleaner as it abstracts the error handling out of the JSX. If I'm wrapping many components, it could noisy to parse through all of the <ErrorBoundary>
<Something />
</ErrorBoundary>
<ErrorBoundary>
<Something />
</ErrorBoundary>
...
<Something /> // Oops missed that one |
Every time I get an error in a
|
Perhaps concern about the try-catch performance penalty will soon be obsolete: https://stackoverflow.com/a/19728876/200224 Not only does one comment mention that recent versions of V8 can optimize functions containing try-catch blocks, that answer explains that most of the performance penalty can be avoided by moving the try-catch into a wrapper function so that the main function which could throw can be optimized. From Petka Antonov's optimization killers guide:
var errorObject = {value: null};
function tryCatch(fn, ctx, args) {
try {
return fn.apply(ctx, args);
}
catch(e) {
errorObject.value = e;
return errorObject;
}
}
var result = tryCatch(mightThrow, void 0, [1,2,3]);
//Unambiguously tells whether the call threw
if(result === errorObject) {
var error = errorObject.value;
}
else {
//result is the returned value
} |
Yes, and this is exactly what we’re doing in React 16. 😉
Please read my comment above: #2461 (comment). This is pretty much what we’ve added. |
@gaearon oh, I see. That will be nice, though I assume I will probably end up using |
...nevermind, I thought |
Error boundaries don't have to be at the root, as I mentioned before. You can put them in a few strategic places (e.g. route handler wrapper, and important components big enough to show a useful box). I don't think it's a good idea to show them in place of every individual components. Components can be offscreen, or too little to show the box. In fact in DEV I'd probably always want to see a fullscreen error. We still haven't decided what's the default way to show error boxes in React 16. @bvaughn was prototyping this at some point. We'll get back to it before the release. |
👍 I look forward to exploring this a bit more, hopefully soon |
Is |
No. We settled on |
@gaearon |
It catches for the whole subtree (but not for the component itself). |
Catching errors in child components is now supported. You can read more about this feature in our new blog post: Error Handling in React 16. I think we can close this issue. It is likely the feature will evolve over time, and we’d love your feedback, but let’s discuss it in more focused future issues. |
So I'm trying to put some graceful error handling in case 1 of my views crap out:
However,
MyGoodView
does not get rendered w/ the following stack trace:Seems like error throw React in a bad state where
renderedComponent
isundefined
, thus cannot be unmounted. How do I handle this scenario?The text was updated successfully, but these errors were encountered: