fix(banking): treat EnableBanking upstream 5xx as transient, not reportable (#678)
## 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.
This commit is contained in:
parent
9e237802f6
commit
d7963736d1
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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([
|
||||
|
|
|
|||
Loading…
Reference in New Issue