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

feat: verbose Failed to fetch error #6938

Merged
merged 4 commits into from Feb 24, 2021

Conversation

mathis-m
Copy link
Contributor

Signed-off-by: mathis-m mathis.michel@outlook.de

Description

make TypeError: Failed to fetch more verbose.
According to specification https://fetch.spec.whatwg.org/ it is not possible to distinguish between network errors:

A network error is a response whose status is always 0, status message is always the empty byte sequence, header list is always empty, and body is always null.

It is only viewable via developer tools e.g. console.

In order to make the ui information more verbose I added the following text to clarify the current response state:

"A network error occurred. This could be a CORS issue or a dropped internet connection. It is not possible for us to know."

Motivation and Context

Fixes #4560

How Has This Been Tested?

Screenshots (if appropriate):

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

@tim-lai
Copy link
Contributor

tim-lai commented Feb 17, 2021

@mathis-m What happens right now if it's an invalid url provided as compared to a no-network available? Is it a different err.message?

@mathis-m
Copy link
Contributor Author

mathis-m commented Feb 17, 2021

@tim-lai the error we can see in developer console is not inferable via javascript this is a security feature.
So we can choose to enhance the error message in case the error is the generic error "Failed to fetch."
There is no way to tell why this error occured, besides having a look at the developer console.
In order to enhance the error message we could do something like this:

image

Signed-off-by: mathis-m <mathis.michel@outlook.de>
@tim-lai
Copy link
Contributor

tim-lai commented Feb 18, 2021

@mathis-m Ok, thanks for the update. However, let's remove references to the console. To follow linting best practices, I also think we should remove or comment out the console.error entirely.

@tim-lai tim-lai merged commit 4db2edc into swagger-api:master Feb 24, 2021
@mathis-m
Copy link
Contributor Author

@tim-lai Thanks for the cleanup. I did not have the time to work on that!

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 this pull request may close these issues.

TypeError: failed to fetch is vague and unpleasant to look at when a request fails
2 participants