Introduce new IdempotencyError type
#613
Conversation
A few weeks back a new error type `idempotency_error` was introduced in the API. I put it in to respond to #503, but then forgot to add support for it in this library. This patch introduces a new exception class that represents it.
| error_code: error_data[:code], | ||
| error_message: error_data[:message], | ||
| error_param: error_data[:param], | ||
| error_type: error_data[:type], |
There was a problem hiding this comment.
These should've been symbols (not strings) all along, but because the problem was duplicated in the tests, I never noticed because everything passed.
| error_code: error_data[:code], | ||
| error_message: error_data[:message], | ||
| error_param: error_data[:param], | ||
| error_type: error_data[:type], |
|
Oops, I reviewed the code before reading your comment!
Yeah, I like the idea of having a single source of truth. That said, even stripe-node seems to partially rely on status codes: https://github.com/stripe/stripe-node/blob/7abdb8913ab927bcde07f1d0b9b61ef1048b9269/lib/StripeResource.js#L163-L171. If the API does return unique type strings that we can use to unambiguously instantiate errors, then I'm in favor of using that and ignoring status codes. |
|
Thanks OB.
Oops, you're right. I guess that it's also doing a little bit of both. I was thinking of this section: StripeError.generate = function(rawStripeError) {
switch (rawStripeError.type) {
case 'card_error':
return new _Error.StripeCardError(rawStripeError);
case 'invalid_request_error':
return new _Error.StripeInvalidRequestError(rawStripeError);
case 'api_error':
return new _Error.StripeAPIError(rawStripeError);
case 'idempotency_error':
return new _Error.StripeIdempotencyError(rawStripeError);
}
return new _Error('Generic', 'Unknown Error');
};
Hm, so I think that this is the case, but it's a little suboptimal how nothing server-side guarantees this per se (also I notice that stripe-python instantiates |
A few weeks back a new error type
idempotency_errorwas introduced in theAPI. I put it in to respond to #503, but then forgot to add support for it in
this library. This patch introduces a new exception class that represents it.
r? @ob-stripe One thing that's a little janky about this patch is that we were
previously just doing error type switching off of the returned status code, but
here we add a second ladder on error type because a 400 becomes ambiguous. This
works fine conceptually, but it might be that we just have a single ladder on
error type and ignore the status code completely (this is how stripe-node works
for example). Thoughts?