http2 icon indicating copy to clipboard operation
http2 copied to clipboard

ReadTimeout timer not resetting?

Open GitRowin opened this issue 4 years ago • 16 comments

Keep-Alive connections get closed ReadTimeout seconds after connecting, even if they're actively being used to make HTTP requests:

image

I'm using a ReadTimeout of 5 seconds. As you can see, 5 and 10 seconds after the initial request, the browser has to connect again.

I believe this issue is basically the same as this one: https://github.com/golang/go/issues/16450

GitRowin avatar Aug 26 '21 20:08 GitRowin

Hello. I don't understand your issue. I do, but I need more context. How do you set the ReadTimeout? In fasthttp or in the http2 library?

dgrr avatar Aug 27 '21 06:08 dgrr

server := &fasthttp.Server{
	ReadTimeout: 5 * time.Second,
	Handler: ServeHTTP,
}

tlsConfig := &tls.Config{...}

http2.ConfigureServerAndConfig(server, tlsConfig)

listener, err := net.Listen("tcp4", "0.0.0.0:443")

if err != nil {
	panic(err)
}

tlsListener := tls.NewListener(listener, tlsConfig)
err = server.Serve(tlsListener)

if err != nil {
	panic(err)
}

GitRowin avatar Aug 27 '21 15:08 GitRowin

Are you sure you are using http2? The library doesn't have any kind of read timeout. It doesn't care about timeouts really.

dgrr avatar Aug 27 '21 15:08 dgrr

Yes, I'm sure.

Chrome: image

Firefox: image

GitRowin avatar Aug 27 '21 15:08 GitRowin

Also, when I remove http2.ConfigureServerAndConfig(server, tlsConfig) so it uses HTTP/1.1, it doesn't happen.

GitRowin avatar Aug 27 '21 15:08 GitRowin

Here is the problem. Is not related to http2. Is an issue related to fasthttp.

dgrr avatar Aug 27 '21 16:08 dgrr

I'll let the issue open here, and create a new one in fasthttp

dgrr avatar Aug 27 '21 16:08 dgrr

@GitRowin can you check the version v0.2.4? Thanks

dgrr avatar Aug 28 '21 09:08 dgrr

Connections are no longer being closed 5 seconds after connecting, but timeouts don't work at all now.

GitRowin avatar Aug 28 '21 13:08 GitRowin

Maybe you're misunderstanding the problem.

ReadTimeout documentation:

ReadTimeout is the amount of time allowed to read
the full request including body. The connection's read
deadline is reset when the connection opens, or for
keep-alive connections after the first byte has been read.

Before v0.2.4, the read deadline was only reset when the connection opened, and now read and write timeouts are disabled completely. The expected behavior is the deadline resetting after the first byte of a HTTP request has been read.

GitRowin avatar Aug 28 '21 13:08 GitRowin

What are you expecting then? After 5 seconds return an error to the client? Which kind of error? An http2 specific code or an HTTP status?

dgrr avatar Aug 29 '21 15:08 dgrr

Try the version v0.2.5. I just send a cancel and remove the stream when the stream times out

dgrr avatar Aug 29 '21 18:08 dgrr

The timeout is not working:

image

Server:

server := &fasthttp.Server{
	ReadTimeout: 5 * time.Second,
	Handler:     ServeHTTP,
}

Also:

Servers are encouraged to maintain open connections for as long as possible but are permitted to terminate idle connections if necessary. When either endpoint chooses to close the transport-layer TCP connection, the terminating endpoint SHOULD first send a GOAWAY (Section 6.8) frame so that both endpoints can reliably determine whether previously sent frames have been processed and gracefully complete or terminate any necessary remaining tasks.

From: https://httpwg.org/specs/rfc7540.html#n-connection-management

GitRowin avatar Aug 30 '21 16:08 GitRowin

Oke, makes sense. I'll take a look later

dgrr avatar Aug 30 '21 16:08 dgrr

Hello,

After taking a look I still don't understand what's the issue. I don't know if you mean if a request needs to timeout or the connection needs to be closed after the ReadTimeout.

About the first one: I am working on an implementation for that (so same as fasthttp does Maximum duration for full response reading (including body).) About the second one: Connections are going to be active as long as they reply to ping messages from the server.

dgrr avatar Jan 12 '22 18:01 dgrr

@GitRowin can you try version v0.3.1?

dgrr avatar Jan 12 '22 19:01 dgrr