ruby-grape / grape

An opinionated framework for creating REST-like APIs in Ruby.
http://www.ruby-grape.org
MIT License
9.88k stars 1.22k forks source link

How to handle exception raised in `rescue_from` block? #2482

Open manueljacob opened 1 month ago

manueljacob commented 1 month ago

The purpose of a rescue_from block is of course to handle exceptions in a custom way. However, what should happen if the rescue_from block itself raises an exception? Currently, Grape does not handle such exceptions, likely resulting in an Internal Server Error returned by the web server.

In some cases, it would be beneficial if an exception raised inside a rescue_from block was handled by another rescue_from block. However, it’s unclear whether that’s a good idea in general, as it could result in an infinite rescue_from block loop.

I have a use case where Grape::Exceptions::ValidationErrors should be remapped to a different exception that is a subclass of Grape::Exceptions::Base. What I currently do is the following:

rescue_from Grape::Exceptions::ValidationErrors do |e|
  begin
    raise Api::Exceptions::InvalidValueError, e.full_messages
  rescue Api::Exceptions::InvalidValueError => invalid_value_error
    run_rescue_handler(:error_response, invalid_value_error)
  end
end

However, it no longer works with newer Grape versions (probably since #2377).

For this use case, it would be best if it was possible to raise an exception in the rescue_from block that is then handled by the default rescue handler for Grape exceptions. If that’s an option, I can try to come up with sensible semantics that allow that while avoiding infinite loops.

Otherwise, I’d welcome suggestions for handling this use case cleanly and such that it works with current Grape versions. The best I’ve come up with is to call #error!, however it can be a bit fiddly to convert the exception to the correct arguments for #error!. Note that this requires a recent fix (#2471).

ericproulx commented 1 month ago

If you throw :error the code will handle the case but not a raise. It's quite easy to handle it.

ericproulx commented 1 month ago

Does #2483 is solving it ?

manueljacob commented 1 month ago

If you throw :error the code will handle the case but not a raise. It's quite easy to handle it.

Putting throw :error, invalid_value_error in the rescue block in the above example doesn’t work with or without #2483. It causes the middleware to return an Invalid Response error. The line error_response(response) is not executed in this case because #error? returns false for non-Hashes.

Does #2483 is solving it ?

In case of an exception in the rescue_from block, with your code, it passes the exception to #default_rescue_handler which always returns HTTP error code 500. If it called #error_response for subclasses of Grape::Exceptions::Base, it would work.

dblock commented 1 month ago

Looking at #2483 I think we need more tests, @manueljacob care to add one to that PR that demonstrates what you describe?