fix(currency): make rate fetching resilient to slow CDN (#502)
## 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.
This commit is contained in:
parent
f4bbbfd767
commit
4b90bcfc96
|
|
@ -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<string, array<string, float>> 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<string, float>
|
||||
*/
|
||||
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<string, float> $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<int, string>
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -1,8 +1,14 @@
|
|||
<?php
|
||||
|
||||
use App\Services\CurrencyConversionService;
|
||||
use Illuminate\Http\Client\ConnectionException;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
|
||||
beforeEach(function () {
|
||||
Cache::flush();
|
||||
});
|
||||
|
||||
test('converts crypto to fiat using CDN rates', function () {
|
||||
Http::fake([
|
||||
'cdn.jsdelivr.net/*currencies/eur*' => 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([
|
||||
|
|
|
|||
Loading…
Reference in New Issue