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

Handle thrown exceptions #272

Closed
danielcompton opened this issue Nov 6, 2016 · 5 comments
Closed

Handle thrown exceptions #272

danielcompton opened this issue Nov 6, 2016 · 5 comments
Milestone

Comments

@danielcompton
Copy link
Contributor

@danielcompton danielcompton commented Nov 6, 2016

React doesn't handle exceptions particularly gracefully out of the box. Exceptions thrown during rendering will bubble up and destroy all of the components up to the root, without calling lifecycle methods. facebook/react#2461 is an issue that tracks this.

This exception handling behaviour affects us in re-frame. If app code throws an exception while rendering, then the component is destroyed without calling component-will-unmount, and Reagent doesn't decrement the watch count on the reaction (re-frame subscription). Because the watch count is inaccurate, Reagent never calls the on-dispose, and the subscription stays in the re-frame cache, causing further issues.

It looks like if we provide unstable_handleError as a method on a Reagent component, then the initial mount is called in a try/catch. unstable_handleError is unstable, but this may be worth investigating, either now or later on when React marks it as stable.

@pesterhazy
Copy link
Contributor

@pesterhazy pesterhazy commented Dec 28, 2016

Here's my attempt at creating an "error boundary" using the unstable_handleError API: https://gist.github.com/pesterhazy/d163a8b3f1f1c6a0dac235858776c14b

If used to wrap a root component as explained in the gist, the component gracefully handles exceptions thrown downstream in the component hierarchy.

Let me know if that works for you.

@Deraen Deraen added this to the 0.7.0 milestone May 19, 2017
@Deraen
Copy link
Member

@Deraen Deraen commented May 19, 2017

Would be great to have this improved in the next release.

@pesterhazy
Copy link
Contributor

@pesterhazy pesterhazy commented May 19, 2017

This came up on clojurians the other day. Timothy mentioned that when he tested this recently, Reagent (and React) was able to recover from exceptions thrown in the render functions. I also wasn't able to reproduce the original problem anymore (but didn't try too hard), even without the error boundary set.

I tried to go through React's changlog to see if any recent changes could have fixed this but didn't find anything.

Is it possible that somehow recently this problem has been solved, either on React's or Reagent's side?

If the problem persists ("unable to read property of" or similar errors thrown after fixing an exception in a render method and rerendering), a minimal repro case would be very helpful.

@Deraen Deraen modified the milestones: 0.8.0, 0.7.0 Jun 27, 2017
@danielcompton
Copy link
Contributor Author

@danielcompton danielcompton commented Jul 27, 2017

https://facebook.github.io/react/blog/2017/07/26/error-handling-in-react-16.html is the way this will be handled in React 16. It seems like it should be pretty easy to support in Reagent.

@Deraen
Copy link
Member

@Deraen Deraen commented Oct 20, 2017

Error boundaries are supported with :component-did-cactch lifecycle method: https://github.com/reagent-project/reagent/blob/v0.8.0-alpha2/test/reagenttest/testreagent.cljs#L1003-L1004. Available in 0.8.0-alpha2.

@Deraen Deraen closed this Oct 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants