Skip to content

useFetch should reject with an Error object #113

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

Closed
artdent opened this issue Sep 16, 2019 · 1 comment · Fixed by #114
Closed

useFetch should reject with an Error object #113

artdent opened this issue Sep 16, 2019 · 1 comment · Fixed by #114

Comments

@artdent
Copy link
Contributor

artdent commented Sep 16, 2019

Right now, if there's a request error, parseResponse from useFetch rejects with the response object instead of with an Error object. This causes two difficulties:

  • rejecting with a non-Error means you don't have a full stack trace
  • it contradicts the AsyncState type, so if you try to use state.error in TypeScript you have to cast it through unknown, i.e. ((error as unknown) as Response). (The intermediate step is necessary because Error and Response are unrelated types.)

The best fix is probably to change useFetch to throw an Error object that has a handle to the underlying response, which is unfortunately a backward-incompatible change. The other possibility I can think of is to widen or parameterize the error type, but I'm not sure it's a good idea to make the signature more complicated to support something that's not a great practice (i.e. throwing non-errors).

@artdent artdent changed the title useFetch should reject with an Error object useFetch should reject with an Error object Sep 16, 2019
@ghengeveld
Copy link
Member

I agree, this should be an Error, not a Response object. However we do want to keep a reference to the failed Response, so I think we need to introduce a FetchError that inherits from Error and has an additional response property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants