diff --git a/app/Auth/Oidc/OidcDiscoveryService.php b/app/Auth/Oidc/OidcDiscoveryService.php index 0847afc9a7..eddddb219e 100644 --- a/app/Auth/Oidc/OidcDiscoveryService.php +++ b/app/Auth/Oidc/OidcDiscoveryService.php @@ -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); } diff --git a/app/Http/Controllers/Webhook/Github.php b/app/Http/Controllers/Webhook/Github.php index e644a68c26..c69487031d 100644 --- a/app/Http/Controllers/Webhook/Github.php +++ b/app/Http/Controllers/Webhook/Github.php @@ -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('
', ' ', $e->getMessage())); diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 8130a5400d..674cf95914 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -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) { diff --git a/app/Services/CloudflareHttp.php b/app/Services/CloudflareHttp.php deleted file mode 100644 index 7e301b37fe..0000000000 --- a/app/Services/CloudflareHttp.php +++ /dev/null @@ -1,24 +0,0 @@ -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); - } -} diff --git a/app/Services/CloudflareTokenValidator.php b/app/Services/CloudflareTokenValidator.php index baa622f14b..265268ccb0 100644 --- a/app/Services/CloudflareTokenValidator.php +++ b/app/Services/CloudflareTokenValidator.php @@ -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') { diff --git a/app/Services/CloudflareTunnelService.php b/app/Services/CloudflareTunnelService.php index c55785e59d..ac25a3b0eb 100644 --- a/app/Services/CloudflareTunnelService.php +++ b/app/Services/CloudflareTunnelService.php @@ -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(); } } diff --git a/app/Services/Dns/CloudflareDnsProvider.php b/app/Services/Dns/CloudflareDnsProvider.php index e8d9c0ae71..614ab066dd 100644 --- a/app/Services/Dns/CloudflareDnsProvider.php +++ b/app/Services/Dns/CloudflareDnsProvider.php @@ -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(); } } diff --git a/app/Services/DopplerService.php b/app/Services/DopplerService.php index 2513a4f7d8..b9cf24650e 100644 --- a/app/Services/DopplerService.php +++ b/app/Services/DopplerService.php @@ -50,8 +50,6 @@ class DopplerService private function client(): PendingRequest { return Http::withToken($this->token) - ->acceptJson() - ->connectTimeout(5) - ->timeout(10); + ->acceptJson(); } } diff --git a/app/Support/RetryHttpConnectionFailures.php b/app/Support/RetryHttpConnectionFailures.php new file mode 100644 index 0000000000..1ab42313a0 --- /dev/null +++ b/app/Support/RetryHttpConnectionFailures.php @@ -0,0 +1,47 @@ +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); + } + } +} diff --git a/tests/Feature/DnsProviderManagementTest.php b/tests/Feature/DnsProviderManagementTest.php index b3a6d8a382..2d5a75858d 100644 --- a/tests/Feature/DnsProviderManagementTest.php +++ b/tests/Feature/DnsProviderManagementTest.php @@ -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); }); diff --git a/tests/Feature/HttpConnectionRetryTest.php b/tests/Feature/HttpConnectionRetryTest.php new file mode 100644 index 0000000000..58543a1387 --- /dev/null +++ b/tests/Feature/HttpConnectionRetryTest.php @@ -0,0 +1,40 @@ +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); +});