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

Jump to bottom

net/http: error unwrapping broken on some transport errors #56435

Closed
mraerino opened this issue Oct 26, 2022 · 5 comments
Closed

net/http: error unwrapping broken on some transport errors #56435

mraerino opened this issue Oct 26, 2022 · 5 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@mraerino
Copy link
Contributor

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

$ go version
go version go1.19.2 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/circleci/.cache/go-build"
GOENV="/home/circleci/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/circleci/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/circleci/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.19.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
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 -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2218043753=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Here's my repro that only seems to work on Linux: https://go.dev/play/p/k305YHkn2QM
You can run it a couple of times to get results like this:

Get "http://localhost:35633": write tcp 127.0.0.1:52368->127.0.0.1:35633: use of closed network connection
in chain: false
Get "http://localhost:35633": read tcp 127.0.0.1:52368->127.0.0.1:35633: use of closed network connection
in chain: true
Get "http://localhost:35633": read tcp 127.0.0.1:52368->127.0.0.1:35633: use of closed network connection
in chain: true
Get "http://localhost:35633": readLoopPeekFailLocked: read tcp 127.0.0.1:52368->127.0.0.1:35633: use of closed network connection
in chain: false
Get "http://localhost:35633": write tcp 127.0.0.1:52368->127.0.0.1:35633: use of closed network connection
in chain: false

What did you expect to see?

I expected net.ErrClosed to always be part of the error chain

What did you see instead?

It looks like for the write tcp and readLoopPeekFailLocked the error chain can't be unwrapped entirely.
@mraerino
Copy link
Contributor Author

i can make a PR for this!

@mraerino mraerino changed the title net/http: error unwrapping broken on readLoopPeekFailLocked net/http: error unwrapping broken on some transport errors Oct 26, 2022
@heschi
Copy link
Contributor

heschi commented Oct 26, 2022

cc @neild

@heschi heschi added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 26, 2022
@heschi heschi added this to the Backlog milestone Oct 26, 2022
@neild
Copy link
Contributor

neild commented Oct 26, 2022

These code paths all long predate error wrapping.

I'm happy to review a CL fixing the wrapping in places where we're getting it wrong now.

@neild neild added help wanted and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 26, 2022
@mraerino mraerino mentioned this issue Oct 27, 2022
Closed
@mraerino
Copy link
Contributor Author

PR is here: #56451

@heschi heschi added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 27, 2022
@gopherbot
Copy link

Change https://go.dev/cl/445775 mentions this issue: net/http: build error chains in transport that can be unwrapped

@golang golang locked and limited conversation to collaborators Nov 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

net/http: build error chains in transport that can be unwrapped mraerino/go
4 participants
@neild @mraerino @gopherbot @heschi