-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
net: deprecate Temporary error status #45729
Comments
IMO the A “temporary” error ought to be one that could be resolved by retrying the same call with exactly the same arguments. However, if a |
I agree, but this isn't specific to |
This proposal has been added to the active column of the proposals project |
It sounds like deprecate means "leave things alone, comment that it's not to be used, and stop discussing it". |
Change https://golang.org/cl/322889 mentions this issue: |
Does anyone object to doing this? |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
Change https://golang.org/cl/340261 mentions this issue: |
It's not clear to me what the replacement is for Temporary inside server Accept loops (like net/http.Server's). Since this is closed, I asked about it on golang-nuts. |
Go 1.18 deprecated net.Error.Temporary(). This commit cleans up places where we use it incorrectly. Also, the rpc layer defines some errors that implement interface { Temporary() bool } I added comments to all of the implementations to indicate whether they will be required if net.Error.Temporary is ever ever removed in the future. For HandshakeError, the Temporary() return value is actually important. I moved & rewrote a (previously misplaced) comment there. The ReadStreamError changes were 1. necessary to pacify newer staticcheck and 2. technically, an error can implement Temporary() without being net.Err. This applies to some syscall errors in the standard library. Reading list for those interested: - golang/go#45729 - https://groups.google.com/g/golang-nuts/c/-JcZzOkyqYI - https://man7.org/linux/man-pages/man2/accept.2.html Note: This change was prompted by staticheck: > SA1019: neterr.Temporary has been deprecated since Go 1.18 because it > shouldn't be used: Temporary errors are not well-defined. Most > "temporary" errors are timeouts, and the few exceptions are surprising. > Do not use this method. (staticcheck)
The
net.Error
interface definesTimeout
andTemporary
methods:The meaning of "timeout" is reasonably intuitive: Was an error returned because an operation ran past a deadline?
However, the meaning of "temporary" is not obvious. What makes an error temporary vs. permanent? Is
EHOSTUNREACH
temporary (because it may result from a temporary network routing error)? Is an operation that ran past a deadline permanent (because retrying the operation without extending the deadline will still fail)?There is more discussion of
net.Temporary
in here, and in the following thread:#32463 (comment)
Looking at existing places where the standard library returns an error which implements a
Temporary() bool
method returning true (not counting ones where the temporary status is propagated from a more-specific error):Timeouts:
context
:context.DeadlineExceeded
crypto/tls
: AllDial
timeouts.net
: Various timeouts.net/http
: Timeout when reading headers or bodies. (The error type is namedhttpError
, but it is only used for timeouts.)net/http
: Also, HTTP/2 timeout reading response headers.os
:os.ErrDeadlineExceeded
(defined ininternal/poll
)Non-timeouts:
net
:ECONNRESET
andECONNABORTED
errors returned byaccept()
.net
:EAI_AGAIN
errors returned bygetaddrinfo()
.syscall/syscall_plan9.go
,syscall/syscall_unix.go
,syscall/syscall_js.go
,syscall/syscall_windows.go
:EINTR
,EMFILE
,ENFILE
, plus errors also considered timeouts:EAGAIN
,EWOULDBLOCK
,EBUSY
, andETIMEDOUT
. (Some minor variation between operating systems.)Ignoring cases where "temporary" is redundant with "timeout", the only "temporary" errors I can find are ones resulting from a small number of syscall errors. In most cases, "temporary" appears to mean "a timeout, or out of file descriptors". The documentation for
net.Error
does not make the limited scope of "temporary" errors obvious.There are a number of other places in the standard library where errors propagate through the "temporary" status of another error. The existence of these
Temporary()
methods makes it seem (to me at least) as if temporary errors are more prevalent than they actually are.Perhaps there is a useful definition of a "temporary" error. Perhaps we should provide a well-defined
os.ErrTemporary
or similar. That is not this proposal.I believe that:
net.Error
and indicate a "temporary" status is difficult.Temporary
does not implyTimeout
are surprising and not particularly useful.I propose that we deprecate the
Temporary
method:The text was updated successfully, but these errors were encountered: