diff --git a/app/Exceptions/Banking/InaccessibleBankAccountException.php b/app/Exceptions/Banking/InaccessibleBankAccountException.php new file mode 100644 index 00000000..744004ad --- /dev/null +++ b/app/Exceptions/Banking/InaccessibleBankAccountException.php @@ -0,0 +1,17 @@ +isInaccessibleAccount($e)) { + throw new InaccessibleBankAccountException( + 'EnableBanking account is no longer accessible while fetching transactions.', + previous: $e, + ); + } + if (! $this->isAspspError($e)) { throw $e; } @@ -155,6 +163,13 @@ class EnableBankingProvider implements BankingProviderInterface ); } + if ($this->isInaccessibleAccount($e)) { + throw new InaccessibleBankAccountException( + 'EnableBanking account is no longer accessible while fetching balances.', + previous: $e, + ); + } + if (! $this->isAspspError($e)) { throw $e; } @@ -217,6 +232,17 @@ class EnableBankingProvider implements BankingProviderInterface && ($body['error'] ?? null) === 'EXPIRED_SESSION'; } + private function isInaccessibleAccount(RequestException $e): bool + { + $detail = $this->errorBody($e)['detail'] ?? null; + $errorName = is_array($detail) ? ($detail['error_name'] ?? null) : null; + + // ponytail: the documented per-account 400; widen if other terminal + // account-level codes surface for a single account. + return $e->response->status() === 400 + && $errorName === 'AccountNotAccessibleException'; + } + /** * @return array */ diff --git a/app/Services/Banking/Sync/EnableBankingSyncer.php b/app/Services/Banking/Sync/EnableBankingSyncer.php index 9f329407..e7724117 100644 --- a/app/Services/Banking/Sync/EnableBankingSyncer.php +++ b/app/Services/Banking/Sync/EnableBankingSyncer.php @@ -2,10 +2,12 @@ namespace App\Services\Banking\Sync; +use App\Exceptions\Banking\InaccessibleBankAccountException; use App\Jobs\SendDailyBankTransactionsSyncedEmailJob; use App\Models\BankingConnection; use App\Services\Banking\BalanceSyncService; use App\Services\Banking\TransactionSyncService; +use Illuminate\Support\Facades\Log; class EnableBankingSyncer extends AbstractBankingConnectionSyncer { @@ -37,28 +39,40 @@ class EnableBankingSyncer extends AbstractBankingConnectionSyncer $connection->load('accounts.bank'); foreach ($connection->accounts as $account) { - if ($account->isLinked()) { - $lastTransaction = $account->transactions() - ->latest('transaction_date') - ->first(); + try { + if ($account->isLinked()) { + $lastTransaction = $account->transactions() + ->latest('transaction_date') + ->first(); - $linkedDateFrom = $lastTransaction - ? $lastTransaction->transaction_date->toDateString() - : $dateFrom; + $linkedDateFrom = $lastTransaction + ? $lastTransaction->transaction_date->toDateString() + : $dateFrom; - if ($linkedDateFrom > $dateTo) { - $linkedDateFrom = $dateTo; + if ($linkedDateFrom > $dateTo) { + $linkedDateFrom = $dateTo; + } + + $created = $this->transactionSync->sync($account, $linkedDateFrom, $dateTo, $strategy, saveDailyBalances: false); + $this->balanceSync->sync($account); + } else { + $created = $this->transactionSync->sync($account, $dateFrom, $dateTo, $strategy); + $this->balanceSync->sync($account); + + if ($isFirstSync) { + $this->balanceSync->calculateHistoricalBalances($account); + } } + } catch (InaccessibleBankAccountException) { + // A single account the bank no longer exposes must not break the + // whole connection sync. Skip it; the user can stop syncing it + // from the manage-accounts screen. + Log::warning('Skipping inaccessible EnableBanking account during sync', [ + 'connection_id' => $connection->id, + 'account_id' => $account->id, + ]); - $created = $this->transactionSync->sync($account, $linkedDateFrom, $dateTo, $strategy, saveDailyBalances: false); - $this->balanceSync->sync($account); - } else { - $created = $this->transactionSync->sync($account, $dateFrom, $dateTo, $strategy); - $this->balanceSync->sync($account); - - if ($isFirstSync) { - $this->balanceSync->calculateHistoricalBalances($account); - } + continue; } if ($created > 0) { diff --git a/tests/Feature/OpenBanking/EnableBankingProviderTest.php b/tests/Feature/OpenBanking/EnableBankingProviderTest.php index 66323fc1..59b1195e 100644 --- a/tests/Feature/OpenBanking/EnableBankingProviderTest.php +++ b/tests/Feature/OpenBanking/EnableBankingProviderTest.php @@ -1,6 +1,7 @@ fail('Expected expired banking session exception.'); }); +test('getTransactions wraps an inaccessible account 400 as a non-reportable error', function () { + Http::fake([ + 'api.enablebanking.com/accounts/ext-123/transactions*' => Http::response([ + 'code' => 400, + 'message' => 'Account not found', + 'detail' => [ + 'message' => 'Account not found', + 'error_name' => 'AccountNotAccessibleException', + ], + ], 400), + ]); + + $provider = enableBankingProviderForTest(); + + try { + $provider->getTransactions('ext-123', now()->toDateString(), now()->toDateString()); + } catch (InaccessibleBankAccountException $e) { + expect($e)->toBeInstanceOf(ShouldntReport::class) + ->and($e->getPrevious())->toBeInstanceOf(RequestException::class); + + return; + } + + test()->fail('Expected inaccessible bank account exception.'); +}); + +test('getBalances wraps an inaccessible account 400 as a non-reportable error', function () { + Http::fake([ + 'api.enablebanking.com/accounts/ext-123/balances' => Http::response([ + 'code' => 400, + 'message' => 'Account not found', + 'detail' => [ + 'message' => 'Account not found', + 'error_name' => 'AccountNotAccessibleException', + ], + ], 400), + ]); + + $provider = enableBankingProviderForTest(); + + try { + $provider->getBalances('ext-123'); + } catch (InaccessibleBankAccountException $e) { + expect($e)->toBeInstanceOf(ShouldntReport::class) + ->and($e->getPrevious())->toBeInstanceOf(RequestException::class); + + return; + } + + test()->fail('Expected inaccessible bank account exception.'); +}); + test('getTransactions keeps non-ASPSP client errors reportable', function () { Http::fake([ 'api.enablebanking.com/accounts/ext-123/transactions*' => Http::response([ diff --git a/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php b/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php index b3a331b2..9bdd9991 100644 --- a/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php +++ b/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php @@ -3,6 +3,7 @@ use App\Enums\BankingConnectionStatus; use App\Enums\BankingSyncLogStatus; use App\Exceptions\Banking\ExpiredBankingSessionException; +use App\Exceptions\Banking\InaccessibleBankAccountException; use App\Exceptions\Banking\TransientBankingProviderException; use App\Jobs\SyncAllBankingConnectionsJob; use App\Jobs\SyncBankingConnectionJob; @@ -197,6 +198,49 @@ test('expired session marks the connection expired and emails the user instead o }); }); +test('an inaccessible account is skipped and the rest of the connection still syncs', 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' => 'good-account', + ]); + Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'dead-account', + ]); + + $transactionSync = Mockery::mock(TransactionSyncService::class); + $transactionSync->shouldReceive('sync')->andReturnUsing(function ($account) { + if ($account->external_account_id === 'dead-account') { + throw new InaccessibleBankAccountException('EnableBanking account is no longer accessible.'); + } + + return 2; + }); + + $balanceSync = Mockery::mock(BalanceSyncService::class); + $balanceSync->shouldReceive('sync')->once(); + + $job = new SyncBankingConnectionJob($connection); + + // One dead account must not break the whole connection sync or be reported. + runSync($job, $transactionSync, $balanceSync); + + $connection->refresh(); + expect($connection->status)->toBe(BankingConnectionStatus::Active); + expect($connection->error_message)->toBeNull(); + + $log = BankingSyncLog::where('banking_connection_id', $connection->id)->first(); + expect($log->status)->toBe(BankingSyncLogStatus::Success); + expect($log->metadata['transactions_synced'])->toBe(2); +}); + test('consecutive sync failures accumulate across dispatch cycles', function () { $user = User::factory()->onboarded()->create(); $connection = BankingConnection::factory()->error()->create([