proposal: net: add ErrRetryableAcceptError

[neild]

Proposal Details

Proposal: Add the following to the net package:

// ErrRetryableAcceptError indicates a Listener.Accept error may be transient,
// and that a future call to Accept may succeed. This error is usually wrapped
// in another error, and should be tested for with errors.Is(err, ErrRetryableAcceptError).
//
// Accept errors corresponding to the errno values EMFILE, ENFILE, ECONNRESET, and ECONNABORTED
// match this error.
var ErrRetryableAcceptError = errors.New("ErrRetryableAcceptError")

Implementation will be to add an Is method to net.OpError which performs the appropriate check.

Background/rationale:

Some of the errors returned by net.Listener.Accept are potentially transient: While they indicate that the Accept call has failed, a future call may succeed.

Examples include:

• EMFILE: the process is out of file descriptors
• ENFILE: the system is out of file descriptors
• ECONNRESET and ECONNABORTED: see net/http: possible issues with temporary errors from Accept #6163

A server which calls Accept may want to ignore all these errors and call Accept again, probably after some backoff period. One of the uses for the deprecated net.Error.Temporary method is to check for these conditions, since we report all of the above as "temporary" errors.

As an example, net/http.Server uses net.Error.Temporary to retry potentially-transient Accept errors.

With the deprecation of net.Error.Temporary there is no non-deprecated way to test for these conditions. Temporary was deprecated, not because it isn't useful to test for retryable Accept errors but because the Temporary condition was ill-defined for many other errors, conflated transient errors with timeouts, and was just generally misunderstood. (See #45729.) I propose adding ErrRetryableAcceptError to provide a focused and reasonable well-defined replacement for the specific use case of Accept loops.

As an alternative, it might be nice if Accept just worked and didn't require the user to check for transient errors. However, that would require changing long-established behavior, and defining correct handling of EMFILE (for one example) seems challenging. This proposal is less ambitious in scope.

Further references:

• net/http: server.Serve() uses deprecated net.Error.Temporary()  #66208
• https://groups.google.com/g/golang-nuts/c/-JcZzOkyqYI/m/xwaZzjCgAwAJ

[ianlancetaylor]

The concept of "retryable" seems as nebulous as the concept of "temporary". Some errors in some contexts are retryable, while the same errors in different contexts are not. What if we instead define a couple of new errors that may be used with errors.Is, and update the Accept method docs to refer to them. Could we use something like ErrNoResources (which would include ENOMEM and ENOBUFS) and ErrConnectionFailed?

[mateusz834]

Similar proposal #60281, but only for EMFILE and ENFILE.

[neild]

I think there's some value to capturing the set of errors that http.Server retries on in a single check. Are there cases where users would want to handle ErrNoResources and ErrConnectionFailed differently?

ErrNoResources and ErrConnectionFailed would presumably apply across different syscalls. On one hand, that might be useful (although I'm not sure where else one would use them). On the other hand, it's a less focused proposal than this one, which aims to narrowly address Accept.

I think the old definition of Temporary when used for Accept errors is mostly fine; we deprecated it because it was very nebulously defined for anything other than Accept, and the interaction between timeout and temporary was highly confusing.

[ianlancetaylor]

It seems to me that ErrConnectionFailed is pretty much an automatic retry, whereas ErrNoResources could in some cases lead to load shedding. EMFILE in particular doesn't seem like an automatic retry for some servers.