search

restify / node-restify

The future of Node.js REST development
http://restify.com
MIT License
10.71k stars 982 forks source link

Gzip plugin prevents text/event-stream from working #820

Open jameswomack opened 9 years ago

jameswomack commented 9 years ago

I have to call 

      res.handledGzip && res.handledGzip();
      res.removeHeader('Content-Encoding');

in my SSE plugin to bypass the Gzip plugin—otherwise EventSource fails.

The solution would involve adding support for keep alive connection in the Gzip plugin.

yunong commented 9 years ago

Interesting. Can you please give me some sample code I can run to repro?

jameswomack commented 9 years ago

I've created an example in jameswomack/restify-eventsource.

The commit that shows what needs to be bypassed is 26ac2e8ae3ada96743f0d15a8dc4582d035e4c69

The example file is @ https://github.com/jameswomack/restify-eventsource/blob/restification/example.js

retrohacker commented 7 years ago

Marking as unconfirmed until a maintainer is able to reproduce. We appreciate you creating a repro case!

jameswomack commented 7 years ago

@retrohacker thanks

retrohacker commented 7 years ago

SSE looks super slick :smile:

Is this module being used anywhere, and it it stable? Would love to link to it from the website.

The repro case looks like it conflates a lot of the SSE logic with gzip breaking keep-alive connections, I'm going to attempt to make a smaller repro case here soon. Is it fair to say that the core of this issue is our gzip plugin not supporting keep-alive sessions?

jameswomack commented 7 years ago

Is it fair to say that the core of this issue is our gzip plugin not supporting keep-alive sessions?

@retrohacker yes I think you can say that

Is this module being used anywhere, and it it stable? Would love to link to it from the website.

I've only used it on smaller projects and nothing in the past 6 months. It worked well for me, but I can't speak to it beyond that 😄

sppatel commented 5 years ago

I can confirm I am seeing the same behavior and also confirm that the workaround works but is not ideal since we lose compression. The alternative workaround is to wrap the gzip plugin to conditionalize when it gets invoked based on the accept-header.