From 4b90bcfc96b4dd6340981d53f2e665d02872288f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Mon, 8 Jun 2026 09:10:38 +0200 Subject: [PATCH] fix(currency): make rate fetching resilient to slow CDN (#502) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Hardens `CurrencyConversionService` against a slow/unreachable external FX-rate CDN, which was crashing `/api/cashflow/trend`. ## Sentry - Fixes PHP-LARAVEL-2X — `RuntimeException: Failed to fetch currency rates ... cURL error 28: Operation timed out` - Fixes PHP-LARAVEL-31 — `FatalError: Maximum execution time of 30 seconds exceeded` (Guzzle CurlFactory) ## Root cause For a historical date the service walked up to 8 lookback dates × 2 URLs at a **10s timeout each** (~160s worst case → 30s PHP fatal), and a single network timeout threw a `RuntimeException` that 500'd the whole request. ## Changes - **Bounded timeouts**: 3s connect / 5s total per request. - **Abort on unreachable source**: a connection/timeout error stops the date walk (after trying the fallback host) instead of repeating the same timeout for every candidate date. 404s still walk back to earlier releases. - **Graceful degradation**: failures return an empty rate map instead of throwing — conversions already fall back to `0.0` on missing rates, so the trend endpoint no longer 500s. - **Persistent cache**: historical releases cached 30d (immutable), `latest` 6h, unavailable results 10m (dampens retries during an outage). ## Tests - Updated the former "throws" test to assert graceful degradation to `0.0`. - Added: timeout aborts the date walk after 2 attempts; rates cache across service instances. - All 12 tests pass. --- app/Services/CurrencyConversionService.php | 96 +++++++++++++++---- .../Feature/CurrencyConversionServiceTest.php | 46 ++++++++- 2 files changed, 122 insertions(+), 20 deletions(-) diff --git a/app/Services/CurrencyConversionService.php b/app/Services/CurrencyConversionService.php index 94673b04..ae2dda0c 100644 --- a/app/Services/CurrencyConversionService.php +++ b/app/Services/CurrencyConversionService.php @@ -2,10 +2,11 @@ namespace App\Services; +use Illuminate\Http\Client\ConnectionException; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; -use RuntimeException; class CurrencyConversionService { @@ -15,6 +16,16 @@ class CurrencyConversionService private const HISTORICAL_LOOKBACK_DAYS = 7; + private const HTTP_CONNECT_TIMEOUT_SECONDS = 3; + + private const HTTP_TIMEOUT_SECONDS = 5; + + private const CACHE_TTL_HISTORICAL_SECONDS = 60 * 60 * 24 * 30; + + private const CACHE_TTL_LATEST_SECONDS = 60 * 60 * 6; + + private const CACHE_TTL_UNAVAILABLE_SECONDS = 60 * 10; + /** @var array> Keyed by "{currency}:{date}" */ private array $rateCache = []; @@ -67,7 +78,15 @@ class CurrencyConversionService return $this->rateCache[$cacheKey]; } - $rates = $this->fetchRates($currency, $date); + $persistentKey = "currency-rates:{$cacheKey}"; + + $rates = Cache::get($persistentKey); + + if ($rates === null) { + $rates = $this->fetchRates($currency, $date); + Cache::put($persistentKey, $rates, $this->cacheTtlFor($date, $rates)); + } + $this->rateCache[$cacheKey] = $rates; return $rates; @@ -76,35 +95,58 @@ class CurrencyConversionService /** * Fetch rates from CDN with fallback. * + * A missing release (404) walks back to earlier historical dates, but an + * unreachable source (connection refused or timeout) aborts the walk: the + * same timeout would repeat for every candidate date and risk exhausting + * the request's execution time. Failures degrade to an empty rate map + * rather than throwing, so a slow CDN never crashes the calling endpoint. + * * @return array */ private function fetchRates(string $currency, string $date): array { - $lastException = null; + $sourceUnreachable = false; foreach ($this->candidateDates($date) as $candidateDate) { foreach ($this->rateUrls($currency, $candidateDate) as $url) { try { - $response = Http::timeout(10)->get($url); + $response = Http::connectTimeout(self::HTTP_CONNECT_TIMEOUT_SECONDS) + ->timeout(self::HTTP_TIMEOUT_SECONDS) + ->get($url); + } catch (ConnectionException $e) { + $sourceUnreachable = true; - if ($response->notFound()) { - continue; - } + Log::warning('Currency rate source unreachable', [ + 'currency' => $currency, + 'date' => $candidateDate, + 'url' => $url, + 'error' => $e->getMessage(), + ]); - $response->throw(); - - return $response->json($currency) ?? []; - } catch (\Throwable $e) { - $lastException = $e; + continue; } + + if ($response->notFound()) { + continue; + } + + if ($response->successful()) { + return $response->json($currency) ?? []; + } + + Log::warning('Currency rate source returned an error', [ + 'currency' => $currency, + 'date' => $candidateDate, + 'status' => $response->status(), + ]); + } + + if ($sourceUnreachable) { + break; } } - if ($lastException !== null) { - throw new RuntimeException("Failed to fetch currency rates for {$currency} on {$date}: {$lastException->getMessage()}", 0, $lastException); - } - - Log::warning('Currency rates unavailable, all sources returned 404', [ + Log::warning('Currency rates unavailable', [ 'currency' => $currency, 'date' => $date, ]); @@ -112,6 +154,26 @@ class CurrencyConversionService return []; } + /** + * Resolve the cache lifetime for a fetched rate map. + * + * Historical releases are immutable, so cache them long. The "latest" + * release changes daily. An empty result means the sources were missing or + * unreachable; cache it briefly so a transient outage recovers quickly. + * + * @param array $rates + */ + private function cacheTtlFor(string $date, array $rates): int + { + if ($rates === []) { + return self::CACHE_TTL_UNAVAILABLE_SECONDS; + } + + return $date === 'latest' + ? self::CACHE_TTL_LATEST_SECONDS + : self::CACHE_TTL_HISTORICAL_SECONDS; + } + /** * @return array */ diff --git a/tests/Feature/CurrencyConversionServiceTest.php b/tests/Feature/CurrencyConversionServiceTest.php index aa3581c2..9c9e9e2c 100644 --- a/tests/Feature/CurrencyConversionServiceTest.php +++ b/tests/Feature/CurrencyConversionServiceTest.php @@ -1,8 +1,14 @@ Http::response([ @@ -63,15 +69,49 @@ test('uses fallback URL when primary fails', function () { expect($result)->toBe(1.0 / 0.00002); }); -test('throws when both primary and fallback fail', function () { +test('degrades to zero when both primary and fallback fail', function () { Http::fake([ 'cdn.jsdelivr.net/*' => Http::response('Server Error', 500), 'currency-api.pages.dev/*' => Http::response('Server Error', 500), ]); $service = new CurrencyConversionService; - $service->convert('BTC', 'EUR', 1.0, '2026-01-15'); -})->throws(RuntimeException::class); + $result = $service->convert('BTC', 'EUR', 1.0, '2026-01-15'); + + expect($result)->toBe(0.0); +}); + +test('degrades to zero and stops walking dates when the source times out', function () { + $attempts = 0; + + Http::fake(function () use (&$attempts) { + $attempts++; + + throw new ConnectionException('cURL error 28: Operation timed out'); + }); + + $service = new CurrencyConversionService; + $result = $service->convert('BTC', 'EUR', 1.0, '2026-01-15'); + + expect($result)->toBe(0.0); + + // Only the first candidate date is attempted (primary + fallback); a timing + // out source must not be retried across every historical lookback date. + expect($attempts)->toBe(2); +}); + +test('caches rates across service instances so repeat lookups skip HTTP', function () { + Http::fake([ + 'cdn.jsdelivr.net/*currencies/eur*' => Http::response([ + 'eur' => ['btc' => 0.000015], + ]), + ]); + + (new CurrencyConversionService)->convert('BTC', 'EUR', 1.0, '2026-01-15'); + (new CurrencyConversionService)->convert('BTC', 'EUR', 1.0, '2026-01-15'); + + Http::assertSentCount(1); +}); test('returns zero when base currency is unknown and all sources return 404', function () { Http::fake([