-
Notifications
You must be signed in to change notification settings - Fork 619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Server-sent events support #129
Comments
I didn't actually intend to, but seem to have accidentally opened a pull-request (#130) nonetheless. |
:) no worries. Contributions are very welcome. SSE vaguely rings a bell but very, very far in the back of my head. This looks simple enough but I just need to figure out how to expose this. Let me read up on SSE and determine whether I can make this automatic by detecting it through the |
This patch adds a proxy.flushinterval option which enables periodic flushing of the repsonse buffer. Since this is a global option and the implications on non-streaming HTTP requests are unknown the current implementation limits the use of the flush interval to requests where the 'Accept' header is set to 'text/event-stream' which identifies SSE requests. This is really a route specific option and should be configured as such once this becomes possible.
@madeddie: Since github doesn't allow me to make changes to your PR I've created a new one which makes the option configurable and just extends the existing HTTP handler. Could you please test this by checking out this branch and running OTOH, I'm not sure if for SSE requests there should always be a non-zero flush interval since it otherwise doesn't seem to work. Need to think about that. |
Original PR #130 by @madeddie This patch adds a proxy.flushinterval option which enables periodic flushing of the repsonse buffer. Since this is a global option and the implications on non-streaming HTTP requests are unknown the current implementation limits the use of the flush interval to requests where the 'Accept' header is set to 'text/event-stream' which identifies SSE requests. This is really a route specific option and should be configured as such once this becomes possible.
Added attribution to your PR. |
Awesome, looks like what I need! Thanks. I've closed my PR. |
It works like expected. There might be more situations where people want in-between flushes as well as configurable keepalive etc, so I think it's a good idea to keep it in the scope of #111 |
I think I'm going to change the behavior which makes SSE work out-of-the box, e.g. set a default flush interval for auto-detected SSE connections. fabio's goal is to be zero-conf. Will do a bit more digging if a |
I've set it to 1s in our env (we use Fabio both outside to container as well as container to container, so the time adds up sometimes) to keep the connection fast. We don't have a lot of users, so, no idea about any potential problems with load or anything. The idea of SSE is that any event is separated from others by an empty line (so 2 line returns). So the strictest way of doing this is by flushing buffer on empty line, but I have no clue how to do that :) (it'll probably mean rewriting some basis net/http methods, so; non-trivial). In short, as long as there are no obvious performance penalties, 1s for SSE buffer flush is an ok default, as long as it's specifically mentioned in the docs somewhere. |
I've set the default to |
Original PR fabiolb#130 by @madeddie This patch adds a proxy.flushinterval option which enables periodic flushing of the repsonse buffer. Since this is a global option and the implications on non-streaming HTTP requests are unknown the current implementation limits the use of the flush interval to requests where the 'Accept' header is set to 'text/event-stream' which identifies SSE requests. This is really a route specific option and should be configured as such once this becomes possible.
Original PR fabiolb#130 by @madeddie This patch adds a proxy.flushinterval option which enables periodic flushing of the repsonse buffer. Since this is a global option and the implications on non-streaming HTTP requests are unknown the current implementation limits the use of the flush interval to requests where the 'Accept' header is set to 'text/event-stream' which identifies SSE requests. This is really a route specific option and should be configured as such once this becomes possible.
Original PR fabiolb#130 by @madeddie This patch adds a proxy.flushinterval option which enables periodic flushing of the repsonse buffer. Since this is a global option and the implications on non-streaming HTTP requests are unknown the current implementation limits the use of the flush interval to requests where the 'Accept' header is set to 'text/event-stream' which identifies SSE requests. This is really a route specific option and should be configured as such once this becomes possible.
Original PR fabiolb#130 by @madeddie This patch adds a proxy.flushinterval option which enables periodic flushing of the repsonse buffer. Since this is a global option and the implications on non-streaming HTTP requests are unknown the current implementation limits the use of the flush interval to requests where the 'Accept' header is set to 'text/event-stream' which identifies SSE requests. This is really a route specific option and should be configured as such once this becomes possible.
Any concerns about detecting SSE by response instead of request headers, like when the server is sending "Content-Type: text/event-stream"? Afaik you can't set accept headers in browsers without js. |
@hamann Since the only difference in behavior is the https://github.com/fabiolb/fabio/blob/master/proxy/http_proxy.go#L133-L139 When the response comes back I can inspect the headers but then the request is handled and the response is returned. Furthermore, if the There is no way to keep state between requests. Also, your next request may end up on a different fabio instance. What is preventing you from setting the |
Ok, thanks for clarification. I asked, because we have a resource which we open in the browser (without any js or html at all) and get server-sent events (like content from log files). That stopped working as we switched from haproxy to fabio, but I guess it should be solveable with a few lines of js. |
We're hosting some containers making use of server-sent events EventSource and noticed this doesn't work with Fabio.
If you're unfamiliar with SSE, it's comparable to long polling and is a form of HTTP server push.
The problem is that the default ReverseProxy implementation of Go doesn't flush its buffer while the request is still open.
I'm somewhat sure this is not all to it, but if I simply add a FlushInterval to the ReverseProxy instance, the application starts working as expected.
I've made a quick hack here, please let me know if this is something you might want to add Fabio.
master...madeddie:server_sent_events
The text was updated successfully, but these errors were encountered: