-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add support to keep open streaming connections with Puma #1858
Conversation
1b66c5b
to
cf969dc
Compare
Falcon doesn't buffer the response body. Are you using Rack 3? Can you print out the middleware stack? Can you run with falcon verbose mode to see how the output body is being handled? |
@ioquatix ah I think I see the issue. It looks like request that establishes the stream (which creates an infinite loop to hold the connection) prevents the server from handling anymore requests. |
This PR is not using Rack 3 @ioquatix (I'm working on it in #1857) – if we're talking Rack 3, would it be easier to get our tests and example working with Puma/Falcon if we adopted "callable body" (rack/rack#1745, puma/puma#2740)? I need to play with this myself, and catch up on everything discussed, rack/rack#1600 also has a fair bit of discussion, still have a big bunch unread emails from rack/rack from last year and summer 2021 😅 |
@dentarg you should consult the upgrade guide if you want to see a summary of all changes that affect applications/middlewares): https://github.com/rack/rack/blob/main/UPGRADE-GUIDE.md Yes you should rely on callable bodies as this is the correct way to implement streaming with Rack. Everything else is a hack and may have unintended buffering. |
a155406
to
a8ec48d
Compare
when 'HTTP' | ||
'net/http/server' | ||
when 'puma' | ||
'puma/rack/handler' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests output show puma is not installed, skipping integration tests
so I think this require is not working as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh good catch 😬 Explains why things went so smooth 😆
@ioquatix I also tried to test this with Falcon as well, but it didn't seem to work. Does Falcon buffer the response?
This is somewhat modeled after streaming in Roda: https://github.com/jeremyevans/roda/blob/c919ac9c7e88f159f8c6185b0bdb5a9311e24d93/lib/roda/plugins/streaming.rb#L143