fix(banking): handle EnableBanking expired sessions as reconnect, not error
When an EnableBanking session expires before its consent window (HTTP 401 `EXPIRED_SESSION`), the sync re-threw a raw RequestException. That meant the event was reported to Sentry on every scheduled retry (escalating noise) and, because EnableBanking does not notify on auth failure, the user was left with a silently broken connection stuck in `Error` status and no email telling them to reconnect. Treat it as the lifecycle event it is: - The provider now wraps a 401 `EXPIRED_SESSION` in a new `ExpiredBankingSessionException` (implements `ShouldntReport`). - The sync job catches it and routes through the existing expiry path: marks the connection `Expired`, emails `BankingConnectionExpiredEmail`, logs a skipped attempt, and returns without throwing. - The provider's HTTP error logger downgrades the expected expiry from `error` to `warning`. Reuses the date-based expiry handling via a shared `markExpired()` helper.
This commit is contained in:
parent
801f6c7cd4
commit
5ad95f03a9
|
|
@ -0,0 +1,18 @@
|
|||
<?php
|
||||
|
||||
namespace App\Exceptions\Banking;
|
||||
|
||||
use Exception;
|
||||
use Illuminate\Contracts\Debug\ShouldntReport;
|
||||
use Throwable;
|
||||
|
||||
class ExpiredBankingSessionException extends Exception implements ShouldntReport
|
||||
{
|
||||
public function __construct(
|
||||
string $message,
|
||||
public readonly ?string $provider = null,
|
||||
?Throwable $previous = null,
|
||||
) {
|
||||
parent::__construct($message, 0, $previous);
|
||||
}
|
||||
}
|
||||
|
|
@ -5,6 +5,7 @@ namespace App\Jobs;
|
|||
use App\Contracts\BankingConnectionSyncer;
|
||||
use App\Enums\BankingConnectionStatus;
|
||||
use App\Enums\BankingSyncLogStatus;
|
||||
use App\Exceptions\Banking\ExpiredBankingSessionException;
|
||||
use App\Exceptions\Banking\TransientBankingProviderException;
|
||||
use App\Mail\BankingConnectionAuthFailedEmail;
|
||||
use App\Mail\BankingConnectionExpiredEmail;
|
||||
|
|
@ -71,19 +72,7 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue
|
|||
$syncer = $syncerFactory->make($connection);
|
||||
|
||||
if ($syncer->expires() && $connection->isExpired()) {
|
||||
$shouldNotify = $connection->status !== BankingConnectionStatus::Expired;
|
||||
|
||||
$connection->update(['status' => BankingConnectionStatus::Expired]);
|
||||
Log::info('Banking connection expired, skipping sync', ['connection_id' => $connection->id]);
|
||||
|
||||
if ($shouldNotify && $connection->user->canReceiveEmails()) {
|
||||
Mail::to($connection->user)->send(new BankingConnectionExpiredEmail(
|
||||
$connection->user,
|
||||
$connection,
|
||||
));
|
||||
}
|
||||
|
||||
$this->logSyncAttempt($connection, BankingSyncLogStatus::Skipped, $startTime, metadata: ['reason' => 'expired']);
|
||||
$this->markExpired($connection, $startTime);
|
||||
|
||||
return;
|
||||
}
|
||||
|
|
@ -122,6 +111,10 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue
|
|||
]);
|
||||
|
||||
$this->logSyncAttempt($connection, BankingSyncLogStatus::Success, $startTime, metadata: $metadata ?: null);
|
||||
} catch (ExpiredBankingSessionException) {
|
||||
$this->markExpired($connection, $startTime);
|
||||
|
||||
return;
|
||||
} catch (\Throwable $e) {
|
||||
$context = [
|
||||
'connection_id' => $connection->id,
|
||||
|
|
@ -178,6 +171,30 @@ class SyncBankingConnectionJob implements ShouldBeUnique, ShouldQueue
|
|||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark the connection as expired and notify the user to reconnect.
|
||||
*
|
||||
* Reached both when the stored consent window lapses and when the provider
|
||||
* reports the session itself has expired mid-sync. Either way it is an
|
||||
* expected lifecycle event, not a failure to report.
|
||||
*/
|
||||
private function markExpired(BankingConnection $connection, float $startTime): void
|
||||
{
|
||||
$shouldNotify = $connection->status !== BankingConnectionStatus::Expired;
|
||||
|
||||
$connection->update(['status' => BankingConnectionStatus::Expired]);
|
||||
Log::info('Banking connection expired, skipping sync', ['connection_id' => $connection->id]);
|
||||
|
||||
if ($shouldNotify && $connection->user?->canReceiveEmails()) {
|
||||
Mail::to($connection->user)->send(new BankingConnectionExpiredEmail(
|
||||
$connection->user,
|
||||
$connection,
|
||||
));
|
||||
}
|
||||
|
||||
$this->logSyncAttempt($connection, BankingSyncLogStatus::Skipped, $startTime, metadata: ['reason' => 'expired']);
|
||||
}
|
||||
|
||||
private function handlePermanentError(BankingConnection $connection, BankingConnectionSyncer $syncer, \Throwable $e): void
|
||||
{
|
||||
$connection->update([
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
namespace App\Services\Banking;
|
||||
|
||||
use App\Contracts\BankingProviderInterface;
|
||||
use App\Exceptions\Banking\ExpiredBankingSessionException;
|
||||
use App\Exceptions\Banking\TransientBankingProviderException;
|
||||
use Firebase\JWT\JWT;
|
||||
use Illuminate\Http\Client\ConnectionException;
|
||||
|
|
@ -103,6 +104,14 @@ class EnableBankingProvider implements BankingProviderInterface
|
|||
previous: $e,
|
||||
);
|
||||
} catch (RequestException $e) {
|
||||
if ($this->isExpiredSession($e)) {
|
||||
throw new ExpiredBankingSessionException(
|
||||
'EnableBanking session expired while fetching account transactions.',
|
||||
provider: 'enablebanking',
|
||||
previous: $e,
|
||||
);
|
||||
}
|
||||
|
||||
if (! $this->isAspspError($e)) {
|
||||
throw $e;
|
||||
}
|
||||
|
|
@ -140,6 +149,14 @@ class EnableBankingProvider implements BankingProviderInterface
|
|||
previous: $e,
|
||||
);
|
||||
} catch (RequestException $e) {
|
||||
if ($this->isExpiredSession($e)) {
|
||||
throw new ExpiredBankingSessionException(
|
||||
'EnableBanking session expired while fetching account balances.',
|
||||
provider: 'enablebanking',
|
||||
previous: $e,
|
||||
);
|
||||
}
|
||||
|
||||
if (! $this->isAspspError($e)) {
|
||||
throw $e;
|
||||
}
|
||||
|
|
@ -192,6 +209,16 @@ class EnableBankingProvider implements BankingProviderInterface
|
|||
&& ($body['error'] ?? null) === 'ASPSP_ERROR';
|
||||
}
|
||||
|
||||
private function isExpiredSession(RequestException $e): bool
|
||||
{
|
||||
$body = $this->errorBody($e);
|
||||
|
||||
// ponytail: only the documented EXPIRED_SESSION code; widen if other
|
||||
// terminal "reconnect required" session codes (e.g. revoked) surface.
|
||||
return $e->response->status() === 401
|
||||
&& ($body['error'] ?? null) === 'EXPIRED_SESSION';
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array<string, mixed>
|
||||
*/
|
||||
|
|
@ -211,11 +238,11 @@ class EnableBankingProvider implements BankingProviderInterface
|
|||
->acceptJson()
|
||||
->throw(function ($response, $exception) {
|
||||
$body = $response->json();
|
||||
$isAspspError = $response->status() === 400
|
||||
&& is_array($body)
|
||||
&& ($body['error'] ?? null) === 'ASPSP_ERROR';
|
||||
$error = is_array($body) ? ($body['error'] ?? null) : null;
|
||||
$isExpected = ($response->status() === 400 && $error === 'ASPSP_ERROR')
|
||||
|| ($response->status() === 401 && $error === 'EXPIRED_SESSION');
|
||||
|
||||
Log::log($isAspspError ? 'warning' : 'error', 'EnableBanking API error', [
|
||||
Log::log($isExpected ? 'warning' : 'error', 'EnableBanking API error', [
|
||||
'status' => $response->status(),
|
||||
'body' => $body,
|
||||
'exception' => get_class($exception),
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
<?php
|
||||
|
||||
use App\Exceptions\Banking\ExpiredBankingSessionException;
|
||||
use App\Exceptions\Banking\TransientBankingProviderException;
|
||||
use App\Services\Banking\EnableBankingProvider;
|
||||
use Illuminate\Contracts\Debug\ShouldntReport;
|
||||
|
|
@ -56,6 +57,56 @@ test('getTransactions wraps connection failures as non-reportable transient erro
|
|||
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([
|
||||
'code' => 401,
|
||||
'message' => 'Session is expired',
|
||||
'error' => 'EXPIRED_SESSION',
|
||||
'detail' => null,
|
||||
], 401),
|
||||
]);
|
||||
|
||||
$provider = enableBankingProviderForTest();
|
||||
|
||||
try {
|
||||
$provider->getTransactions('ext-123', now()->toDateString(), now()->toDateString());
|
||||
} catch (ExpiredBankingSessionException $e) {
|
||||
expect($e)->toBeInstanceOf(ShouldntReport::class)
|
||||
->and($e->provider)->toBe('enablebanking')
|
||||
->and($e->getPrevious())->toBeInstanceOf(RequestException::class);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
test()->fail('Expected expired banking session exception.');
|
||||
});
|
||||
|
||||
test('getBalances wraps an expired session 401 as a non-reportable expired session error', function () {
|
||||
Http::fake([
|
||||
'api.enablebanking.com/accounts/ext-123/balances' => Http::response([
|
||||
'code' => 401,
|
||||
'message' => 'Session is expired',
|
||||
'error' => 'EXPIRED_SESSION',
|
||||
'detail' => null,
|
||||
], 401),
|
||||
]);
|
||||
|
||||
$provider = enableBankingProviderForTest();
|
||||
|
||||
try {
|
||||
$provider->getBalances('ext-123');
|
||||
} catch (ExpiredBankingSessionException $e) {
|
||||
expect($e)->toBeInstanceOf(ShouldntReport::class)
|
||||
->and($e->provider)->toBe('enablebanking')
|
||||
->and($e->getPrevious())->toBeInstanceOf(RequestException::class);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
test()->fail('Expected expired banking session exception.');
|
||||
});
|
||||
|
||||
test('getTransactions keeps non-ASPSP client errors reportable', function () {
|
||||
Http::fake([
|
||||
'api.enablebanking.com/accounts/ext-123/transactions*' => Http::response([
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
use App\Enums\BankingConnectionStatus;
|
||||
use App\Enums\BankingSyncLogStatus;
|
||||
use App\Exceptions\Banking\ExpiredBankingSessionException;
|
||||
use App\Exceptions\Banking\TransientBankingProviderException;
|
||||
use App\Jobs\SyncAllBankingConnectionsJob;
|
||||
use App\Jobs\SyncBankingConnectionJob;
|
||||
|
|
@ -154,6 +155,49 @@ test('transient banking provider error on final attempt uses retry later message
|
|||
expect($connection->consecutive_sync_failures)->toBe(1);
|
||||
});
|
||||
|
||||
test('expired session marks the connection expired and emails the user instead of reporting an error', function () {
|
||||
Mail::fake();
|
||||
|
||||
$user = User::factory()->onboarded()->create();
|
||||
$connection = BankingConnection::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'last_synced_at' => now()->subDay(),
|
||||
'valid_until' => now()->addMonths(3),
|
||||
]);
|
||||
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 ExpiredBankingSessionException(
|
||||
'EnableBanking session expired while fetching account transactions.',
|
||||
provider: 'enablebanking',
|
||||
)
|
||||
);
|
||||
|
||||
$balanceSync = Mockery::mock(BalanceSyncService::class);
|
||||
|
||||
$job = new SyncBankingConnectionJob($connection);
|
||||
|
||||
// An expired session is an expected lifecycle event: it must not throw or be reported.
|
||||
runSync($job, $transactionSync, $balanceSync);
|
||||
|
||||
$connection->refresh();
|
||||
expect($connection->status)->toBe(BankingConnectionStatus::Expired);
|
||||
|
||||
$log = BankingSyncLog::where('banking_connection_id', $connection->id)->first();
|
||||
expect($log->status)->toBe(BankingSyncLogStatus::Skipped);
|
||||
expect($log->metadata)->toBe(['reason' => 'expired']);
|
||||
|
||||
Mail::assertQueued(BankingConnectionExpiredEmail::class, function (BankingConnectionExpiredEmail $mail) use ($user, $connection) {
|
||||
return $mail->user->is($user)
|
||||
&& $mail->bankingConnection->is($connection);
|
||||
});
|
||||
});
|
||||
|
||||
test('consecutive sync failures accumulate across dispatch cycles', function () {
|
||||
$user = User::factory()->onboarded()->create();
|
||||
$connection = BankingConnection::factory()->error()->create([
|
||||
|
|
|
|||
Loading…
Reference in New Issue