Let a `rescue_from` handler propagate an exception instead of rendering it

[dduugg]

Since #2703 (3.3.0) an exception raised inside a rescue_from block is caught and redispatched instead of escaping the middleware stack. That fixed #2482, and it also removed the only way an app could let a specific exception class reach Rack middleware mounted above Grape. There's no opt-out today.

The case I hit: Rails' ActiveRecord::Middleware::DatabaseSelector routes a request to a read replica, and a custom config.active_record.database_resolver can catch ActiveRecord::ReadOnlyError and retry the block against the primary. The app re-raised that error from rescue_from(:all) so the resolver upstream could do its job. On 3.3+ it no longer gets there, so the request 500s where it used to recover, and nothing notices, since the redispatch renders an ordinary 500.

env['grape.exception'] (#2855) is enough to report the exception but not to act on it, because the response is already built by then. Raising a non-StandardError does escape, but it bypasses every intervening rescue => e. Dropping rescue_from :all gives up the catch-all for everything else, and #2737 makes excluding a single class from it an ArgumentError.

#2840 made the same call for the rendering case and added Grape.config.raise_rendering_errors to opt back out. I'd like the equivalent for the handler case:

rescue_from ActiveRecord::ReadOnlyError, propagate: true

Verified the behavior above against 4.0.1. Happy to open a PR if the direction seems right.

[dblock]

I think propagate: true is reasonable. This is closely related to the discussion in #2840, where we added raise_rendering_errors so applications could preserve the previous behavior and allow rendering exceptions to reach upstream Rack middleware. This seems like the equivalent escape hatch for exceptions raised inside rescue_from handlers.

How would this interact with rescue_from :all? I’d expect a specific registration to take precedence:

rescue_from ActiveRecord::ReadOnlyError, propagate: true

rescue_from :all do |error|
  # handle everything else
end

ActiveRecord::ReadOnlyError would escape to upstream Rack middleware, while other exceptions would still use the catch-all, independent of declaration order.

I think rescue_from :all, propagate: true should raise ArgumentError, since that is effectively equivalent to not registering :all.

One question: should propagation apply only when this exception is raised from another rescue_from handler, or whenever it reaches Grape? I lean toward always propagating it—the declaration then consistently assigns that exception to upstream middleware.

As a smaller alternative, the closest implementation to raise_rendering_errors would be a global compatibility setting such as:

Grape.configure do |config|
  config.raise_rescue_handler_errors = true
end

The existing rescue in run_rescue_handler could simply re-raise when that setting is enabled instead of calling redispatch. This would restore the pre-3.3 behavior for all exceptions raised inside handlers and would work naturally with rescue_from :all. It is less selective than propagate: true, but it is likely the smallest implementation and the most direct counterpart to raise_rendering_errors.

@dduugg @ericproulx, which direction do you think is best here: the selective propagate: true DSL or the smaller global raise_rescue_handler_errors compatibility setting or both?

@dduugg feel free to implement and open PR(s) for either solution and let's discuss and pick which one(s) we want

Posted by an AI on behalf of @dblock.

[ericproulx]

👋 @dduugg,

I like the propagate since it's a way to have a kind of exception list. If you only have one

  rescue_from ActiveRecord::ReadOnlyError, propagate: true

in your codebase, the propagate: true is appealing but adding multiple propagate: true at multiple places is IMOO annoying and you might forget one. I would keep the propagate: true and add a global exceptions list. Wdyt ?