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

How a "429 Too Many Requests" or equivalent should be handled? #632

Open
devnix opened this issue Jan 11, 2021 · 2 comments
Open

How a "429 Too Many Requests" or equivalent should be handled? #632

devnix opened this issue Jan 11, 2021 · 2 comments

Comments

@devnix
Copy link

devnix commented Jan 11, 2021

Hi! I'm improving an existing Redsys driver for Omnipay.

Currently, I have a queue of refunds for my project. A cronjob gets the unprocessed refunds of the queue, updates their status, logs the error if it happens, and marks it as executed.

I have an edge case with Redsys. When they consider that we are going too fast (they don't specify any rate limit) they give a 200 HTTP code with a custom status in the response with the code "SIS0295", which could be the HTTP equivalent of 429. In this case the error is logged but the refund is not marked as executed.

The thing is in the driver if I get an error I will throw an InvalidResponseException, but I don't know of a standard way to differentiate an error which will always occur against an error that I can fix by waiting for a few minutes and then trying again.

I've been poking around the PayPal driver, but I've not found any reference for handling 429 Unprocessable Entity - RATE_LIMIT_REACHED or similar.

@devnix
Copy link
Author

devnix commented Jan 11, 2021

Thinking out loud, I'm between this ones in RefundResponse::__construct()

// Best one? I see that there is no way to get the original request or access to
// the configured RequestFactory, so I'm forced to create my own Request class
// just to fake the data, which smells to me like a design problem
throw new \Omnipay\Common\Http\Exception\RequestException('Too many requests', $request);
throw new \Omnipay\Common\Http\Exception\NetworkException('Too many requests', 429);
throw new \Omnipay\Common\Exception\InvalidRequestException('Too many requests', 429);
throw new Omnipay\Common\Exception\RuntimeException('Too many requests', 429); // Or default 0 code

I've been looking through several Omnipay gateways and I don't find any implementing this, so I'm very concerned about how to proceed so my gateway would remain compatible with the rest of the existing ones.

EDIT: Maybe throwing Omnipay\Common\Http\Exception\RequestException at RefundRequest::sendData() before creating RefundResponse would be the best approach?

@devnix
Copy link
Author

devnix commented Jan 12, 2021

Maybe a good implementation on the omnipay-common side would be an \Omnipay\Common\Http\Exception\TooManyRequestsHttpException extending \Omnipay\Common\Http\Exception\InvalidRequestException in the line of the symfony/http-kernel component.

WDYT @judgej?

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

1 participant