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: Timeout while reading response body gives unexposed http.httpError #58279

Open
exargon opened this issue Feb 2, 2023 · 2 comments
Open

net/http: Timeout while reading response body gives unexposed http.httpError #58279

exargon opened this issue Feb 2, 2023 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@exargon
Copy link

exargon commented Feb 2, 2023

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

$ go version
go version go1.20 linux/amd64

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/user/projects/go/test_http/go.mod"
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 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3588466548=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Issue not reproducible on playground (needs network).
Might need to adjust timeout the hit the reading of the body on your network.
https://play.golang.com/p/9yaX7aYDf4A

package main

import (
	"bytes"
	"errors"
	"io"
	"log"
	"net/http"
	"time"
)

const httpTimeout = 600 * time.Millisecond

func main() {
	client := &http.Client{Timeout: httpTimeout}
	for call := 20; call >= 0; call-- {
		data := make([]byte, 100000)
		buffer := bytes.NewBuffer(data)
		req, _ := http.NewRequest(http.MethodGet, "https://httpbin.org/anything", buffer)
		resp, err := client.Do(req)
		if err != nil {
			log.Printf("Header timeout: %s", err)
			continue
		}
		defer resp.Body.Close()
		body, err := io.ReadAll(resp.Body)
		if err != nil {
			for newErr := err; newErr != nil; newErr = errors.Unwrap(newErr) {
				log.Printf("type: %T\n value: %+v\n", newErr, newErr)
			}
			return
		}
		log.Printf("No error: %d", len(body))
	}
}

What did you expect to see?

A *url.Error, since that is what Do and related method returns, but another exposed error type would also work. Preferably wrapping the underlying error (context.DeadlineExceeded in this case)

What did you see instead?

A *http.httpError, which is not exposed, and does not wrap underlying errors. This makes it impossible to handle this timeout with errors.Is or errors.As
@bcmills
Copy link
Contributor

bcmills commented Feb 2, 2023

(attn @neild)

@bcmills bcmills added this to the Backlog milestone Feb 2, 2023
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 2, 2023
@exargon
Copy link
Author

exargon commented Feb 3, 2023 •
edited

So after looking a bit more at it, I noticed I am able to catch the error using the net.Error interface, and then use the Timeout method to check that it is a timeout.

var netErr net.Error
if errors.As(err, &netErr) && netErr.Timeout(){
...
}

I guess this is good enough for most usage (including my current ones), but it should be documented better. Returning an unexposed type like this seems like bad practice though? An exposed type would also allow for more specific handling (especially if also wrapping underlying errors) and avoid confusion during debugging

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
2 participants
@bcmills @exargon