tmaxmax / go-sse

Fully featured, spec-compliant HTML5 server-sent events library
https://pkg.go.dev/github.com/tmaxmax/go-sse
MIT License
296 stars 16 forks source link

Client connects with HTTP/2 but reconnects using HTTP/1.1 #34

Open hspaay opened 3 weeks ago

hspaay commented 3 weeks ago

Hi, I'm using the go-sse client to connect to a SSE server. This works fine. The initial connection is established with HTTP/2. When stopping and starting the server, the client automatically reconnects as expected. The interesting bit however is that the reconnect uses HTTP/1.1.

Is there a way to force a reconnect using HTTP/2?

A stripped down version of the code to establish the connection:

req, err := http.NewRequest("GET", serverURL, bytes.NewReader([]byte{}))
req.Header.Add("Authorization", "bearer "+bearerToken)
sseCtx, sseCancelFn := context.WithCancel(context.Background())
req = req.WithContext(sseCtx)
sseClient := sse.Client{}
conn := sseClient.NewConnection(req)
newBuf := make([]byte, 0, 1024*65)
conn.Buffer(newBuf, myMaxSSEMessageSize)   // thank you for this feature
remover := conn.SubscribeToAll(mySSEHandler)
go func() {
    err := conn.Connect()
    remover()
}()

PS: This is with golang 1.21

tmaxmax commented 3 weeks ago

Woah, this is an interesting one, thank you for flagging it!

As at the moment I'm a little strapped on time, a workaround could be that you disable retry amd implement a simple retry loop yourself (hopefully your retrying needs are not too complicated). Or you could use the cenkalti/backoff library – it was previously used internally to implement retrying but I've dropped it because I had to kind of contrive the client code around it and didn't want external dependencies to appear in the library's API (neither of which problems should be an issue to you, it is a great library).

I'll be back with an update once I get to take a look. Let me know if you find anything else of interest!

hspaay commented 3 weeks ago

Interesting .. when following your suggesting and disabling retry and instead restarting the client with a new request, the restarted client is still reported by the server as connecting wiht HTTP/1.1. Adding a second client, it also connects as http/1.1.

When not restarting the server, both clients connect with http/2. This leads me to believe that this is not an issue with the go-sse client. For some reason the server restart causes connections to fall back to http/1.1.

The server side has two configurations. One is a standard tls server using http package and the go-sse server. After a reconnect it starts raising "http: superfluous response.WriteHeader call" errors. The second configuration is using my own sse server (a simple test server) which does not raise this error. I hope this still makes sense.

It looks like I've got to start stripping this down to a simpler stand-alone test setup to get to the bottom of it.

hspaay commented 3 weeks ago

To add to the weirdness. The client doesnt retry any more (? uhm yes, I though it used to ... :question: ) when the server restarts. When the server disconnects gracefully it sends the response header which is missing Content-Type.

When on the server side I add:

w.Header().Set("Content-Type", "text/event-stream")

before :

ServeHTTP(w, r)

Then the client does retry when the server restarts as the Content-Type text/event-stream is present.

This makes sense as in client_connection.go:210, the call to ResponseValidator fails because in client.go:108 there is a check for this header.

Maybe this should go into a separate issue.

hspaay commented 3 weeks ago

Last update: good news. I've managed to have the reconnect use http/2. The problem is not in go-sse.

The fix was to create a http.Client using a http2.Transport instead of the DefaultTransport. Oddly enough, when using DefaultTransport the first connection does use http/2 but a reconnect switches to http/1.1. I don't understand why but guess some internal state is changed.

Maybe the code below can be useful in the documentation for others encountering the same issue.

Create the http client as follows (when the CA certificate is known and residing in myCaCertPool):

func CreateHttp2TLSClient(caCert *x509.Certificate, timeout time.Duration) *http.Client {
    // the CA certificate is set in NewTLSClient
    caCertPool := x509.NewCertPool()
    if caCert != nil {
        caCertPool.AddCert(caCert)
    }
    tlsConfig := &tls.Config{
        RootCAs:            caCertPool,
        InsecureSkipVerify: caCert == nil,
    }
    //tlsTransport := http.DefaultTransport.(*http.Transport)
    tlsTransport := &http2.Transport{
        AllowHTTP: true,
        DialTLSContext: func(ctx context.Context, network, addr string, cfg *tls.Config) (net.Conn, error) {
            c, err := tls.Dial(network, addr, cfg)
            return c, err
        },
        TLSClientConfig: tlsConfig,
    }
    return &http.Client{
        Transport: tlsTransport,
        Timeout:   timeout,
    }
}
tmaxmax commented 1 week ago

Hm, very interesting and peculiar! Thank you for reporting this and looking into it. I'll use the information you've provided to investigate the standard library a little to see why this is happening – if the cause is clearly determined, it can be established whether this is a problem with go-sse's usage of the standard library, an issue with the standard library, or simply neither.

I'll get back with an update when I find some time to research this. Leaving the issue open for reference, in case other people encounter this – and for the others that read this and have this problem, please share your experience too!