facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.59k stars 46.79k forks source link

Throwing Error from hook not caught in error boundary #14981

Closed joepuzzo closed 5 years ago

joepuzzo commented 5 years ago

This may be more of a question than an issue but is it possible to throw errors in a hook and have them caught in an error boundary? Or will this not work?

gaearon commented 5 years ago

What have you tried?

joepuzzo commented 5 years ago

So basically I created a hook that wraps apollos client.. and I tried to throw from that hook. See code below: Note the commented line where i throw

import React, { useState, useContext, useEffect } from 'react';
import { ApolloClientContext } from '../../context/context';

const useQuery = ({ query, variables }) => {

  const client = useContext(ApolloClientContext);

  const [loading, setLoading] = useState(true);
  const [data, setData] = useState();
  const [error, setError] = useState();

  const fetchData = async (params) => {
    setLoading(true);
    setError(undefined);
    try {
      const { data: result } = await client.query(params);
      setData(result);
    }
    catch (e) {
      // THIS IS THE IMPORTANT LINE
      if( e.type === 'some type where i want to throw') { 
          throw e 
      }
      setError(e);
    }
    setLoading(false);
  };

  useEffect( () => {
     console.log('FETCHING DATA');
     fetchData({
       query,
       variables
     });
  }, []);

  return {
    loading,
    data,
    error,
    query: fetchData
  };
};

export default useQuery;

I know its throwing. And I also know that I have an error boundary wrapping it.

joepuzzo commented 5 years ago

Here is intended use case:

import React, { useState } from 'react';
import { useQuery } from '../../hooks';

const ComponentWithHook = ({
  foo
}) => {

  const {
    loading,
    data,
    error
  } = useQuery({
    query: SOME_QUERY,
    variables: { foo },
  });

  return (
    <SomeViewComponent 
      loading={loading}
      data={data}
      error={error}
    />
  );
};

class MyErrorBoundary extends React.Component {

  constructor(props) {
    super(props);
    this.state = {};
  }

  componentDidCatch(error, errorInfo) {
    this.setState({
      error
    });
  }

  render() {

    return (
      {error : <small>Ahh there was an error</small> : <ComponentWithHook foo="bar" />}
    );
  }
}
PierreAndreis commented 5 years ago

@joepuzzo It seems to me that error boundaries do catch errors thrown inside hook https://codesandbox.io/s/24l03vz15j?fontsize=14

joepuzzo commented 5 years ago

Mine is async... could that be the issue?

joepuzzo commented 5 years ago

Oh wait so is yours

PierreAndreis commented 5 years ago

@joepuzzo I see. The issue here maybe is that you are not awaiting this code inside the hook, so react moves on before it should. (please someone correct me if i'm wrong)

  useEffect( () => {
     console.log('FETCHING DATA');
    // this should await here so useEffect awaits for the resolution of the promise
     fetchData({
       query,
       variables
     });
  }, []);

I was able to replicate by not awaiting and throwing the error, which error boundary didn't catch. As soon as I await the function inside useEffect, then error boundary caught it

joepuzzo commented 5 years ago

You're right.. im not awaiting.. however when i add that, react barfs at me saying "Warning: An Effect function must not return anything besides a function, which is used for clean-up."

joepuzzo commented 5 years ago

So I'm assuming this is bad behavior then? And there is a better approach.

PierreAndreis commented 5 years ago

Now i don't know if error boundaries not catching errors that happens inside async function called from useEffect is by design or not. It makes sense from what I can tell

gaearon commented 5 years ago

React can’t catch them — just like it wouldn’t catch errors from async class methods.

You can, however, do a trick like this:

setState(() => {
  throw new Error('hi')
})

That should do what you want, both in classes and with Hooks.

joepuzzo commented 5 years ago

hmm sneaky but I like it lol

iwilson-r7 commented 5 years ago

@gaearon The combination of that approach, but using the preferred getDerivedStateFromError instead of componentDidCatch and rendering a child component when the error boundary is triggered causes Uncaught Invariant Violation: Rendered fewer hooks than expected. This may be caused by an accidental early return statement. instead of rendering the fallback UI

See the modified code sandbox: https://codesandbox.io/s/2wwj41v7on

Is there something unsupported about this approach?

Swapping getDerivedStateFromError back to componentDidCatch fixes it, so it seems like a bug

danielkcz commented 5 years ago

@iwilson-r7 Just encountered the same issue. Interestingly enough, it works correctly in a production build without any hiccups. But it is certainly annoying in development as making a mistake basically throws away everything, not even dev error overlay is shown, just console message which is not helpful at all.

zeg-io commented 5 years ago

https://reactjs.org/blog/2017/07/26/error-handling-in-react-16.html

Error Boundaries aren't supported in React Hooks yet :(

techieshark commented 5 years ago

In case anyone lands here using hooks, here's what I'm using, since we don't have setState. If folks know a better way to propagate the error I'd love to know.

import React, { useReducer } from "react";

const initialState = {};

function reducer(state, action) {
  if (action.type === 'error' && action.error) {
    throw action.error;
  }
}

function Component() {
  const [state, dispatch] = useReducer(reducer, initialState);

  useEffect(() => {
      memoizedCallAPI().then((response) => {
        // handle good response
      }).catch(error => {
        dispatch({ type: 'error', error });
      });
  }

Update

Actually we can just do this:

  const [/* state */, setState] = useState();
  setState(() => {
    throw error;
  })
timothysantos commented 5 years ago

Hi @techieshark , what do you mean by your update? Are you replacing useReducer to useState to throw errors?

techieshark commented 5 years ago

@timothysantos originally i incorrectly thought I could only use the useReducer, but later realized useState works as well. Both seem to work, but useState alone seemed simpler if the reducer / dispatch functionality is not also needed.

So in the end, with hooks, my full code looked like:

function Component() {
  const [/* state */, setState] = useState();

  useEffect(() => {
      memoizedCallAPI().then((response) => {
        // handle good response
      }).catch(error => {
        setState(() => {
         throw error;
         })
      });
  }

I hope that clarifies?

bernardbaker commented 4 years ago

Does anyone know how to stop the error stack showing up in the UI?

@techieshark @timothysantos

backus commented 3 years ago

This wont suit everyone's needs since this only works for ErrorBoundaries sitting at the top of your component hierarchy, but if you want to make sure your error boundary handles uncaught promise errors, you can use window.addEventListener("unhandledrejection") like so:

import React from "react";

export class TopLevelErrorBoundary extends React.Component<{}, { hasError: boolean }> {
  constructor(props: {}) {
    super(props);
    this.state = { hasError: false };
  }

  static getDerivedStateFromError(_error: any) {
    return { hasError: true };
  }

  componentDidMount() {
    window.addEventListener("unhandledrejection", this.onUnhandledRejection);
  }

  componentWillUnmount() {
    window.removeEventListener("unhandledrejection", this.onUnhandledRejection);
  }

  onUnhandledRejection = (event: PromiseRejectionEvent) => {
    event.promise.catch((error) => {
      this.setState(TopLevelErrorBoundary.getDerivedStateFromError(error));
    });
  };

  componentDidCatch(error: any, errorInfo: any) {
    console.log("Unexpected error occurred!", error, errorInfo);
  }

  render() {
    if (this.state.hasError) {
      return (
        <YourErrorViewHere />
      );
    }

    return this.props.children;
  }
}

Also I realize this is an old issue so sorry for noise of resurfacing. This is just one of the top results on Google now, so figured I would chime in 😅

kwantdev commented 2 years ago

It would be great if somebody would explain why throwing inside a setState method allows ErrorBoundary catches the exception. Thanks!

nevcos commented 1 year ago

It would be great if React provided a clear API to catch an error in async scenarios. Being required to use useState for this is the opposite of clear and semantic code...

nikkwong commented 9 months ago

A huge problem with the

setState(() => throw error)

approach is that it doesn't stop the execution of the component. So if you have something like this:

let data = null;
try {
  data = await fetch(...);
} catch (e) {
  setError(() => {
    throw e;
  });
}
console.log("data", data);

The last line (console logging) will still execute, which is problematic for my use case (involving useSWR), where there really is no workaround. Can we get first class support for this case rather than this seemingly hacky workaround?