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

Exceptions thrown in the formatter should be catchable by rescue_from :all #359

Closed
lucaspiller opened this issue Mar 8, 2013 · 3 comments · Fixed by #378
Closed

Exceptions thrown in the formatter should be catchable by rescue_from :all #359

lucaspiller opened this issue Mar 8, 2013 · 3 comments · Fixed by #378

Comments

@lucaspiller
Copy link

I added a basic error handler using rescue_from :all that logs exceptions, notifies Airbrake and returns a basic 500 response. Unfortunately this doesn't work as described in the documentation, and this isn't called for all exceptions.

We had an issue with a decorator not being found, and so the formatter (we are using Rabl) was throwing an exception. This was caught here, and thrown again:

lib/grape/middleware/formatter.rb

This was caught in the error middleware and passed directly to error_response with no options for logging or changing it (other than by hacking around with error_formatter):

lib/grape/middleware/error.rb

In the end I changed the error middleware to raise an exception for this exception (...) which isn't great, but it allows me to return consistent errors when something goes wrong. It would be great if rescue_from :all worked as described, and allowed you to rescue from all exceptions 😄

@dblock
Copy link
Member

dblock commented Mar 8, 2013

I think it's a real problem. Would be amazing if you could write a test that reproduces this, to start.

@kbarrette
Copy link
Contributor

I've just run afoul of this as well - I have a entity that was exposing a method that raised an exception, and rather than generating a backtrace in the logs, etc, Grape was just swallowing the exception here https://github.com/intridea/grape/blob/master/lib/grape/middleware/formatter.rb#L31 and returning an error response via https://github.com/intridea/grape/blob/master/lib/grape/middleware/error.rb#L25

It seems wrong to have the formatter middleware blindly rescue all exceptions. Looks like this was part of the fix for #309 - how about having the XML formatter raise a more specific error and catching just that?

@dblock
Copy link
Member

dblock commented Apr 14, 2013

This was resolved.

@dblock dblock closed this as completed Apr 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants