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

Closed
zeisss opened this issue Oct 16, 2020 · 7 comments
Closed

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

zeisss opened this issue Oct 16, 2020 · 7 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@zeisss
Copy link

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

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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 16, 2020
@toothrot toothrot added this to the Backlog milestone Oct 16, 2020
@neild
Copy link
Contributor

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 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.

@zeisss
Copy link
Author

zeisss commented Jan 4, 2022

This still occures with go version go1.17.3 linux/amd64.

@elagergren-spideroak
Copy link

Friendly ping about this issue. Do we have an idea which direction we want to go wrt unwrapping nothingWrittenError or making it implement Unwrap?

@konradreiche
Copy link

This has been fixed recently, see: #56451

@zeisss
Copy link
Author

zeisss commented Jun 23, 2023

I agree, 531ba0c should fix this and went live with Go 1.20.

@zeisss zeisss closed this as completed Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@toothrot @zeisss @neild @konradreiche @elagergren-spideroak