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

CompressionLayer causes event streams to buffer, breaking them #420

Closed
ekzhang opened this issue Oct 6, 2023 · 2 comments
Closed

CompressionLayer causes event streams to buffer, breaking them #420

ekzhang opened this issue Oct 6, 2023 · 2 comments

Comments

@ekzhang
Copy link

ekzhang commented Oct 6, 2023

Bug Report

When using Server-Sent Events (SSE), a streaming body is sent in real time with type text/event-stream. This is a standard MIME type for many websites, such as notification feeds, or streaming chat completion APIs.

However, if you apply the default CompressionLayer on a site with SSE, it will cause all of the server-sent events to buffer until reaching a given size, before they are able to reach the HTTP client.

I straced a server running hyper over HTTP/1.1, with an event stream outputting messages every 1 second, and it does still write chunks of data to the socket every second. However, the web browser (as well as curl --no-buffer --compressed) is not able to decode the messages individually.

I also confirmed this behavior with other streaming middleware implementations, such as FastAPI's GZipMiddleware. It seems that compression encoding generally doesn't work nicely with text/event-stream.

Version

0.4.4

Platform

Linux ip-172-31-47-18 5.15.0-1045-aws #50~20.04.1-Ubuntu SMP Wed Sep 6 17:29:11 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Crates

compression module with compression-gzip feature

Description

I wanted to ask if it's possible to include

NotForContentType::const_new("text/event-stream")

in the DefaultPredicate for tower_http::compression::CompressionLayer. This would reduce the chance of an issue like this happening for application developers, who commonly use SSE for live event feeds.

Thank you so much for your work on this library.

@BenJeau
Copy link
Contributor

BenJeau commented Feb 13, 2024

Addressed as part of #465

@jplatte
Copy link
Collaborator

jplatte commented Feb 25, 2024

And shippped in 0.5.3.

@jplatte jplatte closed this as completed Feb 25, 2024
athoscouto added a commit to tursodatabase/libsql that referenced this issue Jul 17, 2024
This is caused by a fixed issue on tower-http [1]. We can't update
right now, because it requires us to change too much logic. The work
around is to avoid compressing SSE responses until we update tower.

[1] tower-rs/tower-http#420
github-merge-queue bot pushed a commit to tursodatabase/libsql that referenced this issue Jul 17, 2024
This is caused by a fixed issue on tower-http [1]. We can't update
right now, because it requires us to change too much logic. The work
around is to avoid compressing SSE responses until we update tower.

[1] tower-rs/tower-http#420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants