saloonphp / saloon

🤠 Build beautiful API integrations and SDKs with Saloon
https://docs.saloon.dev
MIT License
2.09k stars 107 forks source link

Feature | Support retry on async request #323

Closed gdebrauwer closed 9 months ago

gdebrauwer commented 1 year ago

Currently, retrying requests only works when the requests happen synchronously. This PR fixes that so retrying requests also works on asynchronous requests

The tests are copied from RetryRequestTest and they are adapted so all executed requests are asynchronous.

Sammyjo20 commented 1 year ago

Hey @gdebrauwer wow, this is an awesome addition to Saloon - thank you very much! I didn't think of promise chaining when I built the send-and-retry functionality in Saloon.

I think I'm going to look at this PR properly this weekend as I'd like to do some testing, especially around pooling requests and making sure the performance isn't impacted with the request chaining. This looks very promising though.

Sammyjo20 commented 10 months ago

Hey @gdebrauwer sorry for the delay - over the next few weeks I will try to sit down and test this out.

Sammyjo20 commented 10 months ago

Nice implementation @gdebrauwer, thanks for doing this - I've got some tweaks I'm going to make and I'm going to see if there's a way I can reuse some of this code. I wonder if I could use the sendAsync method in the send method so we don't have duplicated code to maintain. I could do something like: $this->sendAsync()->wait() for synchronous requests.

Sammyjo20 commented 9 months ago

Hey @gdebrauwer just wanted to say thank you for working on this feature for Saloon. I've thought about it and have decided that for now at least I won't implement this feature. There's going to be a lot of duplicated code inside of the SendsRequests trait and I've tried different ways to get around it but I feel it should probably wait for v4 where we have a bit more freedom to change how Saloon works for this.

Again, thank you so much!