Retry on 5.x?

[bvitale]

We were using the retry option on 4.x, with a number (retry count). What is the default retry behavior in 5.x?

[panva]

No retry in 5.x

[paulius-valiunas]

you win this round :) I still think the correct place to have retry logic would be in this package because of all the reasons I mentioned above. But I guess me and all your other users will waste our time trying to figure out what kind of errors http.request can throw (it's not documented anywhere) and implementing the same thing on our own.

[panva]

I don't feel like I'm winning anything, in fact I am thinking whether to reconsider my position or not.

There have been no indices to so far in the year or so that 5.x is out. The most prevailing network related error is a timeout, which is dealt with by, well, increasing the permitted timeout duration through available means.

In my deployments of openid-client i'm not seeing ENOTFOUND, ECONNREFUSED, or ECONNRESET that frequently (or at all) to make me care.

Then, "specific HTTP status codes" you mention but did not provide much insight into, do not have a prescribed handling in OAuth 2.0, if an HTTP status code has a prescribed behaviour in OAuth 2.0 it is built in, usually with a IANA registered error code which is then echoed in the accompanied OPError rejection. Unexpected HTTP Responses usually come in the form of RPError with a description, those were never handled through the retry got mechanism anyway.

me and all your other users

If this was a problem for "all my other users" there would be way more related activity around this since the 5.x release a year ago, and by now close to 50% of the weekly npm downloads are for 5.x, with no retry.

I personally would choose to not hide network issues behind the API because most likely a frequent network issue is not something to hide, it's something to deal with on the network/host configuration layer, not in the software, hiding the fact that there's something rotting on the network.

[paulius-valiunas]

oh yeah, I said it half-jokingly. I don't set up new packages for testing very often so in order to save time I just went with what Node.js documentation had to say, which is pretty much nothing. Of course my assumptions with the limited information I had were inaccurate, so I had to acknowledge your victory here :)

The specific status codes and other errors I had in mind are documented here: https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md#retry

For example, in our deployments I've seen a few timeouts even with limit as long as 10 seconds, as well as EAI_AGAIN errors that come from DNS queries. I do agree that ideally the network should be made more stable so these things don't happen, but we don't live in an ideal world and that is not in my control. I'm not saying these errors should be handled by default, but I think it's very reasonable to expect being able to configure some retries if desired.

I also wish we lived in a world where auth servers only sent responses that were explicitly specified in OAuth 2.0, but again, we don't live in a perfect world and that means we get to deal with all kinds of unexpected issues from a 500 response to a temporary DNS outage.

And the most annoying part of all this is that Node.js doesn't document their errors properly so I'll have to read through got's code to figure out what errors they process and how.

[panva]

I am confident Node.js documents their errors. I'm not able to look for that at this time tho.

[paulius-valiunas]

Well sure, they have this: https://nodejs.org/api/errors.html#nodejs-error-codes
...but there's no telling which error codes apply to http requests and which don't. Also the http page says nothing at all about errors, other than the fact that you can subscribe to error events:
https://nodejs.org/api/http.html#class-httpclientrequest
https://nodejs.org/api/http.html#httprequesturl-options-callback

So I can either guess around or refer to a known good implementation, such as got.