sse
sse copied to clipboard
This library doesn't seem to follow the EventSource spec.
Hello, me again... I noticed that this library just reads a line and parses it, looking for a <field>:
and then sends that as an event. This is not correct according to the EventSource spec.
Is there a reason you chose to implement this library in the current form, or were you trying to comply with the standard?
If so, I've made some changes to a local copy of your library to make it follow the standard (although it's not 100% yet) and would be happy to create a PR.
Here's a sample gist that demonstrates the issue: https://gist.github.com/jeffreydwalter/a01b14c51a6c0fc405278f49e0d99db2
Here's a fork that has my proposed changes (doesn't 100% follow the standard yet): https://github.com/jeffreydwalter/sse/commit/998bb6797a1720029c2c8655ccf3c5ed7fc96d86)
When I run the above gist, I get:
go run blah.go
ID:
EVENT: message
DATA:
ERROR:
ID:
EVENT:
DATA: {"status":"connected"}
ERROR:
^Csignal: interrupt
What I would expect (according to how I understand the EventSource standard is that the Event should be like this:
$ go run blah.go
ID:
EVENT: message
DATA: {"status":"connected"}
ERROR:
This library is also missing the retry
semantics and the handling of the Last-Event-ID
doesn't seem right.
I've used this Python sse client library in production code and have looked at it's code quite a bit. It seems to follow the standard pretty closely and would be a good library to model.
Hi!
As the library was released with a server implementation and was only intended to be used in conjunction with one another, there was never any emphasis on conforming to the EventSource Spec per say.
Given that this library is seeing more use than I anticipated, any changes to improve adherence to ES spec is a good thing.
So far the changes on your branch look great! :)
Once you've got something that you're happy to submit, please open a PR and i'll do some testing
Cheers
Going to leave this open since retry isn't fully implemented.
@purehyperbole What is the status of this? Isn't it implemented fully now to be complient with EventSources specifications?
Any update on this?
@loeffel-io The implementation adheres to the SSE specs on everything with the exception of supporting the retry
field. We use exponential backoff to determine when to reconnect, so it never made sense to support that usecase where that value is actually used, which isn't everywhere.
It's something that could be added easily however, or you can set your own reconnect strategy by setting ReconnectStrategy
Thanks for the info @purehyperbole
The implementation adheres to the SSE specs on everything with the exception of supporting the retry field.
Great news! I suggest changing the issue title to reflect this new state. I got a bit worried for a moment when evaluating if we should use this for a new project.
FYI you can find spec compliant and optimised decoder here maybe you find it useful. You can use the decoder with any io.Reader
, so it should not be hard to integrate. You could ignore the rest of the primitives.
Hi. We have noticed there is a mandatory query parameter "stream" in the server implementation. Without providing the parameter, the connection will not be established. This seems to be something that is not specified by the spec and will prevent to use this library with other sse client implementations. Is it possible to make this parameter optional or completely remove it?
Thanks.