Http:retry(...) does not retry request when throw param is set to false

[Cintrust]

• Laravel Version: 9.2.0
• PHP Version: 8.1.0
• Database Driver & Version: MSQL v8.0.27

Description:

Calling Http::retry(3, 100, function(){return true} ,throw: false)->post(...); does not retry the request but returns the response of the first attempt.

Steps To Reproduce:

run php artisan tinker and paste the following code.
or just paste it inside any controller

$response = Http::retry(3, 100,function($e){
 report($e);
return true;
},   false)->post("https://sandbox.monnify.com/api/v2/bank-transfer/reserved-accounts");

check logs. No error would be reported

[eduance]

In addition to what @JawadR1 has posted I wanted to confirm that it was actually retrying the given request.

Lets have a look together.

Http::retry(3, 100, function ($exception) { report($e); }

The retry() method simply sets some properties within the PendingRequest class, free of charge!

However, it is important to understand what those properties will end up being used for. Like grannie always tell me, you can have loads of properties, as long as you make some profit on them.

The first parameter is the amount of tries, the second is the time in between the requests and the third is a parameter named when which basically asks the developer: when do you really want to retry your request?

Therefore the report() function doesn't make sense within this callback, as the docs say:

The third argument should be a callable that determines if the retries should actually be attempted. For example, you may wish to only retry the request if the initial request encounters an ConnectionException.

Now, my last point that I wanted to make sure that I wasn't being too hasty with posting my answer and wanted to make sure that it did actually try to retry the request. The post() method calls the send() method which ends up calling the retry() helper.

This method has a beautiful test written for it and works like a charm and has last been tested less than a month ago, feel free to check it out here:

https://github.com/laravel/framework/blob/9.x/tests/Support/SupportHelpersTest.php#L572

Last but not least, as has been pointed out by @JawadR1 we can say that everything is working as supposed to as the last boolean accepted by the Http::retry() method is whether you want the method to throw a guaranteed RequestException or to just output the last response.

Hopefully this has clarified some of the confusion, check out Laracasts or other forums to ask all your questions or check out the documentation: https://laravel.com/docs/9.x/http-client#retries

[littleblue50]

I know I'm not the OP but I am having this exact issue too and thank you for your well thought out and thorough response, but in my case (and I think the OPs if I'm reading it correctly) the retry isn't actually done. I tested this by logging calls to the retried endpoint as well as within the closure itself to confirm it doesn't.

[eduance]

@littleblue50 Could you show how you are using the retry method?

[Cintrust]

Thank you @JawadR1, @eduance for detailed the response but Infact dispite what the documentation says the retry does not occur at all when the throw param is set to false.

The code snippet I included above will not retry the request as it should. Laravel makes the request and gives up after one trial (no retrial is done).

Therefore the report() function doesn't make sense within this callback,

The report() function inside the callback is meant to be a source of truth to confirm if the request is actually being retried.

I will take a look at the test and confirm if this particular case was handled.

[Cintrust]

@eduance,
I have looked at the test located at
https://github.com/laravel/framework/blob/9.x/tests/Support/SupportHelpersTest.php#L572

I could not see any test for Http:retry()
Which is what I am having issues with.
The retry function as you mentioned works as it should.

[eduance]

@Cintrust Sorry for the late reply. I see what you mean, I indeed don't see it retry when the throw parameter is set to false. I'll see when I have time to help out with a PR or feel free to submit a PR for it.

[driesvints]

Heya, thanks for reporting.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as separate commits on the main/master branch and share the repository here? Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Please do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!

[driesvints]

A fix for this was submitted by @gdebrauwer, thanks!

[jordanade]

I'm having a related problem: when retry() is used, it seems to work, but exceptions are thrown for 4xx errors instead of the documented default behavior.