Allow to pass method name to :with option of rescue_from #1257
Conversation
| end | ||
|
|
||
| rescue_from :all, with: :server_error! | ||
| rescue_from ArgumentError, with: Rack::Response.new('rescued with a method', 400) |
There was a problem hiding this comment.
I think with: :method is very useful, however a rescue with a Rack::Response is probably dangerous, because the instance of the Rack::Response is constructed at load time. This tends to cause all kinds of wonderful behavior like memory leaks, people are reckless with them. I think the above should not be allowed, you should do a -> () { Rack...} when you want to return something like this.
There was a problem hiding this comment.
Yeah, you're right. But the dangerous behaviour is expected.
Should I remove that and introduce lambda approach?
There was a problem hiding this comment.
I think that yes. You should either use a symbol or a proc here.
namusyaka
force-pushed
the
implement-with-option
branch
from
30fb7c7 to
114cc10
Compare
|
Force pushed. |
| @@ -10,6 +10,8 @@ | |||
| * [#1243](https://github.com/ruby-grape/grape/pull/1243): Add `header` support for middleware - [@namusyaka](https://github.com/namusyaka). | |||
| * [#1252](https://github.com/ruby-grape/grape/pull/1252): Allow default to be a subset or equal to allowed values without raising IncompatibleOptionValues - [@jeradphelps](https://github.com/jeradphelps). | |||
| * [#1255](https://github.com/ruby-grape/grape/pull/1255): Allow param type definition in route_param - [@namusyaka](https://github.com/namusyaka) | |||
| * [#1257](https://github.com/ruby-grape/grape/pull/1257): Allow to pass method name to :with option of rescue_from - [@namusyaka](https://github.com/namusyaka) | |||
There was a problem hiding this comment.
We didn't have with: before, right? So both of these are the same change, which is "Added rescue_from with: ..." that can take a method name or a proc.
There was a problem hiding this comment.
Nevermind, we did, it's early in the AM ;)
|
One last thing we need a test (and possibly a fix for) is when you mix a block and rescue_from ArgumentError, with: error_handler do | ... |
# is this called?
end |
namusyaka
force-pushed
the
implement-with-option
branch
from
114cc10 to
2575bc1
Compare
|
Done. Thank you for your review! |
|
@dblock Let me know if you have any problems. |
|
merged via 110c4f1. I just edited the CHANGELOG a bit. Nice work @namusyaka, thanks! |
|
Thanks! |
Closes #790
I think the feature is useful for writing error handling without block.
Thoughts?