net/http: GOAWAY reported as http.http2GoAwayError instead of http2.GoAwayError

[Miosss]

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

go version go1.11.1 windows/amd64

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

go env Output

set GOARCH=amd64
set GOBIN=
set GOCACHE=...\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\go
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=...\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=...\AppData\Local\Temp\go-build204600086=/tmp/go-build -gno-record-gcc-switches

What did you do?

Get an URL, and try to read the body:

response, err := http.Get(task.currentURL)
response.Body.Read()

The server occasionaly closes the connection with GOAWAY, probably when overloaded with requests - the sample app scrapes many urls in short period of time.

What did you expect to see?

Error that could be caught by the code and potentially handled (retry GET):

if e, ok := err.(http2.GoAwayError); ok {
                        ...
}

The http2.GoAwayError struct I found on the github in other issues.

What did you see instead?

Error was not recognized by above assert, instead it was passed to general error handler. Reflection on the error gave this data:

http.http2GoAwayError <- typeof name
http2: server sent GOAWAY and closed the connection; LastStreamID=1999, ErrCode=NO_ERROR, debug=""

It seams that error is of type http.http2GoAwayError which is internal package's struct and cannot be used in my code.

My HTTPS server (nginx) upgrades connections from HTTP to HTTP/2, so maybe the issue is when connection from http.Get is upgraded under the hood to HTTP/2 and the code does not translate the error codes appropriately?

Btw. Error reporting is not the best go's feature - it is hard to know what types of error can be returned to handle them accordingly. And the docs do not always tell everything either. The GOAWAY error I got from testing, though I wrote error handling.

[dgryski]

@bradfitz

[agnivade]

I think you mean to say that http2GoAwayError should be exposed. I will re-title the issue appropriately.

[Miosss]

@agnivade Not sure exactly what should be the solution, but there should be a way to "catch" this error.
Why is the http2.GoAwayError not returned in this case? Looks like the perfect candidate for this situation and it is already exposed.

Btw, the posiibility to return package-specific error to other packages seems somewhat wrong.

[agnivade]

Oh my bad, I thought you wanted to expose the error. The original issue title was correct then.

[bradfitz]

This is an unfortunate side effect of how we bundle http2 into net/http. There really are two distinct types, copies of each other: http.GoAwayError and net/http.http2GoAwayError. We're not going to export the latter.

But this isn't even an error that's meant to be acted on: the server sends those during graceful shutdown but shouldn't be interrupting existing requests in flight. If the GOAWAY last_stream_id is less than the one we just sent, the Transport will already reschedule it onto a different/new connection. The only time you should be seeing this if the response body was already partially sent/consumed and the server just closed the connection un-gracefully.

But that's no more interesting than a server that just had its http server crash or otherwise closed its TCP connections ungracefully. If it's important to you, retry. Why is this specific error interesting? If it is, we could add an func IsGoAwayError(err error) somewhere, or a func asking a more interesting question, because it's not clear what a user is really asking with "Is this a goaway error?".

Could you explain more about why you care about this error in particular?

[Miosss]

The only time you should be seeing this if the response body was already partially sent/consumed and the server just closed the connection un-gracefully.

That's probably my case. I send multiple requests to low performance server at one time (scrapper). It seems that when nginx feels overloaded it just sends goaway in the middle of response - which effectively means that I get no useful response.

But this is really not important - tha case is that sometimes TCP connection returns error instead of useful response - developer should have opportunity to handle it I suppose.

I should retry you say, and it is precisely what I do. But when should I retry?

I react to http.Get errors, which all should be of type *url.Error - ok, it works. Then it occurs, that request is not really finished after call to Get - because reading response.Body is the real request execution, network action, etc. - and it can return error too (which is this case). godoc's for response.Body do not indicate what type of error to expect - by trial and error I am handling those: *net.OpError and *http2.GoAwayError, but as we agreed, the latter never occurs in this case - not in this form at least.
So what, you say, catch all errors - well, as you may expect, different errors require different actions. And what's more important is that http.http2GoAwayError is not implementing interfaces for other expected errors, like *net.OpError``.

So in effect, I can react to this specific error either via if err != nil which is very big cannon (especially when you use error for application specific errors during processing of some task, which is nice feature of go), or the workaround that I use now:

if reflect.TypeOf(r.GetError()).String() == "http.http2GoAwayError"

In conclusion - I do not want to react precisely to http.http2GoAwayError (but I should have possibility if I needed to), but I want to catch all network / http errors during request in simple type assertion - to retry.

[ghost]

if reflect.TypeOf(r.GetError()).String() == "http.http2GoAwayError"

Or strings.Contains(err.Error(), "http2: server sent GOAWAY").

[bradfitz]

@Miosss, I'm going to close this as a duplicate of #9424 then, which is about errors & error handling in the HTTP client. Feel free to add your requirements there.