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.

Sign up for GitHub

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

Jump to bottom

net: DNSError doesn't properly wrap context errors in lookup.go #63727

Closed
AdminBenni opened this issue Oct 25, 2023 · 1 comment
Closed

net: DNSError doesn't properly wrap context errors in lookup.go #63727

AdminBenni opened this issue Oct 25, 2023 · 1 comment

Comments

@AdminBenni
Copy link

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

$ go version
go1.21.0 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/benedikt/.cache/go-build'
GOENV='/home/benedikt/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/benedikt/go/pkg/mod'
GONOPROXY='[NOT IMPORTANT]'
GONOSUMDB='[NOT IMPORTANT]'
GOOS='linux'
GOPATH='/home/benedikt/go'
GOPRIVATE='[NOT IMPORTANT]'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/benedikt/sdk/go1.21.0'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/benedikt/sdk/go1.21.0/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='0'
GOMOD='[NOT IMPORTANT]'
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 -m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build810391928=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I am using go-amqp. I dial (which calls net's Dialer.DialContext) in a goroutine and then a situation happens in the main routine which causes me to cancel while dialing.

// Main routine
ctx, cancel := context.WithCancel(context.Background())
go otherRoutine(ctx)
defer cancel()
// Other routine
conn, err := amqp.Dial(ctx, endpoint, nil)
if err != nil {
    fmt.Println(errors.Is(err, context.Canceled))
}

What did you expect to see?

true

What did you see instead?

false

Why this is happening

In lookupIPAddr in net/lookup.go we see the following code.

err := &DNSError{
    Err:       mapErr(ctxErr).Error(),
    Name:      host,
    IsTimeout: ctxErr == context.DeadlineExceeded,
}
...
return nil, err

Which of converts the error into a string without wrapping it properly.
mapErr(ctxErr) converts the error to an instance of canceledError, which is defined with the following comment.

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

Which means that canceledError is even meant to be Is compatible with context.Canceled, which is immediately thrown away by calling .Error().

Proposed solution

DNSError should additionally store the original error and implement Unwrap so that Is can properly work to detect the error that spawned the DNSError.
@seankhliao
Copy link
Member

Duplicate of #63116

@seankhliao seankhliao marked this as a duplicate of #63116 Oct 25, 2023
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@seankhliao @AdminBenni