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

Propagate status and statusText from fetchInternal() #290

Closed
petebacondarwin opened this issue Jan 24, 2022 · 3 comments
Closed

Propagate status and statusText from fetchInternal() #290

petebacondarwin opened this issue Jan 24, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@petebacondarwin
Copy link
Contributor

See #287 (comment)

@petebacondarwin petebacondarwin added the enhancement New feature or request label Jan 24, 2022
@kination
Copy link
Contributor

@petebacondarwin seems it is already sending error with status/statusText

throw new Error(
      `Failed to fetch ${resource} - ${response.status}: ${response.statusText}\nInvalid JSON response:\n${jsonText}`
    );

is there something more for this issue?

@petebacondarwin
Copy link
Contributor Author

The idea is that rather than throwing a generic error here, we return an object that contains the status information, so that the caller can then choose what to do (and perhaps throw an error if appropriate).

@threepointone
Copy link
Contributor

Based on the last few months, looks like the .code has been useful and is usually a more refined form of .status. When we don't have a .code, we throw the error with the .status and .statusText in the message, which has been sufficient. I think we may not need to do anything else here, so do you suppose we should close this out, and open a fresh issue when we have a specific need?

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

No branches or pull requests

3 participants