Skip to content
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

How to handle exception raised in rescue_from block? #2482

Open
manueljacob opened this issue Jul 24, 2024 · 4 comments
Open

How to handle exception raised in rescue_from block? #2482

manueljacob opened this issue Jul 24, 2024 · 4 comments
Labels

Comments

@manueljacob
Copy link
Contributor

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
Copy link
Contributor

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

@ericproulx
Copy link
Contributor

Does #2483 is solving it ?

@manueljacob
Copy link
Contributor Author

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
Copy link
Member

dblock commented Jul 25, 2024

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

@dblock dblock added the question label Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants