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

Error Middleware Rescuable Doesn't Handle Inheritance #542

Closed
xevix opened this issue Dec 29, 2013 · 3 comments
Closed

Error Middleware Rescuable Doesn't Handle Inheritance #542

xevix opened this issue Dec 29, 2013 · 3 comments

Comments

@xevix
Copy link
Contributor

xevix commented Dec 29, 2013

Suppose you have an error hierarchy as below:

module APIErrors
  class ParentError < StandardError; end
  class Child < ParentError; end
end

In the API file, suppose you then attempt to rescue from all errors in a standard way:

rescue_from APIErrors::ParentError do |e|
  Rack::Response.new({
    error: "#{e.class} error",
    message: e.message
  }.to_json, e.status)
end

Now suppose a Child error is raised:

raise APIErrors::Child, "Rescue me, please!"

Because the middleware checks only for a list of classes in rescuable?(klass) and the Child class is not included in the list, it will never rescue this.

Is there a way to have all ParentError-derivative classes be rescued?

@dblock
Copy link
Member

dblock commented Dec 30, 2013

You can always rescue :all and re-raise an error. But I think we should extend the behavior of rescue_from to allow your scenario. I am not sure what syntax it takes, but I would take a pull request that makes it possible without breaking existing functionality.

@xevix
Copy link
Contributor Author

xevix commented Dec 30, 2013

Ah, indeed, my fault. My example should have included also other rescue_from for other exceptions such as Grape::Exceptions::ValidationErrors, the idea being to separate different classes of errors. This would be specifically for errors of application-level origin (the API).

Awesome, I'll work on a pull request to enable this.

@dblock
Copy link
Member

dblock commented Jan 3, 2014

Closing, fixed in 0.7.0.

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

No branches or pull requests

2 participants