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`
This commit is contained in:
parent
1024122e57
commit
917a9a655f
|
|
@ -0,0 +1,20 @@
|
|||
<?php
|
||||
|
||||
namespace App\Exceptions\Banking;
|
||||
|
||||
use Exception;
|
||||
use Illuminate\Contracts\Debug\ShouldntReport;
|
||||
use Throwable;
|
||||
|
||||
class TransientBankingProviderException extends Exception implements ShouldntReport
|
||||
{
|
||||
public function __construct(
|
||||
string $message,
|
||||
public readonly ?string $provider = null,
|
||||
public readonly ?int $statusCode = null,
|
||||
public readonly ?string $providerCode = null,
|
||||
?Throwable $previous = null,
|
||||
) {
|
||||
parent::__construct($message, 0, $previous);
|
||||
}
|
||||
}
|
||||
|
|
@ -4,6 +4,7 @@ namespace App\Jobs;
|
|||
|
||||
use App\Enums\BankingConnectionStatus;
|
||||
use App\Enums\BankingSyncLogStatus;
|
||||
use App\Exceptions\Banking\TransientBankingProviderException;
|
||||
use App\Mail\BankingConnectionAuthFailedEmail;
|
||||
use App\Models\BankingConnection;
|
||||
use App\Models\BankingSyncLog;
|
||||
|
|
@ -138,11 +139,19 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue
|
|||
|
||||
$this->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();
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string, mixed>
|
||||
*/
|
||||
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),
|
||||
]);
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,111 @@
|
|||
<?php
|
||||
|
||||
use App\Exceptions\Banking\TransientBankingProviderException;
|
||||
use App\Services\Banking\EnableBankingProvider;
|
||||
use Illuminate\Contracts\Debug\ShouldntReport;
|
||||
use Illuminate\Http\Client\ConnectionException;
|
||||
use Illuminate\Http\Client\RequestException;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
|
||||
test('getTransactions wraps EnableBanking ASPSP errors as non-reportable transient errors', function () {
|
||||
Http::fake([
|
||||
'api.enablebanking.com/accounts/ext-123/transactions*' => 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);
|
||||
}
|
||||
|
|
@ -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([
|
||||
|
|
|
|||
Loading…
Reference in New Issue