Fix streaming responses with Rack middleware #1445
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
unflxw
added
the
bug
|
✔️ All good! |
unflxw
force-pushed
the
fix-streaming-responses-rack-middleware
branch
3 times, most recently
from
d9ca3f1 to
ff845a6
Compare
tombruijn
approved these changes
Aug 28, 2025
unflxw
force-pushed
the
fix-streaming-responses-rack-middleware
branch
from
ff845a6 to
a6fe0f4
Compare
The `Rack::Events` middleware is not compatible with streaming bodies. It wraps the body in a `Rack::Events::EventedBodyProxy`, which unlike a `Rack::BodyProxy`, always responds to `#each`, even if the body it is proxying does not respond to `#each`. According to the Rack spec, when a body responds to both `#each` and `#call`, the former is preferred, ultimately causing a `NoMethodError` on the original body. Deprecate the use of `Rack::Events` for our event-based Rack instrumentation. Implement a modified `Appsignal::Rack::Events` which wraps the body in a `Rack::BodyProxy`, ensuring it does not respond to `#each` when the body it's proxying does not respond to `#each`. Since `Appsignal::Rack::Events`' modifications break the `on_send` event, we do not recommend its direct usage. Instead, we provide a convenience `Appsignal::Rack::EventMiddleware` wrapper, which uses our modified events middleware with our event handler. A deprecation warning is emitted when `Appsignal::Rack::EventMiddleware` is used with `Rack::Events`.
unflxw
force-pushed
the
fix-streaming-responses-rack-middleware
branch
from
a6fe0f4 to
c94315e
Compare
unflxw
added a commit
that referenced
this pull request
Sep 2, 2025
A fix for the issue where `::Rack::Events` breaks streaming bodies was introduced in version 3.2.1. Define `Appsignal::Rack::Events`, our patched implementation, only when the Rack version present is below 3.2.1. Otherwise, alias it to `::Rack::Events`. Modify how the deprecation warning is silenced, doing so through `Appsignal::Rack::EventMiddleware`, not `Appsignal::Rack::Events`. Test the deprecation warning's behaviour more precisely by testing the behaviour of `Appsignal::Rack::EventHandler` via the middleware that it is initialised, rather than in isolation. See #1445, rack/rack#2373, rack/rack#2375 for context.
unflxw
added a commit
that referenced
this pull request
Sep 3, 2025
A fix for the issue where `::Rack::Events` breaks streaming bodies was introduced in version 3.2.1. Define `Appsignal::Rack::Events`, our patched implementation, only when the Rack version present is below 3.2.1. Otherwise, alias it to `::Rack::Events`. Modify how the deprecation warning is silenced, doing so through `Appsignal::Rack::EventMiddleware`, not `Appsignal::Rack::Events`. Test the deprecation warning's behaviour more precisely by testing the behaviour of `Appsignal::Rack::EventHandler` via the middleware that it is initialised, rather than in isolation. See #1445, rack/rack#2373, rack/rack#2375 for context.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
Rack::Eventsmiddleware is not compatible with streaming bodies. It wraps the body in aRack::Events::EventedBodyProxy, which unlike aRack::BodyProxy, always responds to#each, even if the body it is proxying does not respond to#each.According to the Rack spec, when a body responds to both
#eachand#call, the former is preferred, ultimately causing aNoMethodErroron the original body.Deprecate the use of
Rack::Eventsfor our event-based Rack instrumentation. Implement a modifiedAppsignal::Rack::Eventswhich wraps the body in aRack::BodyProxy, ensuring it does not respond to#eachwhen the body it's proxying does not respond to#each.Since
Appsignal::Rack::Events' modifications break theon_sendevent, we do not recommend its direct usage. Instead, we provide a convenienceAppsignal::Rack::EventMiddlewarewrapper, which uses our modified events middleware with our event handler.A deprecation warning is emitted when
Appsignal::Rack::EventMiddlewareis used withRack::Events.Fixes #1442.