Skip to content
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

Closed
madeddie opened this issue Jul 20, 2016 · 12 comments
Closed

Server-sent events support #129

madeddie opened this issue Jul 20, 2016 · 12 comments
Milestone

Comments

@madeddie
Copy link

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

@madeddie
Copy link
Author

I didn't actually intend to, but seem to have accidentally opened a pull-request (#130) nonetheless.

@magiconair
Copy link
Contributor

:) 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 Accept header (like Websockets) or whether there is some other mechanism. I'll add this to the list of things that need to be configured through #111

magiconair added a commit that referenced this issue Jul 20, 2016
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.
@magiconair
Copy link
Contributor

@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 go install? It should behave the same way as your patch as long as you configure the proxy.flushinterval to a value > 0s.

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.

magiconair added a commit that referenced this issue Jul 20, 2016
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.
@magiconair
Copy link
Contributor

Added attribution to your PR.

@madeddie
Copy link
Author

madeddie commented Jul 20, 2016

Awesome, looks like what I need! Thanks. I've closed my PR.
Ow and will test in the morning and report.

@madeddie
Copy link
Author

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

@magiconair
Copy link
Contributor

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 1s flush interval for SSE connections is a sane default. Opinions are welcome.

@madeddie
Copy link
Author

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.

magiconair added a commit that referenced this issue Jul 30, 2016
Original PR #130 by @madeddie

Add a proxy.flushinterval option which enables periodic
flushing of the repsonse buffer for SSE connections which
have the 'Accept' header set to 'text/event-stream'.

This is really a route specific option and should be
configured as such once this becomes possible.
@magiconair
Copy link
Contributor

I've set the default to 1s and merged it to master.

madeddie pushed a commit to madeddie/fabio that referenced this issue Aug 4, 2016
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.
madeddie pushed a commit to LibertyGlobal/fabio that referenced this issue Aug 4, 2016
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.
madeddie pushed a commit to LibertyGlobal/fabio that referenced this issue Aug 4, 2016
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.
ptqa pushed a commit to LibertyGlobal/fabio that referenced this issue Jan 13, 2017
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.
@hamann
Copy link

hamann commented Jul 14, 2017

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.

@magiconair
Copy link
Contributor

@hamann Since the only difference in behavior is the FlushInterval to be non-zero I fail to see how this would work?

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 FlushInterval is not set then you may have to wait for the full response to arrive before you can even see the response headers.

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 Accept header?

@hamann
Copy link

hamann commented Jul 17, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants