Skip to content

net: make errCanceled and errTimeout be "errors.Is" context.Canceled and context.DeadlineExceeded #51428

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

Closed
gprossliner opened this issue Mar 2, 2022 · 10 comments

Comments

@gprossliner
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.17.6 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/[email protected]/.cache/go-build"
GOENV="/home/[email protected]/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/[email protected]/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/[email protected]/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.6"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/[email protected]/proj/polly2/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build117018826=/tmp/go-build -gno-record-gcc-switches"

What did you do?

This is part of a bigger application, but here is a repo:
https://go.dev/play/p/mD-R6gerA4c

What did you expect to see?

I expect that the net.OpErr which is returned by net.dialer.DialContext included the wrapped original error, which is a context.Canceled error.

This may be the problematic function. There is also a TODO from @bradfitz

go/src/net/net.go

Lines 419 to 434 in 40e24a9

// mapErr maps from the context errors to the historical internal net
// error values.
//
// TODO(bradfitz): get rid of this after adjusting tests and making
// context.DeadlineExceeded implement net.Error?
func mapErr(err error) error {
switch err {
case context.Canceled:
return errCanceled
case context.DeadlineExceeded:
return errTimeout
default:
return err
}
}

What did you see instead?

UNEXPECTED:dial tcp: operation was canceled
EXPECTED: Get "http://github.com": context canceled
@ianlancetaylor
Copy link
Contributor

This can be confusing but I don't see how to return context.Canceled without breaking existing applications.

One thing we can do is add an Is method to net.errCanceled that makes it match context.Canceled. Then using errors.Is(err, context.Canceled) will return true for net.errCanceled, and I think that your program would work. Similarly for net.errTimeout. Does that seem useful?

@bcmills
Copy link
Contributor

bcmills commented Mar 3, 2022

Another option might be to give net.errCanceled an Unwrap method that returns context.Canceled, even though the context.Canceled error text does not appear in the errCanceled text.

It's not obvious to me whether that is better or worse than adding the Is method.

@gprossliner
Copy link
Author

gprossliner commented Mar 3, 2022

Adding the Is function is probably the best option.

@ianlancetaylor ianlancetaylor changed the title net: Wrapping context.Canceled error results in unexpected behavior proposal: net: add Is method to errCanceled and errTimeout so that they match context.{Canceled,DeadlineExceeded} Mar 4, 2022
@gopherbot gopherbot added this to the Proposal milestone Mar 4, 2022
@ianlancetaylor
Copy link
Contributor

OK, I turned this into a proposal for the behavior change.

@rsc
Copy link
Contributor

rsc commented Mar 16, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc changed the title proposal: net: add Is method to errCanceled and errTimeout so that they match context.{Canceled,DeadlineExceeded} proposal: net: make errCanceled and errTimeout errors.Is context.Canceled and context.DeadlineExceeded Mar 23, 2022
@rsc rsc changed the title proposal: net: make errCanceled and errTimeout errors.Is context.Canceled and context.DeadlineExceeded proposal: net: make errCanceled and errTimeout be "errors.Is" context.Canceled and context.DeadlineExceeded Mar 23, 2022
@rsc
Copy link
Contributor

rsc commented Mar 23, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Mar 30, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: net: make errCanceled and errTimeout be "errors.Is" context.Canceled and context.DeadlineExceeded net: make errCanceled and errTimeout be "errors.Is" context.Canceled and context.DeadlineExceeded Mar 30, 2022
@rsc rsc modified the milestones: Proposal, Backlog Mar 30, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/396877 mentions this issue: net: support error.Is of network errors and context errors

@gprossliner
Copy link
Author

Hey guys! You're doing such an incredible work. Not only from the Language perspective, but in terms of process and participation. Going in less than a month from Proposal to Acceptance is really impressive!

@hopehook
Copy link
Member

hopehook commented Apr 3, 2022

It's really awesome!!!

@rsc rsc moved this to Accepted in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@golang golang locked and limited conversation to collaborators Apr 3, 2023
@rsc rsc removed this from Proposals Apr 6, 2023
@dmitshur dmitshur removed this from the Backlog milestone Aug 22, 2023
@dmitshur dmitshur added this to the Go1.19 milestone Aug 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants