tower-rs / tower-http

HTTP specific Tower utilities.
675 stars 156 forks source link

feat: add SSE compression exclusion within default predicate #465

Closed BenJeau closed 6 months ago

BenJeau commented 6 months ago

Motivation

Discussed in this Github discussion from Axum's repo:

And had opened a docs PR within Axum's repo to add a note about compression for SSEs:

If not careful or properly handled, general compression with SSE will result in missing some or all events.

Solution

Exclude SSEs from being compressed by checking the content type of text/event-stream within the DefaultPredicate

FrankReh commented 6 months ago

Could the problem have been that the compression buffer isn't flushed for the SSE case?

BenJeau commented 6 months ago

Could the problem have been that the compression buffer isn't flushed for the SSE case?

Potentially, I didn't had the time to investigate but saw that disabling compression resolved all my issues.


I can't close the linked issue https://github.com/tower-rs/tower-http/issues/420 - if someone could, it would be great to cleanup the issues

jplatte commented 6 months ago

I'll try to remember to close it after the fix has shipped in a crates.io release.

ekzhang commented 6 months ago

Thanks for fixing this!

Besides text/event-stream, another candidate content type that involves dynamically pushing data is multipart/x-mixed-replace -- which is used sometimes in image tags: https://wiki.tcl-lang.org/page/multipart%2Fx-mixed-replace#:~:text=multipart%2Fx%2Dmixed%2Dreplace%20is%20an%20HTTP%20content%20type,content%20to%20the%20web%20browser.

(I think that one is a lot less common though)