-
-
Notifications
You must be signed in to change notification settings - Fork 27
Description
The current client API offers a great deal of flexibility:
- with an
sse.Client
multipleConnection
s with the same configuration can be made - there can be multiple event listeners (for distinct or the same event)
- event listeners can be added or removed after the connection is established
- event listeners can listen to a single event type or all of them.
This doesn't come for free, though: both the user-facing API and the implementation code are complex, and the client uses a bit more resources, generates more garbage and must ensure serialized concurrent access to internal state. Moreover, the current client:
- does not announce in any way what the connection status is – see Appendix 1 to this proposal for context and how it could be tackled.
- is cumbersome for consuming OpenAI-style streams – see Appendix 2 for a utility specifically designed for reading such streams.
As of now, the instantiation of a connection with cancellation, some custom configuration and sending data on a channel looks as follows:
ctx, cancel := context.WithCancel(context.Background()) // or other means of creating a context, might come from somewhere
defer cancel()
client := sse.Client{
/* your options */
}
r, _ := http.NewRequestWithContext(ctx, http.MethodGet, "http://localhost:8000", http.NoBody)
conn := client.NewConnection(r)
ch := make(chan sse.Event)
go func() {
for ev := range ch {
fmt.Println("%s\n\n", event.Data)
}
}()
conn.SubscribeMessages(func(event sse.Event) {
ch <- event
})
if err := conn.Connect(); err != nil {
fmt.Fprintln(os.Stderr, err)
}
I've added the channel because from what I have observed, most users of the library create callbacks which mainly send the events on a channel to be consumed elsewhere.
I think this is quite a mouthful and I wonder whether enough use of the aforementioned flexibilities is made for them to justify the current API.
Here's another way in which I think the client could be designed. Instead of having a Client
type and a Connection
type with many methods, we could instead have the following:
package sse
// Connect does the HTTP request, receives the events from the server and sends them
// on the given channel.
//
// Returns errors if any of the parameters are invalid. Besides that it has the exact same
// behavior as `sse.Connection.Connect` has.
func Connect(req *http.Request, msgs chan<- Event, config *ConnectConfig) error
type ConnectConfig struct {
/* the same stuff that is on sse.Client currently */
}
Usage would look as follows:
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ch := make(chan sse.Event)
go func() {
for ev := range ch {
if ev.Type == "" {
fmt.Printf("%s\n\n", event.Data)
}
}
}()
config := &see.ConnectConfig{
/* your options */
}
r, _ := http.NewRequestWithContext(ctx, http.MethodGet, "http://localhost:8000", http.NoBody)
if err := sse.Connect(r, ch, config); err != nil {
fmt.Fprintln(os.Stderr, err)
}
It is not that much shorter, but assuming that the context comes from elsewhere and that the configuration is already defined, the code necessary for establishing a connection is significantly shorter – creating an http.Request
and calling Connect
. Connection with the default configuration would also not need a separate top-level function – just pass nil
instead of a ConnectionConfig
!
There are two important changes here, though:
- checking that the received events are of the desired type is now the user's responsibility
- new event listeners cannot be added (so easily) on the fly – the user would have to implement this themselves
For example, if we receive three diferent event types and we handle them differently, previously one could do:
conn.SubscribeEvent("a", func(e sse.Event) {
aCh <- e
})
conn.SubscribeEvent("b", func(e sse.Event) {
bCh <- e
})
conn.SubscribeEvent("c", func(e sse.Event) {
cCh <- e
})
if err := conn.Connect(); err != nil {
// handle error
}
With this change, it would look like this:
evs := make(chan sse.Event)
go func() {
for e := range evs {
switch e.Type {
case "a":
aCh <- e // or just handle them here instead of sending them on another channel
case "b":
bCh <- e
case "c":
cCh <- e
}
}
}()
if err := sse.Connect(req, evs, nil); err != nil {
// handle error
}
On the flipside, simple requests would be easier to make. Consider a request to ChatGPT:
prompt, _ := json.Marshal(OpenAIRequest{
Model: "gpt-4-1106-preview",
Messages: msgs,
Stream: true,
})
r, _ := http.NewRequest(http.MethodPost, "https://api.openai.com/v1/chat/completions", strings.NewReader(string(prompt)))
r.Header.Add("Authorization", fmt.Sprintf("Bearer %s", OpenAIKey))
r.Header.Add("Content-Type", "application/json")
conn := sse.NewConnection(r)
conn.SubscribeMessages(func(ev sse.Event) {
events <- ev // it is processed elsewhere
})
if err := conn.Connect(); err != nil {
/* handle error */
}
This would be the new version:
prompt, _ := json.Marshal(OpenAIRequest{
Model: "gpt-4-1106-preview",
Messages: msgs,
Stream: true,
})
r, _ := http.NewRequest(http.MethodPost, "https://api.openai.com/v1/chat/completions", strings.NewReader(string(prompt)))
r.Header.Add("Authorization", fmt.Sprintf("Bearer %s", OpenAIKey))
r.Header.Add("Content-Type", "application/json")
if err := sse.Connect(r, events, nil); err != nil {
/* handle error */
}
There are obvious benefits:
- much less boilerplate – no more
NewConnection
->SubscribeMessages
->Connect
- it is not possible to connect without receiving the messages
- given that the connection code is shorter, focus is moved on the creation of the request
- handling the response data happens directly in user's code – there's no function boundary to separate business logic, no inversion of control
As an analogy, imagine if the net/http.Client
would be used something like this:
conn := http.Client.NewConnection(req)
conn.HandleResponse(func(res *http.Response) {
// do something with the response
})
if err := conn.Connect(); err != nil {
// handle error
}
It would be painful to use.
The main advantage of the new API would be, I believe, that the control of the response is fully in the library user's hands. There are no callbacks one needs to reason about; there is no need for the user to look up the source code to find out how the Connection
behaves in various respects – for example, in what order the event listeners are called; finally, in a paradoxical manner there would be one single way to do things: for example, if one wants to handle multiple event types, currently they can register multiple callbacks for each event, or write the same switch
code as above inside a callback passed to SubscribeAll
. Also, it would be much easier to maintain – this change would result in ~200LOC and 6 public API entities being removed. This reduction in API surface reduces documentation and in the end how much the user must learn about the library in order to use it effectively.
Looking forward on your input regarding this API change!