Skip to content

TimeoutHandler and http.Server timeout usage issue at same time #35316

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

Closed
bentcoder opened this issue Nov 2, 2019 · 1 comment
Closed

TimeoutHandler and http.Server timeout usage issue at same time #35316

bentcoder opened this issue Nov 2, 2019 · 1 comment

Comments

@bentcoder
Copy link

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

$ go version
go version go1.13 darwin/amd64

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/hi/Library/Caches/go-build"
GOENV="/Users/hi/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/hi/Server/Go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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/pr/g7230wl10mnf0fnqd6cc1_f80000gn/T/go-build184956356=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

http.server

&http.Server{
    Addr:         env("HTTP_SERVER_ADDRESS"),
    ReadTimeout:  time.Duration(5) * time.Second,
    WriteTimeout: time.Duration(5) * time.Second,
    IdleTimeout:  time.Duration(120) * time.Second,
    ConnState: func(conn net.Conn, state http.ConnState) {
        fmt.Println(state.String())
    }
}

http.server.TimeoutHandler

http.TimeoutHandler(h, time.Duration(5) * time.Second, "uuuuu")

When I use timeout settings shown above, long running request response (sleeping on purpose - e.g. 7 sec) is always curl: (52) Empty reply from server rather than 503 Service Unavailable and the connection state goes from active to closed. Then the wr.Write() causes ErrHandlerTimeout which is expected as per doc.

What I was expecting is that, response with a 503 Service Unavailable because the server and TimeoutHandler timeouts are same. To achieve this outcome, I'll have to reduce TimeoutHandler duration so it timeouts just before server does - e.g. 4sec. However, timeouts are not inline anymore and cause misunderstanding. Or just remove WriteTimeout which doesn't feel appealing.

Another thing is that, TimeoutHandler is not finely configurable. For example, it doesn't accept Content-Type; body cannot be empty ..... Never mind the actual issue mentioned above, this, on its own is enough for me to ditch TimeoutHandler anyway.

Finally, TimeoutHandler doesn't serve the purpose for me so is there any way of returning 503 Service Unavailable or even better 408 Request Timeout if the server timeouts are met as they are more Request focused?

Thanks

@FiloSottile
Copy link
Contributor

The Server timeouts are documented to close the connection upon firing. They couldn't do otherwise because if the ReadTimeout fires, the client is still sending the request, while if the WriteTimeout fires the handler might have already started sending the response.

If you make WriteTimeout and TimeoutHandler race, which one wins will be undefined, but I would indeed expect the connection to get closed by the WriteTimeout before TimeoutHandler has a chance to send a response.

If you need to change the behavior of TimeoutHandler, you cam reimplement it in your own code, it does not use any internals as far as I know.

@golang golang locked and limited conversation to collaborators Nov 4, 2020
gopherbot pushed a commit to golang/tools that referenced this issue Apr 16, 2021
…code

Modifies README.md to be clearer about the necessary version of the
typescript compiler. Adds generated code that allows unmarshalling
type errors on Initialize messages.

See https://go-review.googlesource.com/c/tools/+/310109
Fixes  golang/go#35316

Change-Id: Id128c23e807e67e02d1354edaa0b164c9d36101c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/310753
Trust: Peter Weinberger <[email protected]>
Trust: Rebecca Stambler <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants