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/http: http.nothingWrittenError incompatible with errors.Is/As #42020

Open
zeisss opened this issue Oct 16, 2020 · 3 comments
Open

net/http: http.nothingWrittenError incompatible with errors.Is/As #42020

zeisss opened this issue Oct 16, 2020 · 3 comments
Milestone

Comments

@zeisss
Copy link

@zeisss zeisss commented Oct 16, 2020

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

$ go version
go1.15.2

Does this issue reproduce with the latest release?

I think so, yes.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/zeisss/Library/Caches/go-build"
GOENV="/Users/zeisss/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/zeisss/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/zeisss/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.15.2/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.15.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/x0/0_rddyy16qn1sy9n_pclmxqh0000gn/T/go-build239938081=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

  1. Performed a http request via net/http.Client
  2. Received a net/http.nothingWrittenError (write tcp 172.31.16.237:54422->1.2.3.4:80: write: connection reset by peer)
  3. Tried checking it with var opError *net.OpError; if errors.As(err, &opsError) { ... } to map to a user friendly errorcode
  4. Error gets reported as an unknown error Rollbar, thus I know it is a http.nothingWrittenError.

What did you expect to see?

I expected errors.As() to return true for this error

What did you see instead?

I saw errors.As() returning false.

I believe the problem to occur, since the error is neither the right type nor does Unwrap() return the OpError error at any point (since it is directly in http.nothingWrittenError. The same problem can be observed for errors.Is(). See https://play.golang.org/p/0XhEPLe0bRy

Use Case

We are running a load test generator written in Go and we need to provide our users with an aggregated error report, thus we need to be able to unpack and understand the errors we receive.

@toothrot toothrot changed the title net/go: http.nothingWrittenError incompatible with errors.Is/As net/http: http.nothingWrittenError incompatible with errors.Is/As Oct 16, 2020
@toothrot
Copy link
Contributor

@toothrot toothrot commented Oct 16, 2020

It seems like implementing Unwrap on net/http.nothingWrittenError makes sense to me. Alternatively, as it's not exported, it could be changed to work with errors.Is/As, which is essentially what it is doing before that existed.

/cc @neild

I'd be happy to do this change if others agree.

@toothrot toothrot added this to the Backlog milestone Oct 16, 2020
@neild
Copy link
Contributor

@neild neild commented Oct 16, 2020

I don't see a problem in principle, but I wonder if we should be returning nothingWrittenError at all. Perhaps it should be unwrapped before being returned to the user, as is done with transportReadFromServerError here:

https://go.googlesource.com/go/+/refs/heads/master/src/net/http/transport.go#605

I haven't traced all the code pathways, so I'm not sure if adding a case for nothingWrittenError at that location would be sufficient or not.

@zeisss
Copy link
Author

@zeisss zeisss commented Oct 17, 2020

If it makes a difference: nothing written is an information we would be happy to expose to our users. We are currently thinking about using "net/http/httptrace" for that though, so I guess nothing gets lost if you never return nothingWrittenError.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.