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

componentDidCatch is not getting called when there is an error in promise #11334

Closed
john1jan opened this issue Oct 23, 2017 · 13 comments
Closed

componentDidCatch is not getting called when there is an error in promise #11334

john1jan opened this issue Oct 23, 2017 · 13 comments

Comments

@john1jan
Copy link

@john1jan john1jan commented Oct 23, 2017

As per the new react 16 release doc it says

"React 16 prints all errors that occurred during rendering to the console in development, even if the application accidentally swallows them."

I have a Parent component and a Child component. I have triggered an error in then block of promise. But it will call catch method of the promise, componentDidCatch of parent is not getting called. I am not sure whether this the expected behaviour.

Here is the jsfiddle https://jsfiddle.net/john1jan/Luktwrdm/14/

@gaearon

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Oct 23, 2017

As it says:

Error boundaries catch errors during rendering, in lifecycle methods, and in constructors of the whole tree below them.

What you're showing is an error in a Promise callback. You happened to fire the thing that called it from componentDidMount, but the error doesn't technically happen inside componentDidMount so error boundaries won't catch it.

Error boundaries are a mechanism to prevent React from getting into an inconsistent state when it doesn't know what to render due to an error. This is not the case in your example: even despite the error, the rendering is unaffected.

Here is an example where a Promise does lead to re-rendering, which throws and then gets swallowed. You can see that in that case, error boundary receives the error: https://jsfiddle.net/t37cutyk/

So it is working as designed.

@gaearon gaearon closed this Oct 23, 2017
@john1jan

This comment has been minimized.

Copy link
Author

@john1jan john1jan commented Oct 26, 2017

Thanks, @gaearon for explaining. But the doc is little confusing. It would be great if you can give an example of how an application can accidentally swallow an error.

In the https://jsfiddle.net/t37cutyk/, I dont think the child component is swallowing the error.

@gaearon

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Oct 26, 2017

I am referring to this block:

fetch("https://jsonplaceholder.typicode.com/posts").then((response) => {
      this.setState({posts: []});
    }).catch((error)=>{
    	console.log("Error caught in catch",error);
    })
    ;

If you leave the catch() clause empty and not log the error (effectively swallowing it), React will still report it as unhandled to the browser, and pass it to the error boundary. Even though you "caught" it. This is intentionally because people have been accidentally swallowing errors like this for years, and we got a few dozens different issues about this over time.

@john1jan

This comment has been minimized.

Copy link
Author

@john1jan john1jan commented Oct 30, 2017

Thanks again @gaearon. https://jsfiddle.net/john1jan/ed53v0m2/1/
I have removed catch block as you said. And even I have removed error boundary from Parent component. Even though the child components throws that error without swallowing it and does not render. So when the whole component tree is not rendered there is a possibility that developer will notice this at the time of development or testing.

Just correct me if I am wrong. I think "Swalling an error" means "even if the error is thrown in the child component it is rendered"

@gaearon

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Oct 30, 2017

I’m not sure what you’re asking. Yes, it is expected that if an error is thrown during rendering, the tree is unmounted unless you created an error boundary.

“Swallowing” an error means using a catch block and then ignoring the error object. This makes it impossible for developers to figure out what went wrong. What I’m saying is that React now protects against this by printing the error to the console even if it was caught.

@dudko

This comment has been minimized.

Copy link

@dudko dudko commented Jan 25, 2018

@gaearon, your fiddle does not define state of child component and crashes before fetch is resolved. Fixed example makes your explanation more obvious. Anyway, thanks a lot for it!

@AlicanC

This comment has been minimized.

Copy link
Contributor

@AlicanC AlicanC commented Sep 11, 2018

@gaearon, you have closed this issue, but isn't this a great case for Zones? Lifecycle methods could be ran in a Zone and the error handling mechanism (domenic/zones#9) could be used to catch async errors.

@AlicanC

This comment has been minimized.

Copy link
Contributor

@AlicanC AlicanC commented Sep 11, 2018

And meanwhile an API (like this.throw) can be exposed so we can:

componentDidMount() {
  doAsync()
    .then(() => ...)
    .catch((error) => this.throw(error));
}

Would React be capable of handling asynchronously sent errors?

@ViggoV

This comment has been minimized.

Copy link

@ViggoV ViggoV commented Sep 21, 2018

Would it be bad practice to use the same error boundary component for uncaught errors, like fetch and event handlers, by setting the state and then throwing an error in one of the lifecycle methods based on that?

class MyBuggyComponent extends React.Component {
  constructor(props) {
    super(props);
    this.state = { error: null };
  }

  handleClick() {
    try {
      badStuff();
    } catch(err) {
      this.setState({ error: err.message });
    }
  }

  render() {
    if (this.state.error) throw new Error(this.state.error);
    return (
      <div onClick={this.handleClick}>CLICK TO FAIL</div>
    );
  }
}
@AlicanC

This comment has been minimized.

Copy link
Contributor

@AlicanC AlicanC commented Sep 22, 2018

If you want a workaround, this could work too:

  handleClick() {
    try {
      badStuff();
    } catch(err) {
      this.setState(() => { throw err; });
    }
  }
@joepuzzo

This comment has been minimized.

Copy link

@joepuzzo joepuzzo commented Oct 10, 2018

A better way to handle this can be done with a custom error handler. See this code sandbox

https://codesandbox.io/s/mopl2vzm18

Look at the line that has <MyRequest url="https://jsonplaceholder.typicode.com/todos/1">

Try the scenario shown, then try chaning that line to this:
<MyRequest url="https://jsonplaceholder.typicode.co">

Then this:
<MyRequest url="https://jsonplaceholder.typicode.com/todos/1/asdf">

You will see how depending on the status code you can handle errrors at the comp level via props OR have the error boundary handle them! In this case, the error boundary will handle 404 cases only

The code could be cleaned up a little but the pattern/idea is solid

@ericvicenti

This comment has been minimized.

Copy link

@ericvicenti ericvicenti commented Feb 1, 2019

if you're using hooks, I think the following should work nicely with error boundaries:

function MyComponent({ input }) {
  const [asyncResult, setAsyncResult] = useState(null);
  if (asyncResult.error) {
    throw new Error('Catch me, parent!');
  }
  useEffect(() => {
    asyncThing(input)
      .then(result => setAsyncResult({ result })
      .catch(error => setAsyncResult({ error });
  }, [ input ]);
  return <SomeView thing={asyncResult && asyncResult.result} />;

Of course this is boilerplatey, so you'd probably want this behavior packaged in something like useAsyncResult(asyncCallback, inputs), which would be used like this:

function MyComponent({ input }) {
  const thing = useAsyncResult(async () => {
    // other async logic could go here
    return await asyncThing(input);
  }, [ input ]);
  return <SomeView thing={thing} />;
}

PS. I haven't tested this yet, but I'm off to do that now

@technion

This comment has been minimized.

Copy link

@technion technion commented Mar 2, 2019

@ericvicenti Thanks for that tip - I've followed that pattern and it works well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.