From 917a9a655fb5dbdbcfca12a2114862365a05297d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Wed, 6 May 2026 08:24:05 +0100 Subject: [PATCH] Handle transient EnableBanking sync failures (#358) ## Problem Sentry showed two related production issues in the same sync path: - `PHP-LARAVEL-13`: EnableBanking returned HTTP `400` from `GET /accounts/{accountId}/transactions` with body `ASPSP_ERROR` / `Error interacting with ASPSP`. This is an upstream bank/provider failure, but the app threw an unhandled `RequestException`. - `PHP-LARAVEL-14`: the same transactions endpoint timed out after 20s, throwing an unhandled `ConnectionException`. Both originated from `EnableBankingProvider::getTransactions()` and bubbled through `SyncBankingConnectionJob`, creating Sentry issues for expected transient provider/bank outages. ## New behavior - Wrap EnableBanking `400 ASPSP_ERROR` responses in `TransientBankingProviderException`. - Wrap EnableBanking transaction connection failures / timeouts in the same transient exception. - Mark that exception as `ShouldntReport`, so these expected upstream failures stop creating Sentry issues. - Keep queue retry behavior intact. The job still retries and only marks the connection as `Error` after normal retry exhaustion. - Log transient sync failures as warnings instead of errors. - Show users a retry-later message when retries are exhausted: the bank provider is temporarily unavailable. - Leave other `400` responses reportable. Validation / app-side request bugs still throw `RequestException`. - Leave auth failures and rate-limit handling unchanged. Fixes PHP-LARAVEL-13 Fixes PHP-LARAVEL-14 ## Testing - `vendor/bin/pint --dirty --format agent` - `php artisan test --compact tests/Feature/OpenBanking/EnableBankingProviderTest.php tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php tests/Feature/OpenBanking/SyncBankingConnectionJobTest.php` --- .../TransientBankingProviderException.php | 20 ++++ app/Jobs/SyncBankingConnectionJob.php | 17 ++- .../Banking/EnableBankingProvider.php | 58 ++++++++- .../OpenBanking/EnableBankingProviderTest.php | 111 ++++++++++++++++++ .../OpenBanking/SyncRetryAndLoggingTest.php | 48 ++++++++ 5 files changed, 248 insertions(+), 6 deletions(-) create mode 100644 app/Exceptions/Banking/TransientBankingProviderException.php create mode 100644 tests/Feature/OpenBanking/EnableBankingProviderTest.php diff --git a/app/Exceptions/Banking/TransientBankingProviderException.php b/app/Exceptions/Banking/TransientBankingProviderException.php new file mode 100644 index 00000000..2d82ea61 --- /dev/null +++ b/app/Exceptions/Banking/TransientBankingProviderException.php @@ -0,0 +1,20 @@ +logSyncAttempt($connection, BankingSyncLogStatus::Success, $startTime, metadata: $metadata ?: null); } catch (\Throwable $e) { - Log::error('Banking sync failed', [ + $context = [ 'connection_id' => $connection->id, 'error' => $e->getMessage(), 'attempt' => $this->attempts(), - ]); + ]; + + if ($e instanceof TransientBankingProviderException) { + $context['provider'] = $e->provider; + $context['status_code'] = $e->statusCode; + $context['provider_code'] = $e->providerCode; + } + + Log::log($e instanceof TransientBankingProviderException ? 'warning' : 'error', 'Banking sync failed', $context); if ($this->isRateLimitError($e)) { $this->applyRateLimitBackoff($connection, $e); @@ -369,6 +378,10 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue private function friendlyErrorMessage(\Throwable $e): string { + if ($e instanceof TransientBankingProviderException) { + return __('The bank provider is temporarily unavailable. We will try syncing again later.'); + } + if ($e instanceof RequestException) { $status = $e->response->status(); diff --git a/app/Services/Banking/EnableBankingProvider.php b/app/Services/Banking/EnableBankingProvider.php index 87359173..1fe589f1 100644 --- a/app/Services/Banking/EnableBankingProvider.php +++ b/app/Services/Banking/EnableBankingProvider.php @@ -3,8 +3,11 @@ namespace App\Services\Banking; use App\Contracts\BankingProviderInterface; +use App\Exceptions\Banking\TransientBankingProviderException; use Firebase\JWT\JWT; +use Illuminate\Http\Client\ConnectionException; use Illuminate\Http\Client\PendingRequest; +use Illuminate\Http\Client\RequestException; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; @@ -89,9 +92,32 @@ class EnableBankingProvider implements BankingProviderInterface $query['strategy'] = $strategy; } - $response = $this->client()->get("/accounts/{$accountId}/transactions", $query); + try { + $response = $this->client()->get("/accounts/{$accountId}/transactions", $query); - $response->throw(); + $response->throw(); + } catch (ConnectionException $e) { + throw new TransientBankingProviderException( + 'EnableBanking did not respond while fetching account transactions.', + provider: 'enablebanking', + previous: $e, + ); + } catch (RequestException $e) { + if (! $this->isAspspError($e)) { + throw $e; + } + + $body = $this->errorBody($e); + $providerCode = $body['error'] ?? null; + + throw new TransientBankingProviderException( + 'EnableBanking bank connector failed while fetching account transactions.', + provider: 'enablebanking', + statusCode: $e->response->status(), + providerCode: is_string($providerCode) ? $providerCode : null, + previous: $e, + ); + } $data = $response->json(); @@ -135,6 +161,24 @@ class EnableBankingProvider implements BankingProviderInterface $response->throw(); } + private function isAspspError(RequestException $e): bool + { + $body = $this->errorBody($e); + + return $e->response->status() === 400 + && ($body['error'] ?? null) === 'ASPSP_ERROR'; + } + + /** + * @return array + */ + private function errorBody(RequestException $e): array + { + $body = $e->response->json(); + + return is_array($body) ? $body : []; + } + private function client(): PendingRequest { return Http::baseUrl(self::BASE_URL) @@ -143,9 +187,15 @@ class EnableBankingProvider implements BankingProviderInterface ->withToken($this->generateJwt()) ->acceptJson() ->throw(function ($response, $exception) { - Log::error('EnableBanking API error', [ + $body = $response->json(); + $isAspspError = $response->status() === 400 + && is_array($body) + && ($body['error'] ?? null) === 'ASPSP_ERROR'; + + Log::log($isAspspError ? 'warning' : 'error', 'EnableBanking API error', [ 'status' => $response->status(), - 'body' => $response->json(), + 'body' => $body, + 'exception' => get_class($exception), ]); }); } diff --git a/tests/Feature/OpenBanking/EnableBankingProviderTest.php b/tests/Feature/OpenBanking/EnableBankingProviderTest.php new file mode 100644 index 00000000..1379f5f7 --- /dev/null +++ b/tests/Feature/OpenBanking/EnableBankingProviderTest.php @@ -0,0 +1,111 @@ + Http::response([ + 'code' => 400, + 'message' => 'Error interacting with ASPSP', + 'detail' => 'Unknown error', + 'error' => 'ASPSP_ERROR', + ], 400), + ]); + + $provider = enableBankingProviderForTest(); + + try { + $provider->getTransactions('ext-123', '2025-05-05', '2026-05-05', strategy: 'longest'); + } catch (TransientBankingProviderException $e) { + expect($e)->toBeInstanceOf(ShouldntReport::class) + ->and($e->provider)->toBe('enablebanking') + ->and($e->statusCode)->toBe(400) + ->and($e->providerCode)->toBe('ASPSP_ERROR') + ->and($e->getPrevious())->toBeInstanceOf(RequestException::class); + + return; + } + + test()->fail('Expected transient banking provider exception.'); +}); + +test('getTransactions wraps connection failures as non-reportable transient errors', function () { + Http::fake([ + 'api.enablebanking.com/accounts/ext-123/transactions*' => Http::failedConnection(), + ]); + + $provider = enableBankingProviderForTest(); + + try { + $provider->getTransactions('ext-123', now()->toDateString(), now()->toDateString()); + } catch (TransientBankingProviderException $e) { + expect($e)->toBeInstanceOf(ShouldntReport::class) + ->and($e->provider)->toBe('enablebanking') + ->and($e->statusCode)->toBeNull() + ->and($e->providerCode)->toBeNull() + ->and($e->getPrevious())->toBeInstanceOf(ConnectionException::class); + + return; + } + + test()->fail('Expected transient banking provider exception.'); +}); + +test('getTransactions keeps non-ASPSP client errors reportable', function () { + Http::fake([ + 'api.enablebanking.com/accounts/ext-123/transactions*' => Http::response([ + 'code' => 400, + 'message' => 'Invalid date range', + 'error' => 'VALIDATION_ERROR', + ], 400), + ]); + + $provider = enableBankingProviderForTest(); + + expect(fn () => $provider->getTransactions('ext-123', 'bad-date', now()->toDateString())) + ->toThrow(RequestException::class); +}); + +function enableBankingProviderForTest(): EnableBankingProvider +{ + $privateKey = <<<'PEM' +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDWoizjYmPaLQqn +uGJQxJCl18MxlJmTgoDzITt/hIW2CEFegbuKuynz7HCFM7xdAg6WRmHfOevLXVuq ++erPk9gcqC1ePLWzwzmNLIIPpPrO4pkFTZF91T46kJY9/J6QclzbrbI4wB9l3SKA +14h0O2R2sh1DubnSN5H7JeHyZtIal+aJe7jxuLyKxKkWY80a/jq7rGIzQFJFCFFV +zLcRKyqs80l4nGLT00lubmlJj1y2/p0OH7B8ZLwxr2LrH+NAPw9L6/e8jEhHSxHs +LLgOeCEIHO3f7tAfWN6dld08I9puT5JtXp8c5OpkrciDD5C3HvOGjQFNj/W7EmRg +GVIBeDf7AgMBAAECggEAAJOXLJWl9T70krfCfztGFx3MNtmv/P8GF0OPFp/KnsU1 +SoMenxzkb8OkyPYyMPxhi0PemEdAvlByTnk6EwxvgEoNDNa2rXb5gy1zUCPUWMrq +806Ur9AI3Muj7/s57LvJ6HMnalyb58BBvEbwjLNgmiEsRhrML8pA9hd4sGam/vq/ +Xb1BoT8FRPVlmz32w9RFrcQaZ4tO/r8rRNlWFtEV0iOdocK+4NizJvJvCyPYesck +F8+wAoPrHARSOhmzWfzYXXFwJdXcpkuMshQ+COzD2TTZnTZbRn8tWMcL32Bb9b55 +E1CKVPUB99eE182oCHaWNE7HO+2VbMFqExU9oZU+fQKBgQD8HjrFlP234WDChook +ED5btDxJqSpGuHvzgP083Ej8sOLtWcpVJOFEsLiKRzUqBm6wjFrfk8yq+Vk3OgoA +CDV6owfQGwn0Jj7yhYPDlMUf1mqytbeFSrziIaFs8YcV1nxykXbJCQyDIAhjOlXf +je9SifsrBDxOv6re2ky8mzzp1QKBgQDZ8DF64aEntI78SP6CW72fUrQkA9HOnV5s +dZLE/RbybTG/oozjJzJ0OTHiwtz14UVxTXTCEkF4nsrv9W19pw5E/C4wLqq7tdDn +gXxS0CAQ1zCBqQAMrgeMA+mmNc3j7rp/TthMQ+Z+wStOqptkIvigv6EZ/9+jzdSA +C5O5nq4yjwKBgEzhpwhze79kIg6P2nZO4cUzPCM2S+cPAPVrg03Y2wT7p+e7NuEq +AuvgfBXmywaKuZxq4JdHSeVlblhSAZSq7Cv+pTZH2Iw0UYPBRUISDt67kwP2OAWU +me7XVJOVP51gL8j8JN3/PWqLDSO9OUyXysA/xXEDtKRK/H9C0J2/NR8VAoGASr4B +ei8fYcqerw8pmfN0mMt4VFGrBr0ZwQChkUVrNUEVqq9Iui6bMxjabvZ9aSYU9sKl +pFk2cvOijaESJ+G/FxGVlZirnSzBtGPIC26tUJk8XXtkNPUKSY6d9w7EycL52udj +buRqjFYbUCNan4EO27JcwdnrDPZuRmuyAhrViykCgYEA4pLCByU4uISinHpFKWD4 +TMGRZNdyFw1UWET/t3UgYA05iFzgrlaz5WtWy27LVHGIpDZqmR/pqw43tsOX67qi +r6aIG0QnM0a0BlAPUi+7BBZL76TatYBoYlqbvLOaRRaYsL4s4jGph+KUS4Sr/JmK ++Y9QVqKpHPmUKWPRdA7INQ0= +-----END PRIVATE KEY----- +PEM; + + $path = sys_get_temp_dir().'/enablebanking-test-key.pem'; + file_put_contents($path, $privateKey); + + return new EnableBankingProvider('test-app-id', $path); +} diff --git a/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php b/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php index e229851c..cdc3967b 100644 --- a/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php +++ b/tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php @@ -2,6 +2,7 @@ use App\Enums\BankingConnectionStatus; use App\Enums\BankingSyncLogStatus; +use App\Exceptions\Banking\TransientBankingProviderException; use App\Jobs\SyncAllBankingConnectionsJob; use App\Jobs\SyncBankingConnectionJob; use App\Models\Account; @@ -104,6 +105,53 @@ test('temporary error on final attempt sets error status and increments consecut expect($connection->consecutive_sync_failures)->toBe(1); }); +test('transient banking provider error on final attempt uses retry later message', 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', + ]); + + $transactionSync = Mockery::mock(TransactionSyncService::class); + $transactionSync->shouldReceive('sync')->andThrow( + new TransientBankingProviderException( + 'EnableBanking bank connector failed while fetching account transactions.', + provider: 'enablebanking', + statusCode: 400, + providerCode: 'ASPSP_ERROR', + ) + ); + + $balanceSync = Mockery::mock(BalanceSyncService::class); + + $job = new SyncBankingConnectionJob($connection); + $job->job = Mockery::mock(Job::class); + $job->job->shouldReceive('attempts')->andReturn(3); + $job->job->shouldReceive('isReleased')->andReturn(false); + $job->job->shouldReceive('isDeletedOrReleased')->andReturn(false); + $job->job->shouldReceive('hasFailed')->andReturn(false); + + $threw = false; + + try { + $job->handle($transactionSync, $balanceSync); + } catch (TransientBankingProviderException) { + $threw = true; + } + + expect($threw)->toBeTrue(); + + $connection->refresh(); + expect($connection->status)->toBe(BankingConnectionStatus::Error); + expect($connection->error_message)->toContain('bank provider is temporarily unavailable'); + expect($connection->consecutive_sync_failures)->toBe(1); +}); + test('consecutive sync failures accumulate across dispatch cycles', function () { $user = User::factory()->onboarded()->create(); $connection = BankingConnection::factory()->error()->create([