8000 [9.x] Fix Http Client `throw` boolean parameter of `retry` method (#4… · laravel/framework@d4da662 · GitHub
[go: up one dir, main page]
Skip to content

Commit d4da662

Browse files
gdebrauwergdebrauwer
authored
[9.x] Fix Http Client throw boolean parameter of retry method (#41762)
* Add failing test * Fix retryThrow behavior * Fix code when no retry is necessary
1 parent 60115bd commit d4da662

File tree

2 files changed

+85
-10
lines changed

2 files changed

+85
-10
lines changed

‎src/Illuminate/Http/Client/PendingRequest.php

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -707,13 +707,23 @@ public function send(string $method, string $url, array $options = [])
707707
return $this->makePromise($method, $url, $options);
708708
}
709709

710-
return retry($this->tries ?? 1, function () use ($method, $url, $options) {
710+
$shouldRetry = true;
711+
712+
return retry($this->tries ?? 1, function ($attempt) use ($method, $url, $options, &$shouldRetry) {
711713
try {
712-
return tap(new Response($this->sendRequest($method, $url, $options)), function ($response) {
714+
return tap(new Response($this->sendRequest($method, $url, $options)), function ($response) use ($attempt, &$shouldRetry) {
713715
$this->populateResponse($response);
714716

715-
if ($this->tries > 1 && $this->retryThrow && ! $response->successful()) {
716-
$response->throw();
717+
if (! $response->successful()) {
718+
$shouldRetry = $this->retryWhenCallback ? call_user_func($this->retryWhenCallback, $response->toException()) : true;
719+
720+
if ($attempt < $this->tries && $shouldRetry) {
721+
$response->throw();
722+
}
723+
724+
if ($this->tries > 1 && $this->retryThrow) {
725+
$response->throw();
726+
}
717727
}
718728

719729
$this->dispatchResponseReceivedEvent($response);
@@ -723,7 +733,9 @@ public function send(string $method, string $url, array $options = [])
723733

724734
throw new ConnectionException($e->getMessage(), 0, $e);
725735
}
726-
}, $this->retryDelay ?? 100, $this->retryWhenCallback);
736+
}, $this->retryDelay ?? 100, function () use (&$shouldRetry) {
737+
return $shouldRetry;
738+
});
727739
}
728740

729741
/**

‎tests/Http/HttpClientTest.php

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1184,15 +1184,53 @@ public function testRequestIsMacroable()
11841184

11851185
public function testRequestExceptionIsThrownWhenRetriesExhausted()
11861186
{
1187-
$this->expectException(RequestException::class);
1188-
11891187
$this->factory->fake([
11901188
'*' => $this->factory->response(['error'], 403),
11911189
]);
11921190

1193-
$this->factory
1194-
->retry(2, 1000, null, true)
1195-
->get('http://foo.com/get');
1191+
$exception = null;
1192+
1193+
try {
1194+
$this->factory
1195+
->retry(2, 1000, null, true)
1196+
->get('http://foo.com/get');
1197+
} catch (RequestException $e) {
1198+
$exception = $e;
1199+
}
1200+
1201+
$this->assertNotNull($exception);
1202+
$this->assertInstanceOf(RequestException::class, $exception);
1203+
1204+
$this->factory->assertSentCount(2);
1205+
}
1206+
1207+
public function testRequestExceptionIsThrownWithoutRetriesIfRetryNotNecessary()
1208+
{
1209+
$this->factory->fake([
1210+
'*' => $this->factory->response(['error'], 500),
1211+
]);
1212+
1213+
$exception = null;
1214+
$whenAttempts = 0;
1215+
1216+
try {
1217+
$this->factory
1218+
->retry(2, 1000, function ($exception) use (&$whenAttempts) {
1219+
$whenAttempts++;
1220+
1221+
return $exception->response->status() === 403;
1222+
}, true)
1223+
->get('http://foo.com/get');
1224+
} catch (RequestException $e) {
1225+
$exception = $e;
1226+
}
1227+
1228+
$this->assertNotNull($exception);
1229+
$this->assertInstanceOf(RequestException::class, $exception);
1230+
1231+
$this->assertSame(1, $whenAttempts);
1232+
1233+
$this->factory->assertSentCount(1);
11961234
}
11971235

11981236
public function testRequestExceptionIsNotThrownWhenDisabledAndRetriesExhausted()
@@ -1206,6 +1244,31 @@ public function testRequestExceptionIsNotThrownWhenDisabledAndRetriesExhausted()
12061244
->get('http://foo.com/get');
12071245

12081246
$this->assertTrue($response->failed());
1247+
1248+
$this->factory->assertSentCount(2);
1249+
}
1250+
1251+
public function testRequestExceptionIsNotThrownWithoutRetriesIfRetryNotNecessary()
1252+
{
1253+
$this->factory->fake([
1254+
'*' => $this->factory->response(['error'], 500),
1255+
]);
1256+
1257+
$whenAttempts = 0;
1258+
1259+
$response = $this->factory
1260+
->retry(2, 1000, function ($exception) use (&$whenAttempts) {
1261+
$whenAttempts++;
1262+
1263+
return $exception->response->status() === 403;
1264+
}, false)
1265+
->get('http://foo.com/get');
1266+
1267+
$this->assertTrue($response->failed());
1268+
1269+
$this->assertSame(1, $whenAttempts);
1270+
1271+
$this->factory->assertSentCount(1);
12091272
}
12101273

12111274
public function testMiddlewareRunsWhenFaked()

0 commit comments

Comments
 (0)
0