Handle ActionDispatch::Response::RackBody as body

[skatkov]

With our new rails 7.1 project, idempotent requests don't seem to be working. They are not even saved.

As it turns out, Idempo middleware receives an instance of ActionDispatch::Response::RackBody as a body. And since it's not Array, we're not saving that body.

I'm not sure exactly why another project based on rails 7.0 keeps working. Because rails started returning ActionDispatch::Response::RackBody pre 7.0 version. I assume, that this could be due to the way how middleware is being inserted (how deep in a stack).

Here is what we do in our application.rb:

config.middleware.insert_after ActionDispatch::Executor, Idempo, backend: Idempo::ActiveRecordBackend.new

And here is an output of rails middleware:

use Rack::Sendfile
use ActionDispatch::Static
use ActionDispatch::Executor
use Idempo
use ActionDispatch::ServerTiming
use ActiveSupport::Cache::Strategy::LocalCache::Middleware
use Rack::Runtime
use Rack::MethodOverride
use ActionDispatch::RequestId
use ActionDispatch::RemoteIp
use Rails::Rack::Logger
use ActionDispatch::ShowExceptions
use WebConsole::Middleware
use ActionDispatch::DebugExceptions
use ActionDispatch::ActionableExceptions
use ActionDispatch::Reloader
use ActionDispatch::Callbacks
use ActiveRecord::Migration::CheckPending
use ActionDispatch::Cookies
use ActionDispatch::Session::CookieStore
use ActionDispatch::Flash
use ActionDispatch::ContentSecurityPolicy::Middleware
use ActionDispatch::PermissionsPolicy::Middleware
use Rack::Head
use Rack::ConditionalGet
use Rack::ETag
use Rack::TempfileReaper
use ActionDispatch::Static
run Banksvc::Application.routes

But that is just a wild guess.

As a fix, I suggest trying converting body to array — this proved effective in our project.

[julik]

The reason why the behavior differs is likely because there was some tweaking of the rack body in Rails due to the changes in Rack 3.x. The issue with calling to_ary explicitly is that it instantly "materializes" the entire Rack body into strings, and to_ary comes from the Rack 3.x protocol (and has Semantics™ attached to it). There are implications to this (look for RackBody changes in Rails and the related PR discussions and https://github.com/appsignal/appsignal-ruby/pull/1037/files#diff-542053b5df2402727a27d180e5f6f63bf8a646b81d72fa0c013de4177a6becafR23 ).

So it is not as much as needing to "convert" something into something else as it is about knowing how large the response is going to be once it gets read out of the body. Can you check why https://github.com/julik/idempo/blob/main/lib/idempo.rb#L118 does not work? Doesn't Rails set content length by itself?

[skatkov]

@julik I actually traced it to a next line, that rejects to save this body -- since body is not an Array.
https://github.com/julik/idempo/blob/main/lib/idempo.rb#L120

[julik]

yes but do response_headers contain a Content-Length of some description in your testing? I would expect Rails sets it but I'm not sure

[franzliedke]

Whew, I got quite scared when I read the gem is not working for you with Rails 7.1. In our project (also Rails 7.1), it does.

We insert the middleware directly in the base controller (a barely-known Rails feature), can you try that out to get closer to the root cause? (You hinted middleware order may play a role.)

class MyRailsController
  use Idempo, backend: Idempo::ActiveRecordBackend.new, other: options
end