Introduce new Idempotency error class
#436
Conversation
| @@ -114,6 +116,9 @@ private static function _specificAPIError($rbody, $rcode, $rheaders, $resp, $err | |||
| if ($code == 'rate_limit') { | |||
| return new Error\RateLimit($msg, $param, $rcode, $rbody, $resp, $rheaders); | |||
| } | |||
| if ($type == 'idempotency_error') { | |||
There was a problem hiding this comment.
Sorry about the usual pedantry, but it kind of looks like the other conditional ladders for error interpretation in this file are alphabetized, and it might be worth giving this the same treatment. (Also, might want to change this to a switch like the others? It doesn't matter that much though.)
There was a problem hiding this comment.
Do you mean grouping rate_limit and idempotency_error in a switch statement? The problem with that is that the two values are in different variables ($code vs $type).
I've taken a look at how we handle this in other libraries: stripe-ruby completely dropped support for 400 rate_limit errors, which maybe we should do in other libraries as well, but that would be a breaking change and we just released 6.0.0 :/
stripe-python uses a series of semi-complex if statements: https://github.com/stripe/stripe-python/blob/master/stripe/api_requestor.py#L190-L214, but then again Python doesn't have switch.
I'm tempted to update the logic to something like stripe-python anyway, wdyt?
There was a problem hiding this comment.
Oops, my mistake — I wasn't paying enough attention to notice the $code vs. $type distinction there. Feel free to leave it.
I'm tempted to update the logic to something like stripe-python anyway, wdyt?
IMO the logic ladders are at least pretty similar, and honestly PHP's is quite a bit more readable, so it might be okay to just leave as is. Up to you :)
There was a problem hiding this comment.
Yeah, let's leave it as is then. Thanks!
|
Nice one @ob-stripe, and thanks for taking! My bad for not going through and adding these in when the server changes appeared. |
r? @brandur-stripe
cc @stripe/api-libraries
Adds support for the
idempotency_errorerror type. Cf. stripe/stripe-ruby#613.