From d7963736d10d98a1d08526ffa95bd2a994a350cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Wed, 15 Jul 2026 08:40:55 +0200 Subject: [PATCH] fix(banking): treat EnableBanking upstream 5xx as transient, not reportable (#678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `EnableBankingProvider::getTransactions` and `getBalances` classify a set of expected `RequestException`s (401 expired session, 400 inaccessible account, 422 wrong period, 400 ASPSP_ERROR) but let **everything else** fall through to a raw `throw $e`. An upstream **500 `Internal server error`** — from EnableBanking itself or the ASPSP behind it — therefore propagates as a plain `RequestException` and gets `report()`ed as an app error. **Sentry:** `PHP-LARAVEL-3J` — `Illuminate\Http\Client\RequestException: HTTP request returned status code 500` in `EnableBankingProvider::getTransactions`, **36 events / 3 users** since 2026-06-16 (regressed 2026-07-15). Nothing per-event our code can do about the bank's server erroring. ## Fix A 5xx is a transient server-side failure — the same class as a `ConnectionException` (no response), which is **already** wrapped as `TransientBankingProviderException` (logged at `warning`, retried, self-healing, `ShouldntReport`). Classify any upstream 5xx the same way in both `getTransactions` and `getBalances`, via a small `isTransientServerError()` helper placed alongside the other classifiers: ```php private function isTransientServerError(RequestException $e): bool { return $e->response->status() >= 500; } ``` So provider outages retry/self-heal instead of paging Sentry. ## Not changed All non-5xx paths are untouched and stay reportable — 401/400/422/ASPSP keep their dedicated exceptions, and genuine 4xx client errors (e.g. the existing "keeps non-ASPSP client errors reportable" / "keeps unrelated 422 validation errors reportable" tests) still surface. ## Test Added two cases to `EnableBankingProviderTest.php` (an upstream 500 on `getTransactions` and on `getBalances` → `TransientBankingProviderException` / `ShouldntReport`). Full provider suite: **14/14 green** locally. 🤖 Found and fixed autonomously via the Sentry monitoring loop. --- .../Banking/EnableBankingProvider.php | 26 ++++++++++ .../OpenBanking/EnableBankingProviderTest.php | 48 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/app/Services/Banking/EnableBankingProvider.php b/app/Services/Banking/EnableBankingProvider.php index 6ae6336b..46bd9115 100644 --- a/app/Services/Banking/EnableBankingProvider.php +++ b/app/Services/Banking/EnableBankingProvider.php @@ -127,6 +127,15 @@ class EnableBankingProvider implements BankingProviderInterface ); } + if ($this->isTransientServerError($e)) { + throw new TransientBankingProviderException( + 'EnableBanking returned a server error while fetching account transactions.', + provider: 'enablebanking', + statusCode: $e->response->status(), + previous: $e, + ); + } + if (! $this->isAspspError($e)) { throw $e; } @@ -178,6 +187,15 @@ class EnableBankingProvider implements BankingProviderInterface ); } + if ($this->isTransientServerError($e)) { + throw new TransientBankingProviderException( + 'EnableBanking returned a server error while fetching account balances.', + provider: 'enablebanking', + statusCode: $e->response->status(), + previous: $e, + ); + } + if (! $this->isAspspError($e)) { throw $e; } @@ -230,6 +248,14 @@ class EnableBankingProvider implements BankingProviderInterface && ($body['error'] ?? null) === 'ASPSP_ERROR'; } + private function isTransientServerError(RequestException $e): bool + { + // Any upstream 5xx (EnableBanking itself or the ASPSP behind it) is a + // transient server-side failure — same class as a ConnectionException, + // so retry/self-heal rather than report it as an app error. + return $e->response->status() >= 500; + } + private function isExpiredSession(RequestException $e): bool { $body = $this->errorBody($e); diff --git a/tests/Feature/OpenBanking/EnableBankingProviderTest.php b/tests/Feature/OpenBanking/EnableBankingProviderTest.php index 78b66a92..ac4d68c1 100644 --- a/tests/Feature/OpenBanking/EnableBankingProviderTest.php +++ b/tests/Feature/OpenBanking/EnableBankingProviderTest.php @@ -59,6 +59,54 @@ test('getTransactions wraps connection failures as non-reportable transient erro test()->fail('Expected transient banking provider exception.'); }); +test('getTransactions wraps an upstream 500 as a non-reportable transient error', function () { + Http::fake([ + 'api.enablebanking.com/accounts/ext-123/transactions*' => Http::response([ + 'code' => 500, + 'message' => 'Internal server error', + ], 500), + ]); + + $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)->toBe(500) + ->and($e->getPrevious())->toBeInstanceOf(RequestException::class); + + return; + } + + test()->fail('Expected transient banking provider exception.'); +}); + +test('getBalances wraps an upstream 500 as a non-reportable transient error', function () { + Http::fake([ + 'api.enablebanking.com/accounts/ext-123/balances*' => Http::response([ + 'code' => 500, + 'message' => 'Internal server error', + ], 500), + ]); + + $provider = enableBankingProviderForTest(); + + try { + $provider->getBalances('ext-123'); + } catch (TransientBankingProviderException $e) { + expect($e)->toBeInstanceOf(ShouldntReport::class) + ->and($e->provider)->toBe('enablebanking') + ->and($e->statusCode)->toBe(500) + ->and($e->getPrevious())->toBeInstanceOf(RequestException::class); + + return; + } + + test()->fail('Expected transient banking provider exception.'); +}); + test('getTransactions wraps an expired session 401 as a non-reportable expired session error', function () { Http::fake([ 'api.enablebanking.com/accounts/ext-123/transactions*' => Http::response([