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

idempotency_key option persisted since v3.4.1 #598

Closed
jurriaan opened this issue Oct 16, 2017 · 3 comments
Closed

idempotency_key option persisted since v3.4.1 #598

jurriaan opened this issue Oct 16, 2017 · 3 comments

Comments

@jurriaan
Copy link

jurriaan commented Oct 16, 2017

We just noticed that some of our payments were failing due to the fact that the same idempotency key was used as in the invoice creation request.

Traced the issue back to this commit:
80d85a5#diff-56800b7863b39756b91e3c43ffed3f51R19
Where the idempotency key was added to the OPTS_KEYS_TO_PERSIST array.

This bug can be triggered with the following code:

invoice = Stripe::Invoice.create(
  { customer: 'cus_foo' },
  { idempotency_key: 'someidempotencykey' }
)
invoice.pay(source: ideal_source.id)

Where before v3.4.1 it will succeed, and after this version it will raise the following error:

Keys for idempotent requests can only be used for the same endpoint they were first used for ('/v1/invoices/in_foobarbaz/pay' vs '/v1/invoices'). Try using a key other than 'someidempotencykey' if you meant to execute a different request. (Stripe::InvalidRequestError)

It seems to me that this change of the OPTS_KEYS_TO_PERSIST is unwanted, since you don't want to reuse the previously used idempotency_key for different requests.

cc @brandur @brandur-stripe

@ob-stripe
Copy link
Contributor

Hey @jurriaan, thanks for the report. I'll let @brandur-stripe take a closer look, but at first glance I think your assessment is correct and the idempotency key should not be persisted.

brandur-stripe pushed a commit that referenced this issue Oct 16, 2017
Excludes `idempotency_key` from opts to persist between API requests.
Obviously the same idempotency key is not something that we ever want to
use again.

Fixes #598.
@brandur-stripe
Copy link
Contributor

Sorry about the trouble here, and thanks for reporting. I've proposed a patch in #599.

Hm, I wonder if it might be worth adding something to stripe-mock so that it would've caught this problem. i.e. A check for recently re-used idempotency keys.

brandur-stripe pushed a commit that referenced this issue Oct 16, 2017
Excludes `idempotency_key` from opts to persist between API requests.
Obviously the same idempotency key is not something that we ever want to
use again.

Fixes #598.
@brandur-stripe
Copy link
Contributor

Fixed released as part of 3.5.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants