Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce new IdempotencyError type #613

Merged
merged 2 commits into from
Dec 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ Metrics/AbcSize:
# Offense count: 27
# Configuration parameters: CountComments, ExcludedMethods.
Metrics/BlockLength:
Max: 455
Max: 467

# Offense count: 8
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 583
Max: 595

# Offense count: 10
Metrics/CyclomaticComplexity:
Expand Down
5 changes: 5 additions & 0 deletions lib/stripe/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ def initialize(message, param, code, http_status: nil, http_body: nil, json_body
end
end

# IdempotencyError is raised in cases where an idempotency key was used
# improperly.
class IdempotencyError < StripeError
end

# InvalidRequestError is raised when a request is initiated with invalid
# parameters.
class InvalidRequestError < StripeError
Expand Down
62 changes: 28 additions & 34 deletions lib/stripe/stripe_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -280,53 +280,47 @@ def handle_error_response(http_resp, context)
def specific_api_error(resp, error_data, context)
Util.log_error("Stripe API error",
status: resp.http_status,
error_code: error_data["code"],
error_message: error_data["message"],
error_param: error_data["param"],
error_type: error_data["type"],
error_code: error_data[:code],
error_message: error_data[:message],
error_param: error_data[:param],
error_type: error_data[:type],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should've been symbols (not strings) all along, but because the problem was duplicated in the tests, I never noticed because everything passed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

idempotency_key: context.idempotency_key,
request_id: context.request_id)

# The standard set of arguments that can be used to initialize most of
# the exceptions.
opts = {
http_body: resp.http_body,
http_headers: resp.http_headers,
http_status: resp.http_status,
json_body: resp.data,
}

case resp.http_status
when 400, 404
error = InvalidRequestError.new(
error_data[:message], error_data[:param],
http_status: resp.http_status, http_body: resp.http_body,
json_body: resp.data, http_headers: resp.http_headers
)
case error_data[:type]
when "idempotency_error"
IdempotencyError.new(error_data[:message], opts)
else
InvalidRequestError.new(
error_data[:message], error_data[:param],
opts
)
end
when 401
error = AuthenticationError.new(
error_data[:message],
http_status: resp.http_status, http_body: resp.http_body,
json_body: resp.data, http_headers: resp.http_headers
)
AuthenticationError.new(error_data[:message], opts)
when 402
error = CardError.new(
CardError.new(
error_data[:message], error_data[:param], error_data[:code],
http_status: resp.http_status, http_body: resp.http_body,
json_body: resp.data, http_headers: resp.http_headers
opts
)
when 403
error = PermissionError.new(
error_data[:message],
http_status: resp.http_status, http_body: resp.http_body,
json_body: resp.data, http_headers: resp.http_headers
)
PermissionError.new(error_data[:message], opts)
when 429
error = RateLimitError.new(
error_data[:message],
http_status: resp.http_status, http_body: resp.http_body,
json_body: resp.data, http_headers: resp.http_headers
)
RateLimitError.new(error_data[:message], opts)
else
error = APIError.new(
error_data[:message],
http_status: resp.http_status, http_body: resp.http_body,
json_body: resp.data, http_headers: resp.http_headers
)
APIError.new(error_data[:message], opts)
end

error
end

# Attempts to look at a response's error code and return an OAuth error if
Expand Down
25 changes: 20 additions & 5 deletions test/stripe/stripe_client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,10 @@ class StripeClientTest < Test::Unit::TestCase
}
Util.expects(:log_error).with("Stripe API error",
status: 500,
error_code: error["code"],
error_message: error["message"],
error_param: error["param"],
error_type: error["type"],
error_code: error[:code],
error_message: error[:message],
error_param: error[:param],
error_type: error[:type],
idempotency_key: nil,
request_id: nil)

Expand Down Expand Up @@ -411,7 +411,22 @@ class StripeClientTest < Test::Unit::TestCase
assert_equal 'Invalid response object from API: "{\"bar\":\"foo\"}" (HTTP response code was 500)', e.message
end

should "raise InvalidRequestError on 400" do
should "raise IdempotencyError on 400 of type idempotency_error" do
data = make_missing_id_error
data[:error][:type] = "idempotency_error"

stub_request(:post, "#{Stripe.api_base}/v1/charges")
.to_return(body: JSON.generate(data), status: 400)
client = StripeClient.new

e = assert_raises Stripe::IdempotencyError do
client.execute_request(:post, "/v1/charges")
end
assert_equal(400, e.http_status)
assert_equal(true, e.json_body.is_a?(Hash))
end

should "raise InvalidRequestError on other 400s" do
stub_request(:post, "#{Stripe.api_base}/v1/charges")
.to_return(body: JSON.generate(make_missing_id_error), status: 400)
client = StripeClient.new
Expand Down