go-sse icon indicating copy to clipboard operation
go-sse copied to clipboard

what happened if the server has internal error when using go-sse as client

Open zhangxinyang97 opened this issue 1 year ago • 3 comments

here is my test code ` r, _ := http.NewRequestWithContext(c, http.MethodPost, "http://xxxx/v1/generate_stream", strings.NewReader(string(bodyS))) conn := sse.NewConnection(r)

ubsubscribe := conn.SubscribeToAll(func(event sse.Event) {
	eventS, _ := json.Marshal(event)
	switch event.Type {
	case "cycles", "ops":
		fmt.Printf("Metric %s: %s\n", event.Type, event.Data)
	case "close":
		fmt.Println("Server closed!")
	default: // no event name
		util.Log.Info(event.Data)
		messages <- string(eventS)
	}
	if event.Data == "[DONE]" || strings.Contains(event.Data, `finish_reason\":"`) {
		close(finish)
	}
})

go func() {
	<-finish
	util.Log.Debug("!!!!!!!finish")
	ubsubscribe()
}()

if err := conn.Connect(); err != nil {
	util.Log.Error(err.Error())
}`

I used wrong request body and found that it seems the subscriber is stucked and cannot quit.

zhangxinyang97 avatar Apr 15 '24 08:04 zhangxinyang97

Hi, thank you for raising the issue!

By default the client retries indefinitely if the connection is broken. But this doesn't seem to be the case here, as you're saying that your request's body was ill-formed. In this case, a well behaving server should return an error response code (something in the 400 range; if the payload is unexpected and it panics/throws an exception, there should be some catch-all handler for that which returns a 500). The go-sse client by default is configured to stop the connection attempt and return the error if it receives a non-200 response (see code).

It could be the case that the server itself panics/hangs, for example, sending no response to the client but still keeping the connection open or ending it abruptly. In both of these cases the client is configured to just retry, so it retries indefinitely with the same broken request, sent to the same broken server. In this scenario, if it is your server, it should be fixed; if it is another server, you could set the maximum retries number as a preventive measure (code for that is below).

With these being said, if the server responds, it would be helpful if you could provide an HTTP dump or at least the response body and code, so that I could investigate further.

Something you could also try is to configure the client to stop retrying the connection – this would help us pinpoint where it's getting stuck and why. If you could configure the client the following way:

client := sse.Client{
    OnRetry: func(err error, _ time.Duration) {
        log.Println("Retrying: ", err)
    },
    Backoff: sse.Backoff{
        MaxRetries: 5,
    },
}

req, _ := /* build your request with the ill-formed body */
conn := client.NewConnection(req)

// same code as yours from here

we could find out if it's stuck in a retry loop and, if that's the case, log the error which caused the retry and investigate further from there. If this logs nothing and is still stuck, then the problem is elsewhere.

To summarize:

  • if this is your server, make sure that ill-formed bodies are handled accordingly, by returning a proper error response
  • provide a copy of the HTTP response, if possible
  • try the code above to find out what error causes the client to block

Let me know what you manage to do!

tmaxmax avatar Apr 15 '24 12:04 tmaxmax

sorry for late reply, I found that the server is not very standard, and the server throw connection to server lost: EOF, I want no retry under this error, is it ok to set my own ResponseValidator

zhangxinyang97 avatar May 08 '24 08:05 zhangxinyang97

That will not work. What you could do is set sse.Client.Backoff.MaxRetries to -1, which will configure the client to never retry.

The response validator is for checking that a response received successfully is valid – that is, it is not a 404 for example.

tmaxmax avatar May 18 '24 15:05 tmaxmax

Ah, I just realized this is the same underlying issue I ran into. There are some additional details and one potential solution here: https://github.com/tmaxmax/go-sse/pull/38

emidoots avatar Jul 10 '24 04:07 emidoots

Closing this due to inactivity. Feel free to reopen, should you need further assistance.

tmaxmax avatar Aug 26 '24 09:08 tmaxmax