Skip to content

Commit 61dade9

Browse files
committed
refactor(http): address CURLRequest retry review feedback
- Add 504 Gateway Timeout to default retryable status codes - Reuse stored cURL error number when throwing - Add coverage for default 504 retry behavior Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
1 parent a89a89e commit 61dade9

3 files changed

Lines changed: 20 additions & 3 deletions

File tree

system/HTTP/CURLRequest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class CURLRequest extends OutgoingRequest
9191
'max_retries' => 3,
9292
'delay' => 1000,
9393
'max_delay' => 30_000,
94-
'status_codes' => [429, 503],
94+
'status_codes' => [429, 503, 504],
9595
'curl_errors' => false,
9696
'respect_retry_after' => true,
9797
];
@@ -980,7 +980,7 @@ protected function sendRequest(array $curlOptions = []): string
980980
if ($output === false) {
981981
$this->lastCurlError = curl_errno($ch);
982982

983-
throw HTTPException::forCurlError((string) curl_errno($ch), curl_error($ch));
983+
throw HTTPException::forCurlError((string) $this->lastCurlError, curl_error($ch));
984984
}
985985

986986
return $output;

tests/system/HTTP/CURLRequestRetryTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,23 @@ public function testRetryIntegerRetriesDefaultStatusCodes(): void
7272
$this->assertSame([1.0], $this->request->getSleeps());
7373
}
7474

75+
public function testRetryIntegerRetriesDefaultGatewayTimeoutStatusCode(): void
76+
{
77+
$this->request->setOutputs([
78+
"HTTP/1.1 504 Gateway Timeout\r\n\r\nFirst failure",
79+
"HTTP/1.1 200 OK\r\n\r\nSuccess",
80+
]);
81+
82+
$response = $this->request->get('http://example.com', [
83+
'retry' => 1,
84+
'http_errors' => false,
85+
]);
86+
87+
$this->assertSame(200, $response->getStatusCode());
88+
$this->assertSame('Success', $response->getBody());
89+
$this->assertSame([1.0], $this->request->getSleeps());
90+
}
91+
7592
public function testRetryUsesCustomStatusCodes(): void
7693
{
7794
$this->request->setOutputs([

user_guide_src/source/libraries/curlrequest.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ The available retry settings are:
402402
The default is ``1000``.
403403
- ``max_delay``: Maximum delay in milliseconds. This caps both the configured ``delay`` and a
404404
valid ``Retry-After`` header. Set to ``0`` for no maximum delay. The default is ``30000``.
405-
- ``status_codes``: HTTP status codes that should be retried. The default is ``[429, 503]``.
405+
- ``status_codes``: HTTP status codes that should be retried. The default is ``[429, 503, 504]``.
406406
- ``curl_errors``: Whether to retry transient cURL errors. The default is ``false``.
407407
- ``respect_retry_after``: Whether to use a valid ``Retry-After`` header instead of the configured
408408
``delay``. The default is ``true``.

0 commit comments

Comments
 (0)