Skip to content

Commit

Permalink
Raise an error when requests params are invalid (#874)
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 authored and ob-stripe committed Oct 31, 2019
1 parent 4e564a1 commit 299e9ea
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
11 changes: 11 additions & 0 deletions lib/stripe/api_operations/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ module APIOperations
module Request
module ClassMethods
def request(method, url, params = {}, opts = {})
params ||= {}

error_on_invalid_params(params)
warn_on_opts_in_params(params)

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

private def error_on_invalid_params(params)
return if params.nil? || 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
16 changes: 16 additions & 0 deletions test/stripe/api_resource_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,22 @@ 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 "allow making a request with params set to nil" do
stub_request(:post, "#{Stripe.api_base}/v1/charges/ch_123/capture")
.to_return(body: JSON.generate(charge_fixture))

Stripe::Charge.capture("ch_123", nil, "sk_test_secret")
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 299e9ea

Please sign in to comment.