dgrr / http2

HTTP/2 implementation for fasthttp
Apache License 2.0
210 stars 37 forks source link

ReadTimeout timer not resetting? #27

Open GitRowin opened 3 years ago

GitRowin commented 3 years ago

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

dgrr commented 3 years ago

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?

GitRowin commented 3 years ago
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)
}
dgrr commented 3 years ago

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

GitRowin commented 3 years ago

Yes, I'm sure.

Chrome: image

Firefox: image

GitRowin commented 3 years ago

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

dgrr commented 3 years ago

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

dgrr commented 3 years ago

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

dgrr commented 3 years ago

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

GitRowin commented 3 years ago

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

GitRowin commented 3 years ago

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.

dgrr commented 3 years ago

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 commented 3 years ago

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

GitRowin commented 3 years ago

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

dgrr commented 3 years ago

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

dgrr commented 2 years ago

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 commented 2 years ago

@GitRowin can you try version v0.3.1?