fix(cashflow): bound trend window to prevent request timeout (#534)
## Sentry Fixes **[PHP-LARAVEL-3A](https://whisper-money.sentry.io/issues/PHP-LARAVEL-3A)** — `FatalError: Maximum execution time of 30 seconds exceeded` on `GET /api/cashflow/trend`. Top frame in Carbon date math (`getUTCUnit`). ## Root cause `trend()` caps the `months` param at 24, but the `from`/`to` branch validated only `date` — **no span limit**: ```php if (isset($validated['from'], $validated['to'])) { $start = Carbon::parse($validated['from'])->startOfMonth(); $end = Carbon::parse($validated['to'])->endOfMonth(); } ``` The series is then built by `while ($current->lte($end)) { ...; $current->addMonth(); }`. A request with a huge range (e.g. `from=0001-01-01&to=9999-12-31`) runs that loop hundreds of thousands of times — each iteration doing Carbon `format()`/`addMonth()` — and exhausts the 30s timeout. (Sentry strips query strings, which is why the captured URL looked param-less.) The frontend uses the capped `?months=12&to=...` path for the month view, but the uncapped `?from=...&to=...` path for other period types. ## Fix Clamp the computed `start` to at most `MAX_TREND_MONTHS` (24) months before `end`, regardless of branch. Bounds both the month loop and the transaction query window. 24 matches the existing `months` ceiling. ## Test `cashflow trend caps the window for unbounded date ranges` — requests `from=0001-01-01&to=9999-12-31` and asserts exactly 24 data points returned, in ~0.02s. Without the clamp this loops ~96k months and hangs.
This commit is contained in:
parent
cd323bbe52
commit
1d4bcd5082
|
|
@ -17,6 +17,8 @@ use Illuminate\Support\Collection;
|
|||
|
||||
class CashflowAnalyticsController extends Controller
|
||||
{
|
||||
private const MAX_TREND_MONTHS = 24;
|
||||
|
||||
public function __construct(
|
||||
private ExchangeRateService $exchangeRateService,
|
||||
private CategoryTree $tree,
|
||||
|
|
@ -70,7 +72,7 @@ class CashflowAnalyticsController extends Controller
|
|||
public function trend(Request $request): JsonResponse
|
||||
{
|
||||
$validated = $request->validate([
|
||||
'months' => 'nullable|integer|min:1|max:24',
|
||||
'months' => 'nullable|integer|min:1|max:'.self::MAX_TREND_MONTHS,
|
||||
'from' => 'nullable|date',
|
||||
'to' => 'nullable|date',
|
||||
]);
|
||||
|
|
@ -88,6 +90,15 @@ class CashflowAnalyticsController extends Controller
|
|||
$start = $end->copy()->subMonthsNoOverflow($months - 1)->startOfMonth();
|
||||
}
|
||||
|
||||
// Bound the window to the most recent MAX_TREND_MONTHS months so an
|
||||
// unbounded from/to range cannot make the month loop below iterate
|
||||
// indefinitely and exhaust the request timeout.
|
||||
$earliestStart = $end->copy()->subMonthsNoOverflow(self::MAX_TREND_MONTHS - 1)->startOfMonth();
|
||||
|
||||
if ($start->lt($earliestStart)) {
|
||||
$start = $earliestStart;
|
||||
}
|
||||
|
||||
$monthlyTotals = $this->getMonthlyTrendTotals($user->id, $user->currency_code, $start, $end);
|
||||
|
||||
$data = [];
|
||||
|
|
|
|||
|
|
@ -703,6 +703,16 @@ test('cashflow trend can use explicit period bounds', function () {
|
|||
expect($data['2025-05']['income'])->toBe(48000);
|
||||
});
|
||||
|
||||
test('cashflow trend caps the window for unbounded date ranges', function () {
|
||||
$response = $this->getJson('/api/cashflow/trend?'.http_build_query([
|
||||
'from' => '0001-01-01',
|
||||
'to' => '9999-12-31',
|
||||
]));
|
||||
|
||||
$response->assertOk();
|
||||
expect(count($response->json('data')))->toBe(24);
|
||||
});
|
||||
|
||||
test('cashflow trend defaults to 12 months', function () {
|
||||
$response = $this->getJson('/api/cashflow/trend');
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue