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

Workaround for dealing with exceptions thrown in asynchronous render method #5615

Closed
wants to merge 1 commit into from

Conversation

rppc
Copy link

@rppc rppc commented Dec 6, 2015

We provide a workaround for this issue by allowing the user to define a callback function to be executed whenever an exception is thrown inside the body of a render method. This function should be called exceptionCallBack, as in the following example, adapted from this issue:

var a = false;

var MyComponent = React.createClass({
    render: function() {
        if (this.props.a) {
            throw 'err';
        }

        return (      
            <h1 onClick={ () => {a = true; run()} }>Hello {this.props.name}</h1>      
        );
    },
    exceptionCallBack: function(e) {
        console.log(e);
    }   
});

function run() {
    ReactDOM.render(
        <MyComponent a={a} name={"Rui Cardoso"} />,
        document.getElementById('example')
    );
}

ReactDOM.render(<MyComponent a={false} name={"Rui Cardoso"} />, document.getElementById('example'));

This produces the expected result of err being logged to the console when the component is clicked.

@jimfb
Copy link
Contributor

jimfb commented Dec 8, 2015

Thanks for the PR! It's a good attempt, but error boundaries are a little more complicated than this to get right.

This does a try-catch for every render of every component. In V8, a try-catch is a slowpath operation that causes deoptimization, and thus should be avoided for performance reasons. This also only works if the component that crashed during render has an exceptionCallback defined, but the common case is that the component that wants to handle the error is a parent of the component that crashed (otherwise, why would your component define exceptionCallback instead of just fixing the component in the first place). For these reasons, we're not likely to take this PR, so I'm going to close it out in favor of something like #5602. But the attempt is appreciated!

If you're looking for a way to contribute, I would recommend taking a peak at https://github.com/facebook/react/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+bug%22

@jimfb jimfb closed this Dec 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants