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

More informative error types #526

Open
jakearchibald opened this issue Apr 13, 2017 · 21 comments
Open

More informative error types #526

jakearchibald opened this issue Apr 13, 2017 · 21 comments
Labels
addition/proposal New features or enhancements topic: api

Comments

@jakearchibald
Copy link
Collaborator

Just had a chat with an internal team requesting more detailed errors, so they can tell the difference between a timeout / connection failure / bad params etc etc.

Given that we're going to be introducing AbortError, should we revisit other errors?

@annevk
Copy link
Member

annevk commented Apr 13, 2017

To some extent there's #352.

And then there's the question of what exactly resource timing (I believe that's the one, not sure) exposes, when, and how many user agents have decided that's an acceptable level of exposure.

If we have those details, we could consider it. (Then there's the problem of potential compatibility fallout, that we can only measure through telemetry (or maybe not) or we add information to TypeError instances which is rather hackish.)

@Noitidart
Copy link

This is also an issue in react-native because they use what-wg fetch. Is there a way to differentiate between timeout or offline? Currently both situation throw the same error which from my memory is something like "NetworkRequest failed".

https://stackoverflow.com/q/48847068/1828637

@mfalken
Copy link

mfalken commented Apr 12, 2018

A way forward may be to make a list of desired error types and see which ones are OK to expose cross-origin or not. Here's a strawman:

  • "API error" (gave something that's not a Request or a URL to fetch())
  • "network error" (something at the TCP/IP layer caused us to fail to fetch
  • "abort error" (the user closed the tab or clicked stop or something like that)
  • "timeout error" (the browser timed out the request (I'm not sure this really happens for fetch()))
  • "offline error" (this is a subset of network error when the browser knows it's offline)

I guess we already have the name AbortError taken with the new abortable fetch thing.

@annevk
Copy link
Member

annevk commented Apr 12, 2018

Most of those are probably reasonable (assuming "offline error" matches navigator.onLine). @domenic is someone still pursuing ways to add custom data to errors somehow in TC39? It seems hard to switch away from TypeError here.

@annevk
Copy link
Member

annevk commented Apr 12, 2018

@mattto per whatwg/xhr#96 WebKit at least has a network timeout. I suspect that applies to all browsers to some extent.

@jakearchibald
Copy link
Collaborator Author

@annevk why would it be hard to switch from TypeError? Do we think folks are looking at the type currently?

@annevk
Copy link
Member

annevk commented Apr 12, 2018

@jakearchibald I'd expect so; we certainly do so throughout the test suite. I know it's not common practice, but it's also rather hard to guarantee that nobody is doing it.

@jakearchibald
Copy link
Collaborator Author

@mattto is there any way we can get data on that?

Alternatively, we could give priority to FetchObserver and put the data there.

@annevk annevk added the addition/proposal New features or enhancements label Apr 12, 2018
@mfalken
Copy link

mfalken commented Apr 12, 2018

I imagine it'd be hard to get data on that. There's no clear API entry point we could add a UseCounter for. We could probably do a search on HTTPArchive to try to find sites that do this. What would the script code look like? Something like this?
fetch(...).catch(err => { err instanceof TypeError; })

cc @yutakahirano

@domenic
Copy link
Member

domenic commented Apr 12, 2018

@domenic is someone still pursuing ways to add custom data to errors somehow in TC39?

Sadly no.

@yutakahirano
Copy link
Member

@mattto

fetch(...).catch(err => { err instanceof TypeError; })

I feel it very strange because currently TypeError is the only error type fetch throws (except for AbortError, right, but it's still not very popular I guess...). The only usecase I can image except for testing is checking whether the error is thrown by fetch, together with statements that may throw errors other than TypeError, so that should be more complicated. Also, we need to care about .name, I think.

@jakearchibald
Copy link
Collaborator Author

@yutakahirano agreed. This is why I was surprised @annevk said it'd be difficult to switch. Although it seems difficult/impossible to know for sure.

@dcreager
Copy link
Contributor

dcreager commented Jun 5, 2018

This discussion aligns pretty closely with the Network Error Logging spec, which would allow the user agent to upload reports about these kinds of failures to the server owner. NEL currently defines a list of error types (a glorified string enum), but if that list were part of fetch, we wouldn't have to.

@aaronsn
Copy link

aaronsn commented Jun 6, 2019

What's the status of this? This would be very helpful to distinguish being offline from real errors - to help catch bad configurations, bugs in the service worker, etc.

@annevk
Copy link
Member

annevk commented Jun 7, 2019

Nobody is working on this, though it seems like necessary infrastructure, e.g., for Network Error Logging. If someone wants to take this on, there are roughly three parts here:

  1. Identify all the places where we create a "network error" today, annotate them with a private slot regarding the type of error and ensure that annotation makes it all the way to the caller. (E.g., there might be places where we return a new network error at some point as currently there is no distinction.) This should not be too hard, although when we get to the HTTP layer there might be some difficulty in identifying all the appropriate types. (Perhaps Network Error Logging can help.)
  2. Figure out what from 1 we can reasonably expose to JavaScript, without giving attackers more information than they have today.
  3. Figure out out how to expose what remains in 2 to JavaScript.

@jakearchibald
Copy link
Collaborator Author

I still think a fetch observer is the right way to expose it, but yeah, steps 1 & 2 can be done before we decide on that.

@sholladay
Copy link

Hi all, I am one of the maintainers of Ky, which is a fetch-based library for HTTP requests. We would like the ability to differentiate fetch errors from other types of errors, including implementation errors from our users. This currently seems to require inspecting error.message, which is full of problems. Among other things, different browsers throw errors with different messages for the same situation. It's very messy. We would very much appreciate any improvements that could be made here.

@RealAlphabet

This comment was marked as duplicate.

@silverwind
Copy link

There is now https://github.com/sindresorhus/is-network-error, a bandaid solution that matches on the returned error strings, which all implementations handle differently.

This is probably not easily fixable from a spec perspective because code in the wild relies on the existing TypeError class, so one can't just introduce a NetworkError class now.

@annevk
Copy link
Member

annevk commented Sep 28, 2023

Unfortunately I missed the nuance in the more recent replies that are not asking for more granularity but instead are asking for distinguishing the exception when you are further away from the caller. I suppose one thing we could do is try to standardize the message value. Looking at https://github.com/sindresorhus/is-network-error/blob/main/index.js Chromium's "Failed to fetch" seems pretty reasonable.

That's not a great solution, but it's an improvement.

I'm not entirely sure what a great solution would look like.

@disintegrator
Copy link

I came across this issue today and had to come up with a best-effort approach. I tweeted about it:

I can't find a reliable way to detect retryable connection/network errors using fetch across @nodejs, @bunjavascript, @GoogleChrome and other platform. It's really unfortunate that the spec authors landed on throwing TypeError for pretty much anything that can go wrong. This includes CORS and CSP violations on the browser which are not retryable and those errors are opaque to JS code. DOMException is already used by AbortSignal and could have been a good choice since it's .name property reflects an error code. An explicit FetchError would have been even better still.
Playground link

image

Unfortunately I cannot distinguish CSP/CORS-related errors from actual network errors using this code but it’s as close as I can get.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: api
Development

No branches or pull requests