From b7f32e6e1d258a38ca2c98faf547d25716888b95 Mon Sep 17 00:00:00 2001 From: Joel Taylor Date: Fri, 25 Oct 2019 14:03:23 -0700 Subject: [PATCH] Raise an error when requests params are invalid 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)` --- lib/stripe/api_operations/request.rb | 9 +++++++++ test/stripe/api_resource_test.rb | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/lib/stripe/api_operations/request.rb b/lib/stripe/api_operations/request.rb index 579da7c95..087732d84 100644 --- a/lib/stripe/api_operations/request.rb +++ b/lib/stripe/api_operations/request.rb @@ -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) @@ -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) + + 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) diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index c0dfbc163..09a5815d0 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -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))