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

net: can't unwrap DNSError #63109

Closed
budougumi0617 opened this issue Sep 20, 2023 · 4 comments
Closed

net: can't unwrap DNSError #63109

budougumi0617 opened this issue Sep 20, 2023 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@budougumi0617
Copy link

budougumi0617 commented Sep 20, 2023

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

$ go version
go version go1.21.1 darwin/arm64

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='arm64'
GOBIN=''
GOCACHE='/Users/budougumi0617/Library/Caches/go-build'
GOENV='/Users/budougumi0617/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/budougumi0617/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/budougumi0617/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.21.1/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.21.1/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.1'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/ll/hpp009q97nvdhy4j49dmw96h0000gn/T/go-build623399656=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

The release notes for Go 1.19 mention the following specification about the "operation was canceled" error in the net package:

When a net package function returns an "operation was canceled" error, the error will now satisfy errors.Is(err, context.Canceled).

However, when I execute errors.Is(err, context.Canceled) against the "operation was canceled" error that returns from methods like *Dialer.DialContext, it does not evaluate to true. Is this in accordance with the specification?

https://go.dev/play/p/Os-LQrrlUx6

	_, err := (&net.Dialer{}).DialContext(ctx, "tcp", "localhost:12345")

	t.Logf("error was %q\n", err) // error was "dial tcp: lookup localhost: operation was canceled"
	
	if !errors.Is(err, context.Canceled) {
		t.Errorf("%T, was not context.Canceled", err) // *net.OpError, was not context.Canceled
	}

Looking at the execution result, it appears that the *net.DNSError within the *net.OpError has not implemented the Unwrap method. If the Err field in the DNSError struct retained its value as an error type, I believe the Is method of errCanceled (of type canceledError) from the net package would be called, and errors.Is(err, context.Canceled) would return true.

https://pkg.go.dev/net#DNSError

type DNSError struct {
	Err         string // description of the error
	Name        string // name looked for
	Server      string // server used
	IsTimeout   bool   // if true, timed out; not all timeouts set this
	IsTemporary bool   // if true, error is temporary; not all errors set this
	IsNotFound  bool   // if true, host could not be found
}

https://github.com/golang/go/blob/go1.21.1/src/net/net.go#L411-L422

	// For both read and write operations.
	errCanceled         = canceledError{}
	ErrWriteToConnected = errors.New("use of WriteTo with pre-connected connection")
)

// canceledError lets us return the same error string we have always
// returned, while still being Is context.Canceled.
type canceledError struct{}

func (canceledError) Error() string { return "operation was canceled" }

func (canceledError) Is(err error) bool { return err == context.Canceled }

What did you expect to see?

When a net package function returns an "operation was canceled" error, errors.Is(err, context.Canceled) returns true.

What did you see instead?

When a net package function returns an "operation was canceled" error, errors.Is(err, context.Canceled) returns false.

@ianlancetaylor
Copy link
Contributor

The problem, as your test shows, is that net.DNSError does not support unwrapping to an underlying error. I don't know how easy that will be to fix while retaining backward compatibility.

@ianlancetaylor ianlancetaylor changed the title net: The "operation was canceled" error returned by function in net package is not satisfy errors.Is(err, context.Canceled) net: can't unwrap DNSError Sep 20, 2023
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 20, 2023
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Sep 20, 2023
@mateusz834
Copy link
Member

mateusz834 commented Sep 20, 2023

I've made a proposal to fix this issue #63116.

@ianlancetaylor It should retain backward compatibility.

@bcmills
Copy link
Contributor

bcmills commented Sep 20, 2023

Duplicate of #63116

@bcmills bcmills marked this as a duplicate of #63116 Sep 20, 2023
@bcmills bcmills closed this as not planned Won't fix, can't repro, duplicate, stale Sep 20, 2023
@budougumi0617
Copy link
Author

I appreciate your quick triage and proposal creation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants