mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(http): retry connection failures of idempotent requests for all HTTP calls
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
16c6de823b
commit
bb24acd957
@@ -21,7 +21,7 @@ class OidcDiscoveryService
|
||||
$url = $issuerUrl.'/.well-known/openid-configuration';
|
||||
|
||||
try {
|
||||
$response = Http::timeout(5)->connectTimeout(3)->acceptJson()->get($url);
|
||||
$response = Http::timeout(15)->acceptJson()->get($url);
|
||||
} catch (Throwable $e) {
|
||||
throw new OidcDiscoveryException("Failed to fetch discovery document: {$e->getMessage()}", previous: $e);
|
||||
}
|
||||
@@ -68,7 +68,7 @@ class OidcDiscoveryService
|
||||
|
||||
return Cache::remember($cacheKey, 21600, function () use ($jwksUri): array {
|
||||
try {
|
||||
$response = Http::timeout(5)->connectTimeout(3)->acceptJson()->get($jwksUri);
|
||||
$response = Http::timeout(15)->acceptJson()->get($jwksUri);
|
||||
} catch (Throwable $e) {
|
||||
throw new OidcJwksException("Failed to fetch JWKS: {$e->getMessage()}", previous: $e);
|
||||
}
|
||||
|
||||
@@ -735,7 +735,6 @@ class Github extends Controller
|
||||
$data = Http::GitSource($api_url)->withBody(null)
|
||||
->accept('application/vnd.github+json')
|
||||
->timeout(10)
|
||||
->connectTimeout(5)
|
||||
->post("$api_url/app-manifests/$code/conversions")
|
||||
->throw()
|
||||
->json();
|
||||
@@ -841,7 +840,6 @@ class Github extends Controller
|
||||
'Accept' => 'application/vnd.github+json',
|
||||
])
|
||||
->timeout(10)
|
||||
->connectTimeout(5)
|
||||
->get("{$github_app->api_url}/app/installations/{$installation_id}");
|
||||
} catch (\Throwable $e) {
|
||||
return strip_tags(str_replace('<br>', ' ', $e->getMessage()));
|
||||
|
||||
@@ -9,6 +9,7 @@ use App\Models\PersonalAccessToken;
|
||||
use App\Models\Server;
|
||||
use App\Rules\SafeExternalUrl;
|
||||
use App\Support\AtomicFilesystem;
|
||||
use App\Support\RetryHttpConnectionFailures;
|
||||
use Illuminate\Database\Eloquent\Model;
|
||||
use Illuminate\Foundation\Console\ConfigCacheCommand;
|
||||
use Illuminate\Foundation\Console\RouteCacheCommand;
|
||||
@@ -43,6 +44,7 @@ class AppServiceProvider extends ServiceProvider
|
||||
$this->configureModels();
|
||||
$this->configurePasswords();
|
||||
$this->configureSanctumModel();
|
||||
$this->configureHttpClient();
|
||||
$this->configureGitHubHttp();
|
||||
$this->configureGitLabHttp();
|
||||
$this->configureOidcSocialite();
|
||||
@@ -124,6 +126,12 @@ class AppServiceProvider extends ServiceProvider
|
||||
});
|
||||
}
|
||||
|
||||
private function configureHttpClient(): void
|
||||
{
|
||||
Http::globalOptions(['connect_timeout' => 10, 'timeout' => 30]);
|
||||
Http::globalMiddleware(new RetryHttpConnectionFailures);
|
||||
}
|
||||
|
||||
private function configureGitHubHttp(): void
|
||||
{
|
||||
Http::macro('GitSource', function (string $url) {
|
||||
|
||||
@@ -1,24 +0,0 @@
|
||||
<?php
|
||||
|
||||
namespace App\Services;
|
||||
|
||||
use Illuminate\Http\Client\ConnectionException;
|
||||
use Illuminate\Http\Client\PendingRequest;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
|
||||
class CloudflareHttp
|
||||
{
|
||||
/**
|
||||
* The connect timeout is longer than the 5 second DNS resolver retry, so one lost DNS query does not fail the request.
|
||||
* Only idempotent requests are retried after a connection failure; a POST can reach Cloudflare before the timeout.
|
||||
*/
|
||||
public static function client(string $token): PendingRequest
|
||||
{
|
||||
return Http::withToken($token)
|
||||
->acceptJson()
|
||||
->connectTimeout(10)
|
||||
->timeout(20)
|
||||
->retry(2, 500, fn ($exception, $request, ?string $method) => $exception instanceof ConnectionException
|
||||
&& in_array($method, ['GET', 'PUT', 'DELETE'], true), throw: false);
|
||||
}
|
||||
}
|
||||
@@ -2,11 +2,13 @@
|
||||
|
||||
namespace App\Services;
|
||||
|
||||
use Illuminate\Support\Facades\Http;
|
||||
|
||||
class CloudflareTokenValidator
|
||||
{
|
||||
public function validate(string $token, array $capabilities): bool
|
||||
{
|
||||
$client = CloudflareHttp::client($token);
|
||||
$client = Http::withToken($token)->acceptJson();
|
||||
$verification = $client->get('https://api.cloudflare.com/client/v4/user/tokens/verify');
|
||||
|
||||
if (! $verification->successful() || $verification->json('result.status') !== 'active') {
|
||||
|
||||
@@ -14,6 +14,7 @@ use App\Services\Dns\ManagedDnsRecordCleanup;
|
||||
use Illuminate\Contracts\Cache\LockTimeoutException;
|
||||
use Illuminate\Http\Client\PendingRequest;
|
||||
use Illuminate\Http\Client\Response;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
use Illuminate\Support\Sleep;
|
||||
use RuntimeException;
|
||||
use Spatie\Activitylog\Models\Activity;
|
||||
@@ -282,6 +283,6 @@ class CloudflareTunnelService
|
||||
|
||||
private function client(IntegrationToken $token): PendingRequest
|
||||
{
|
||||
return CloudflareHttp::client($token->token);
|
||||
return Http::withToken($token->token)->acceptJson();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,13 +11,13 @@ use App\Models\IntegrationToken;
|
||||
use App\Models\ManagedDnsRecord;
|
||||
use App\Models\Server;
|
||||
use App\Models\ServiceApplication;
|
||||
use App\Services\CloudflareHttp;
|
||||
use App\Support\DnsRecordHints;
|
||||
use Illuminate\Contracts\Cache\LockTimeoutException;
|
||||
use Illuminate\Database\Eloquent\Model;
|
||||
use Illuminate\Http\Client\PendingRequest;
|
||||
use Illuminate\Support\Collection;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
use RuntimeException;
|
||||
use Throwable;
|
||||
|
||||
@@ -512,6 +512,6 @@ class CloudflareDnsProvider
|
||||
|
||||
private function client(IntegrationToken $token): PendingRequest
|
||||
{
|
||||
return CloudflareHttp::client($token->token);
|
||||
return Http::withToken($token->token)->acceptJson();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -50,8 +50,6 @@ class DopplerService
|
||||
private function client(): PendingRequest
|
||||
{
|
||||
return Http::withToken($this->token)
|
||||
->acceptJson()
|
||||
->connectTimeout(5)
|
||||
->timeout(10);
|
||||
->acceptJson();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
<?php
|
||||
|
||||
namespace App\Support;
|
||||
|
||||
use GuzzleHttp\Exception\ConnectException;
|
||||
use GuzzleHttp\Promise\Create;
|
||||
use GuzzleHttp\Promise\PromiseInterface;
|
||||
use Illuminate\Support\Sleep;
|
||||
use Psr\Http\Message\RequestInterface;
|
||||
use Throwable;
|
||||
|
||||
/**
|
||||
* Guzzle middleware that sends a request again once after a connection failure.
|
||||
* cURL counts the DNS lookup in the connect timeout, and a lost DNS query waits 5 seconds before the resolver tries again.
|
||||
* Only idempotent methods are retried, because a POST or PATCH can reach the server before the timeout.
|
||||
*/
|
||||
class RetryHttpConnectionFailures
|
||||
{
|
||||
private const array RETRIED_METHODS = ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE'];
|
||||
|
||||
public function __invoke(callable $handler): callable
|
||||
{
|
||||
return function (RequestInterface $request, array $options) use ($handler): PromiseInterface {
|
||||
return $this->send($handler, $request, $options)->otherwise(function (Throwable $exception) use ($handler, $request, $options): PromiseInterface {
|
||||
if (! $exception instanceof ConnectException || ! in_array($request->getMethod(), self::RETRIED_METHODS, true)) {
|
||||
return Create::rejectionFor($exception);
|
||||
}
|
||||
|
||||
Sleep::usleep(500_000);
|
||||
|
||||
return $this->send($handler, $request, $options);
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The cURL handler rejects the promise on a connection failure, but other handlers (such as Http::fake()) throw.
|
||||
*/
|
||||
private function send(callable $handler, RequestInterface $request, array $options): PromiseInterface
|
||||
{
|
||||
try {
|
||||
return Create::promiseFor($handler($request, $options));
|
||||
} catch (ConnectException $exception) {
|
||||
return Create::rejectionFor($exception);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -945,7 +945,7 @@ test('cloudflare dns reads are retried after a connection failure but record cre
|
||||
expect(fn () => app(CloudflareDnsProvider::class)->createRecord($zone, 'app.example.com', '203.0.113.10'))
|
||||
->toThrow(ConnectionException::class);
|
||||
|
||||
Http::assertSentCount(3);
|
||||
Http::assertSent(fn ($request) => $request->method() === 'POST');
|
||||
Http::assertSequencesAreEmpty();
|
||||
expect(collect(Http::recorded())->filter(fn (array $pair) => $pair[0]->method() === 'POST'))->toHaveCount(1);
|
||||
expect(ManagedDnsRecord::query()->count())->toBe(0);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
<?php
|
||||
|
||||
use Illuminate\Http\Client\ConnectionException;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
use Illuminate\Support\Sleep;
|
||||
|
||||
beforeEach(function () {
|
||||
Sleep::fake();
|
||||
$this->attempts = 0;
|
||||
});
|
||||
|
||||
function fakeConnectionFailures(int $failures): void
|
||||
{
|
||||
Http::fake(function () use ($failures) {
|
||||
test()->attempts++;
|
||||
|
||||
return test()->attempts <= $failures ? Http::failedConnection() : Http::response(['ok' => true]);
|
||||
});
|
||||
}
|
||||
|
||||
test('a read request is retried once after a connection failure', function () {
|
||||
fakeConnectionFailures(1);
|
||||
|
||||
expect(Http::get('https://api.example.com/items')->json('ok'))->toBeTrue()
|
||||
->and($this->attempts)->toBe(2);
|
||||
});
|
||||
|
||||
test('a read request fails when the retry also has a connection failure', function () {
|
||||
fakeConnectionFailures(2);
|
||||
|
||||
expect(fn () => Http::delete('https://api.example.com/items/1'))->toThrow(ConnectionException::class)
|
||||
->and($this->attempts)->toBe(2);
|
||||
});
|
||||
|
||||
test('a post request is not retried because the server can have received it', function () {
|
||||
fakeConnectionFailures(1);
|
||||
|
||||
expect(fn () => Http::post('https://api.example.com/items'))->toThrow(ConnectionException::class)
|
||||
->and($this->attempts)->toBe(1);
|
||||
});
|
||||
Reference in New Issue
Block a user