CharlesStover / fetch-suspense

A React hook compatible with React 16.6's Suspense component.
https://www.npmjs.com/package/fetch-suspense
MIT License
492 stars 25 forks source link

Handle fetch error (network timeout/non-2xx responses) #2

Closed matuscongrady closed 6 years ago

matuscongrady commented 6 years ago

Hello,

I can't figure out how to handle fetch errors with fetch-suspense. I'd like to show an error message to the user if something fails. Could you provide an example, possibly with some comments on it?

Thank you!

quisido commented 6 years ago

Errors should be thrown, so I believe it would be something like this:

try {
  const result = fetchSuspense(etc);
}
catch (e) {

  // Throw the Promise to Suspense
  if (e instanceof Promise) {
    throw e;
  }
  // Error is here as e
}

Unfortunately, throwing Promises is a necessity of Suspense's functionality. I am not a fan of how both errors and promises are handled by being thrown. Depending on your feedback, I may refactor this to be something like: const [ result, error ] = fetchSuspense(etc);. I'm not a fan of having to do const [ result ] = ... instead of const result = ..., but it is arguably worse to have to do try { } catch { throw }.

jimthedev commented 6 years ago
const [ result, error ] = fetchSuspense(etc);

Looks good but presents a big problem in that unhandled errors wouldn't result in anything. Effectively fetchSuspense would suppress errors. It seems like a bad thing and chances are that I'll use fetchSuspense in a custom hook of my own anyhow so catch/throwing isn't a big deal. My app components will useMyAPI() and useMyAPI will use fetchSuspense.

quisido commented 6 years ago

Looks good but presents a big problem in that unhandled errors wouldn't result in anything.

The user would theoretically implement their own error handling. But I suppose at that point,

const [ result, error ] = useFetch(etc);
if (error) {
  throw error;
}

Isn't too different than the try/catch above.

I like this latter format better in a vacuum, but as far as functionality, the current behavior allows Suspense to catch promises and componentDidCatch to catch errors, instead of having to do error handling in the hooked functional component. That may be more in line with React's developmental direction.

ntucker commented 5 years ago

For mutation flows try/catch works great, but when doing GETs in your react components, you'll likely want to let the error throw in the component and then use an error boundary to handle fallbacks.

https://reactjs.org/docs/error-boundaries.html