feat(banking): back off scheduler when EnableBanking returns 429 (#352)
## 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.
This commit is contained in:
parent
21b5692174
commit
f800847591
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<int, mixed>|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();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,22 @@
|
|||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Database\Schema\Blueprint;
|
||||
use Illuminate\Support\Facades\Schema;
|
||||
|
||||
return new class extends Migration
|
||||
{
|
||||
public function up(): void
|
||||
{
|
||||
Schema::table('banking_connections', function (Blueprint $table): void {
|
||||
$table->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');
|
||||
});
|
||||
}
|
||||
};
|
||||
|
|
@ -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();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 () {
|
||||
|
|
|
|||
Loading…
Reference in New Issue