From f80084759133a5e00fc997602266575d3806dfaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Tue, 5 May 2026 08:39:32 +0100 Subject: [PATCH] feat(banking): back off scheduler when EnableBanking returns 429 (#352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Production logs show repeated EnableBanking 429s on the same connections, every cron cycle: ``` [2026-05-04 18:00:55] EnableBanking API error status:429 body: [HUB046] Allowed number of accesses exceeded for consent [2026-05-04 21:47:12] EnableBanking API error status:429 body: Daily PSU not present consultation limit has been exceeded [2026-05-05 00:01:41] same connection, same error [2026-05-05 06:01:41] same connection, same error ``` Root cause: `SyncBankingConnectionJob` returned early on 429s without persisting any backoff state. The scheduler kept re-dispatching the same connection on every run, hammering the provider and burning the daily quota. ## Fix Persist a per-connection backoff window so the scheduler stops re-dispatching until the provider quota resets. - New `rate_limited_until` column on `banking_connections`. - On 429: derive the window from 1. `Retry-After` header if present, 2. "Daily ..." message → next UTC midnight (matches PSU daily limit semantics), 3. default 1 hour (consent / generic). - Job short-circuits with a `Skipped` sync log if the window is still active. - Successful sync clears the window. - `SyncAllBankingConnectionsJob` + `SyncBankingConnections` command filter out connections still inside their backoff. ## Tests - Existing rate-limit test updated (now also asserts the backoff is set). - New: daily message → next UTC midnight. - New: `Retry-After` header honoured (1800s). - New: rate-limited connection skipped without calling provider. - New: successful sync clears `rate_limited_until`. - New: scheduler excludes connections whose backoff has not expired. `php artisan test --compact --filter="SyncBankingConnectionJobTest|SyncRetryAndLoggingTest|SyncBankingConnectionsCommandTest"` → 70 passed. --- .../Commands/SyncBankingConnections.php | 4 + app/Jobs/SyncAllBankingConnectionsJob.php | 4 + app/Jobs/SyncBankingConnectionJob.php | 60 +++++++++ app/Models/BankingConnection.php | 9 ++ ...ted_until_to_banking_connections_table.php | 22 ++++ .../SyncBankingConnectionJobTest.php | 115 +++++++++++++++++- .../OpenBanking/SyncRetryAndLoggingTest.php | 22 ++++ 7 files changed, 234 insertions(+), 2 deletions(-) create mode 100644 database/migrations/2026_05_05_070442_add_rate_limited_until_to_banking_connections_table.php diff --git a/app/Console/Commands/SyncBankingConnections.php b/app/Console/Commands/SyncBankingConnections.php index 119a179e..2f648a93 100644 --- a/app/Console/Commands/SyncBankingConnections.php +++ b/app/Console/Commands/SyncBankingConnections.php @@ -52,6 +52,10 @@ class SyncBankingConnections extends Command ->where(function ($query) { $query->whereNull('valid_until') ->orWhere('valid_until', '>', now()); + }) + ->where(function ($query) { + $query->whereNull('rate_limited_until') + ->orWhere('rate_limited_until', '<=', now()); }); if ($connectionId) { diff --git a/app/Jobs/SyncAllBankingConnectionsJob.php b/app/Jobs/SyncAllBankingConnectionsJob.php index e0b8409d..6ed4fc12 100644 --- a/app/Jobs/SyncAllBankingConnectionsJob.php +++ b/app/Jobs/SyncAllBankingConnectionsJob.php @@ -33,6 +33,10 @@ class SyncAllBankingConnectionsJob implements ShouldQueue $query->whereNull('valid_until') ->orWhere('valid_until', '>', now()); }) + ->where(function ($query) { + $query->whereNull('rate_limited_until') + ->orWhere('rate_limited_until', '<=', now()); + }) ->each(function (BankingConnection $connection) { SyncBankingConnectionJob::dispatch($connection, $this->fullSync); }); diff --git a/app/Jobs/SyncBankingConnectionJob.php b/app/Jobs/SyncBankingConnectionJob.php index 7109cfb3..c486e6b4 100644 --- a/app/Jobs/SyncBankingConnectionJob.php +++ b/app/Jobs/SyncBankingConnectionJob.php @@ -22,6 +22,7 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Http\Client\RequestException; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Mail; use Sentry\State\Scope; @@ -86,6 +87,20 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue return; } + if ($connection->isRateLimited()) { + Log::info('Banking connection rate limited, skipping sync', [ + 'connection_id' => $connection->id, + 'rate_limited_until' => $connection->rate_limited_until?->toIso8601String(), + ]); + + $this->logSyncAttempt($connection, BankingSyncLogStatus::Skipped, $startTime, metadata: [ + 'reason' => 'rate_limited', + 'rate_limited_until' => $connection->rate_limited_until?->toIso8601String(), + ]); + + return; + } + try { $isFirstSync = ! $connection->last_synced_at || $this->fullSync; $metadata = []; @@ -111,6 +126,7 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue 'status' => BankingConnectionStatus::Active, 'last_synced_at' => $syncedAt, 'error_message' => null, + 'rate_limited_until' => null, 'consecutive_sync_failures' => 0, ]; @@ -129,6 +145,7 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue ]); if ($this->isRateLimitError($e)) { + $this->applyRateLimitBackoff($connection, $e); $this->logSyncAttempt($connection, BankingSyncLogStatus::Failed, $startTime, $e); return; @@ -371,6 +388,49 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue return $e instanceof RequestException && $e->response->status() === 429; } + /** + * Persist a backoff window so the scheduler stops re-dispatching + * the same connection until the provider quota resets. + */ + private function applyRateLimitBackoff(BankingConnection $connection, \Throwable $e): void + { + $until = $this->resolveRateLimitBackoffUntil($e); + + $connection->update([ + 'rate_limited_until' => $until, + 'error_message' => $this->friendlyErrorMessage($e), + ]); + + Log::warning('Banking connection rate limited, backing off', [ + 'connection_id' => $connection->id, + 'rate_limited_until' => $until->toIso8601String(), + ]); + } + + private function resolveRateLimitBackoffUntil(\Throwable $e): Carbon + { + $now = now(); + + if ($e instanceof RequestException) { + $retryAfter = $e->response->header('Retry-After'); + + if (is_numeric($retryAfter) && (int) $retryAfter > 0) { + return $now->copy()->addSeconds((int) $retryAfter); + } + + $body = $e->response->json(); + $message = is_array($body) ? (string) ($body['message'] ?? '') : ''; + + // Daily PSU consultation limit resets at midnight UTC. + if (str_contains(strtolower($message), 'daily')) { + return $now->copy()->utc()->addDay()->startOfDay(); + } + } + + // Default: back off one hour for consent / generic 429 responses. + return $now->copy()->addHour(); + } + private function isAuthError(\Throwable $e): bool { return $e instanceof RequestException diff --git a/app/Models/BankingConnection.php b/app/Models/BankingConnection.php index 61800626..9de57f38 100644 --- a/app/Models/BankingConnection.php +++ b/app/Models/BankingConnection.php @@ -19,6 +19,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @property Carbon|null $valid_until * @property Carbon|null $last_synced_at * @property Carbon|null $bank_transactions_email_cutoff_at + * @property Carbon|null $rate_limited_until * @property int $consecutive_sync_failures * @property array|null $pending_accounts_data */ @@ -40,6 +41,7 @@ class BankingConnection extends Model 'last_synced_at', 'bank_transactions_email_cutoff_at', 'error_message', + 'rate_limited_until', 'consecutive_sync_failures', 'pending_accounts_data', 'api_token', @@ -61,6 +63,7 @@ class BankingConnection extends Model 'valid_until' => 'datetime', 'last_synced_at' => 'datetime', 'bank_transactions_email_cutoff_at' => 'datetime', + 'rate_limited_until' => 'datetime', 'pending_accounts_data' => 'array', 'api_token' => 'encrypted', 'api_secret' => 'encrypted', @@ -126,4 +129,10 @@ class BankingConnection extends Model return $this->status === BankingConnectionStatus::Expired || ($this->valid_until && $this->valid_until->isPast()); } + + public function isRateLimited(): bool + { + return $this->rate_limited_until !== null + && $this->rate_limited_until->isFuture(); + } } diff --git a/database/migrations/2026_05_05_070442_add_rate_limited_until_to_banking_connections_table.php b/database/migrations/2026_05_05_070442_add_rate_limited_until_to_banking_connections_table.php new file mode 100644 index 00000000..e56ed7fe --- /dev/null +++ b/database/migrations/2026_05_05_070442_add_rate_limited_until_to_banking_connections_table.php @@ -0,0 +1,22 @@ +dateTime('rate_limited_until')->nullable()->after('error_message'); + }); + } + + public function down(): void + { + Schema::table('banking_connections', function (Blueprint $table): void { + $table->dropColumn('rate_limited_until'); + }); + } +}; diff --git a/tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php b/tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php index 7d38a3ef..9a6fad92 100644 --- a/tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php +++ b/tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php @@ -1313,7 +1313,7 @@ test('failed sync job marks active connection as error so onboarding can continu expect($connection->consecutive_sync_failures)->toBe(1); }); -test('rate limit error does not set connection status to error', function () { +test('rate limit error sets backoff window without erroring connection', function () { $user = User::factory()->onboarded()->create(); $connection = BankingConnection::factory()->create([ 'user_id' => $user->id, @@ -1341,5 +1341,116 @@ test('rate limit error does not set connection status to error', function () { $connection->refresh(); expect($connection->status)->toBe(BankingConnectionStatus::Active); - expect($connection->error_message)->toBeNull(); + expect($connection->rate_limited_until)->not->toBeNull(); + expect($connection->rate_limited_until->isFuture())->toBeTrue(); +}); + +test('daily rate limit backoff lasts until next UTC midnight', function () { + $user = User::factory()->onboarded()->create(); + $connection = BankingConnection::factory()->create([ + 'user_id' => $user->id, + 'last_synced_at' => now()->subDay(), + ]); + Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'ext-123', + ]); + + $body = json_encode(['code' => 429, 'message' => 'Daily PSU not present consultation limit has been exceeded']); + $response = new Response(429, ['Content-Type' => 'application/json'], $body); + + $transactionSync = Mockery::mock(TransactionSyncService::class); + $transactionSync->shouldReceive('sync')->andThrow( + new RequestException(new Illuminate\Http\Client\Response($response)) + ); + + $balanceSync = Mockery::mock(BalanceSyncService::class); + + $job = new SyncBankingConnectionJob($connection); + $job->handle($transactionSync, $balanceSync); + + $connection->refresh(); + $expected = now()->utc()->addDay()->startOfDay(); + expect($connection->rate_limited_until)->not->toBeNull(); + expect($connection->rate_limited_until->equalTo($expected))->toBeTrue(); +}); + +test('rate limit backoff honours Retry-After header', function () { + $user = User::factory()->onboarded()->create(); + $connection = BankingConnection::factory()->create([ + 'user_id' => $user->id, + 'last_synced_at' => now()->subDay(), + ]); + Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'ext-123', + ]); + + $response = new Response(429, ['Retry-After' => '1800'], '{}'); + + $transactionSync = Mockery::mock(TransactionSyncService::class); + $transactionSync->shouldReceive('sync')->andThrow( + new RequestException(new Illuminate\Http\Client\Response($response)) + ); + + $balanceSync = Mockery::mock(BalanceSyncService::class); + + $job = new SyncBankingConnectionJob($connection); + $job->handle($transactionSync, $balanceSync); + + $connection->refresh(); + expect($connection->rate_limited_until)->not->toBeNull(); + expect($connection->rate_limited_until->diffInMinutes(now(), true))->toBeGreaterThanOrEqual(29) + ->toBeLessThanOrEqual(31); +}); + +test('rate limited connection is skipped without calling provider', function () { + $user = User::factory()->onboarded()->create(); + $connection = BankingConnection::factory()->create([ + 'user_id' => $user->id, + 'last_synced_at' => now()->subDay(), + 'rate_limited_until' => now()->addHour(), + ]); + Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'ext-123', + ]); + + $transactionSync = Mockery::mock(TransactionSyncService::class); + $transactionSync->shouldNotReceive('sync'); + $balanceSync = Mockery::mock(BalanceSyncService::class); + + $job = new SyncBankingConnectionJob($connection); + $job->handle($transactionSync, $balanceSync); + + $connection->refresh(); + expect($connection->status)->toBe(BankingConnectionStatus::Active); +}); + +test('successful sync clears rate_limited_until', function () { + $user = User::factory()->onboarded()->create(); + $connection = BankingConnection::factory()->create([ + 'user_id' => $user->id, + 'last_synced_at' => now()->subDay(), + 'rate_limited_until' => now()->subMinute(), + ]); + Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'ext-123', + ]); + + $transactionSync = Mockery::mock(TransactionSyncService::class); + $transactionSync->shouldReceive('sync')->andReturn(0); + $balanceSync = Mockery::mock(BalanceSyncService::class); + $balanceSync->shouldReceive('sync')->andReturnNull(); + + $job = new SyncBankingConnectionJob($connection); + $job->handle($transactionSync, $balanceSync); + + $connection->refresh(); + expect($connection->rate_limited_until)->toBeNull(); }); diff --git a/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php b/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php index 13fde21c..e229851c 100644 --- a/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php +++ b/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php @@ -511,6 +511,28 @@ test('scheduled sync excludes error connections with expired valid_until', funct Queue::assertNotPushed(SyncBankingConnectionJob::class); }); +test('scheduled sync skips connections still within rate limit backoff window', function () { + Queue::fake(SyncBankingConnectionJob::class); + + $user = User::factory()->onboarded()->create(); + + BankingConnection::factory()->create([ + 'user_id' => $user->id, + 'rate_limited_until' => now()->addHour(), + ]); + + $eligible = BankingConnection::factory()->create([ + 'user_id' => $user->id, + 'rate_limited_until' => now()->subMinute(), + ]); + + $job = new SyncAllBankingConnectionsJob; + $job->handle(); + + Queue::assertPushed(SyncBankingConnectionJob::class, 1); + Queue::assertPushed(SyncBankingConnectionJob::class, fn ($job) => $job->bankingConnection->id === $eligible->id); +}); + // --- Manual Retry Reset Tests --- test('manual sync resets consecutive sync failures', function () {