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

Add all missing error codes #897

Merged
merged 1 commit into from
Jul 25, 2019
Merged

Add all missing error codes #897

merged 1 commit into from
Jul 25, 2019

Conversation

remi-stripe
Copy link
Contributor

Fixes #896

r? @ob-stripe
cc @stripe/api-libraries

Copy link
Contributor

@ob-stripe ob-stripe left a comment

Choose a reason for hiding this comment

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

lgtm except the two codes that are defined twice.

error.go Outdated

// The following error codes can be returned though are undocumented
ErrorCodeIncorrectCVC ErrorCode = "incorrect_cvc"
ErrorCodeInvalidCVC ErrorCode = "invalid_cvc"
Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea how this is even working, but both ErrorCodeIncorrectCVC and ErrorCodeInvalidCVC are already defined (with the same values) above.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hah, the case is different on "Cvc" (versus "CVC"). Might be a good idea to change the ones above to upper too.

@remi-stripe remi-stripe force-pushed the remi-add-error-codes branch from ea782af to 4887a3f Compare July 18, 2019 17:39
@ob-stripe ob-stripe assigned remi-stripe and unassigned ob-stripe Jul 18, 2019
@remi-stripe remi-stripe force-pushed the remi-add-error-codes branch from 4887a3f to fb83880 Compare July 18, 2019 17:53
@remi-stripe
Copy link
Contributor Author

Re-assigning to @brandur-stripe as we never finished that one

@remi-stripe remi-stripe force-pushed the remi-add-error-codes branch from da68337 to 16139b5 Compare July 25, 2019 16:29
ErrorCodeResourceMissing ErrorCode = "resource_missing"
ErrorCodeRoutingNumberInvalid ErrorCode = "routing_number_invalid"
ErrorCodeSecretKeyRequired ErrorCode = "secret_key_required"
ErrorCodeSepaUnsupportedAccount ErrorCode = "sepa_unsupported_account"
Copy link
Contributor

Choose a reason for hiding this comment

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

Last one Remi: I think we want to go with "SEPA" instead of "Sepa" here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use Sepa everywhere else in the library though for SepaDebit. I don't think we should change that one but happy to make the change if you prefer on that one

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, k. I looked around online and I think "SEPA" is definitely the overall preferred casing (and use in Stripe's docs, etc. too).

That said, given we have pre-existing Sepa in stripe-go, I'm good just keeping it like this for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm unsure on that one, I think both would make sense so deferring to you but happy to make the change

Copy link
Contributor

Choose a reason for hiding this comment

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

K cool. Easiest to go with "Sepa" for now then, so just leave it. Thanks!

@brandur-stripe
Copy link
Contributor

One more casing issue, but other than that LGTM! Love how comprehensive this list is.

ptal @remi-stripe

@brandur-stripe brandur-stripe merged commit 1a8cbd6 into master Jul 25, 2019
@brandur-stripe brandur-stripe deleted the remi-add-error-codes branch July 25, 2019 18:24
@brandur-stripe
Copy link
Contributor

Released as 61.20.0.

@layby42
Copy link
Contributor

layby42 commented Jul 25, 2019

invoice_payment_intent_requires_action (?)

@remi-stripe
Copy link
Contributor Author

@layby42 Would you have a bit more details than just this code? It's not listed in https://stripe.com/docs/error-codes today which is why it wasn't added. I'd recommend flagging to our support team if it's a code you rely on: https://support.stripe.com/contact

@layby42
Copy link
Contributor

layby42 commented Jul 25, 2019

@remi-stripe happens when you try to pay invoice (Invoices.Pay) for "incomplete" subscription with card payment method (stripe.InvoicePayParams -> PaymentMethod) that requires 3D Secure authorization on attempt to charge.

Also (not directly related) would be nice to get Invoice from stripe.Error (the way it is possible to get PaymentIntent or PaymentMethod) to not re-request if from Stripe.

@remi-stripe
Copy link
Contributor Author

Also (not directly related) would be nice to get Invoice from stripe.Error (the way it is possible to get PaymentIntent or PaymentMethod) to not re-request if from Stripe.
Yeah we've been thinking about this internally too but no short term plan to support this unfortunately.

nadaismail-stripe pushed a commit that referenced this pull request Oct 18, 2024
* updated parser to always push in prebilling field to subscription schedule object

* updated parser to always push in prebilling field to subscription schedule object

* added warning section

* added changes to openapi spec

* added test

* added tests

* stripe billing is behaving properly for backdated orders now

* fixed backdated logic

* addressed feedback

Co-authored-by: arnoldezeolisa <[email protected]>
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

Successfully merging this pull request may close these issues.

ErrorCode doesn't cover all of codes listed in official doc
4 participants