Raise an error when requests params are invalid #874
Conversation
0509573 to
b7f32e6
Compare
|
I played around with a few different ideas to make a helpful error message. Originally, I wanted to incorporate something about the method signature in the error, but I couldn't find a good universal approach. |
|
@ob-stripe It looks like Joel and yourself were already discussing this in #855, so if it's okay, I'm going to leave review to you. For what it's worth, seems fine to me — can't go wrong with better error messages. |
lib/stripe/api_operations/request.rb
Outdated
| @@ -47,6 +48,14 @@ def request(method, url, params = {}, opts = {}) | |||
| end | |||
| end | |||
|
|
|||
| private def error_on_invalid_params(params) | |||
| return if params.is_a?(Hash) | |||
There was a problem hiding this comment.
Shouldn't this check for nil too, since we accept it as a valid value?
| return if params.is_a?(Hash) | |
| return if params.nil? || params.is_a?(Hash) |
There was a problem hiding this comment.
Whoops, yeah there should be a nilcheck there. I'll add a spec for that, too.
There was a problem hiding this comment.
@ob-stripe Actually, it looks like passing nil breaks warn_on_opts_in_params since it assumes params will respond to #key?.
I can add something like params ||= {} before the error/warn checks?
There was a problem hiding this comment.
Yeah, that sounds reasonable.
There was a problem hiding this comment.
Cool, just pushed up the addition with a regression spec.
There are two kinds of API operations: collection and element specific.
The signature between the two is slightly different:
- **collection**: (params, opts)
- **element specific**: (id, params, opts)
If a user doesn't realize the difference, they may attempt to use the
collection signature when performing an element specific operation like:
```
Stripe::PaymentIntent.cancel('pi_1234', 'sk_test_key')
# Results in an error: NoMethodError: undefined method `key?' for "sk_test"
```
The resulting error message isn't very useful for debugging.
Instead,this PR adds a message letting the user know what it's expecting:
`request params should be either a Hash or nil (was a String)`
b7f32e6 to
e889646
Compare
There are two kinds of API operations: collection and element specific.
The signature between the two is slightly different:
If a user doesn't realize the difference, they may attempt to use the
collection signature when performing an element specific operation like:
The resulting error message isn't very useful for debugging.
Instead,this PR adds a message letting the user know what it's expecting:
request params should be either a Hash or nil (was a String)Addresses #855