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

[Fiber] Handle errors in callbacks #8238

Closed
acdlite opened this issue Nov 8, 2016 · 6 comments
Closed

[Fiber] Handle errors in callbacks #8238

acdlite opened this issue Nov 8, 2016 · 6 comments

Comments

@acdlite
Copy link
Collaborator

acdlite commented Nov 8, 2016

During the commit phase (when updates are flushed to the DOM), errors that are thrown inside componentDidMount and componentDidUpdate are trapped so they can be handled later, after the tree has been committed.

An error thrown in a setState callback should be trapped and handled the same way.

Here's the relevant section of code: https://github.com/facebook/react/blob/master/src/renderers/shared/fiber/ReactFiberCommitWork.js#L302-L330

callCallbacks calls all the setState callbacks that are part of the update queue (a linked list of updates). If any of those callbacks throw an error, we should catch the error and continue calling the rest of the callbacks.

This is a good first issue for someone interested in contributing to React/React Fiber. Please read Fiber Principles: Contributing to Fiber before submitting a PR.

@acdlite
Copy link
Collaborator Author

acdlite commented Nov 8, 2016

If anyone wants to claim this, please comment here so everyone knows.

@peterp
Copy link

peterp commented Nov 8, 2016

I wouldn't mind giving it a go, but please don't let that stop anyone else from trying either?

@acdlite
Copy link
Collaborator Author

acdlite commented Nov 8, 2016

@peterp Sure! Just don't want people to work really hard on a PR only to find at the end that someone else has already submitted a fix. Please comment here if/when you're willing to commit to finishing a PR :)

@borisyankov
Copy link
Contributor

I'll have a look at it either. Not afraid to work really hard on it and someone else doing it instead (also known as learning awesome stuff :) )

@moaazsidat
Copy link

Would love to take a look :)

@tychota
Copy link

tychota commented Nov 9, 2016

I can take a look too. Not sure I will ve able to do something but it may helps me to undestand anyway.

ankeetmaini added a commit to ankeetmaini/react that referenced this issue Nov 9, 2016
ankeetmaini added a commit to ankeetmaini/react that referenced this issue Nov 9, 2016
ankeetmaini added a commit to ankeetmaini/react that referenced this issue Nov 9, 2016
ankeetmaini added a commit to ankeetmaini/react that referenced this issue Nov 12, 2016
acusti pushed a commit to brandcast/react that referenced this issue Mar 15, 2017
* Handles risky callbacks on setState. Fixes facebook#8238

* Updates try-catch to cover `callback` when context is not present.

* Updates code to trapErrors instead of swallowing them.

* Fixes flow errors

* Incorporates review comments

* Traps only the first error.

Removes `callbackWasCalled` and updates fiber tests.
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

5 participants