Skip to content

Commit

Permalink
Raise an error when requests params are invalid
Browse files Browse the repository at this point in the history
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)`
  • Loading branch information
joeltaylor committed Oct 25, 2019
1 parent 4e564a1 commit 0509573
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 0 deletions.
8 changes: 8 additions & 0 deletions lib/stripe/api_operations/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module APIOperations
module Request
module ClassMethods
def request(method, url, params = {}, opts = {})
error_on_invalid_params(params)
warn_on_opts_in_params(params)

opts = Util.normalize_opts(opts)
Expand Down Expand Up @@ -47,6 +48,13 @@ def request(method, url, params = {}, opts = {})
end
end

private def error_on_invalid_params(params)
return if params.is_a?(Hash)

raise ArgumentError,
"request params should be either a Hash or nil (was a #{params.class})"
end

private def warn_on_opts_in_params(params)
Util::OPTS_USER_SPECIFIED.each do |opt|
if params.key?(opt)
Expand Down
9 changes: 9 additions & 0 deletions test/stripe/api_resource_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,15 @@ class NestedTestAPIResource < APIResource
end
end

should "error if the params is not a Hash" do
stub_request(:post, "#{Stripe.api_base}/v1/charges/ch_123/capture")
.to_return(body: JSON.generate(charge_fixture))

e = assert_raises(ArgumentError) { Stripe::Charge.capture("ch_123", "sk_test_secret") }

assert_equal "request params should be either a Hash or nil (was a String)", e.message
end

should "error if a user-specified opt is given a non-nil non-string value" do
stub_request(:post, "#{Stripe.api_base}/v1/charges")
.to_return(body: JSON.generate(charge_fixture))
Expand Down

0 comments on commit 0509573

Please sign in to comment.