fix(banking): handle EnableBanking expired sessions as reconnect, not error (#557)

## Problem Sentry issue [PHP-LARAVEL-3J](https://whisper-money.sentry.io/issues/PHP-LARAVEL-3J) — `RequestException: HTTP 401 {"error":"EXPIRED_SESSION"}` in `EnableBankingProvider::getTransactions` — was escalating (26 events, 2 users). When an EnableBanking **session** expires *before* its 90-day consent window (`valid_until`), the sync hit a `401 EXPIRED_SESSION` that was not classified as transient or ASPSP, so it was re-thrown as a raw `RequestException`. Consequences: 1. **Sentry noise** — reported as an error on every scheduled retry until the failure cap. 2. **Silent breakage for the user** — EnableBanking's syncer returns `notifiesOnAuthFailure() === false`, so the 401 went through the generic permanent-error path: connection set to `Error` (not `Expired`) with **no reconnect email**. Confirmed in production: **10 of 17** EnableBanking connections in `error` status are actually expired sessions (`valid_until` in the future + "Authentication failed" message), all with no notification sent. ## Fix Treat a `401 EXPIRED_SESSION` as the expected lifecycle event it is: - New domain exception `ExpiredBankingSessionException` (implements `ShouldntReport`, like `TransientBankingProviderException`). - `EnableBankingProvider::getTransactions()` / `getBalances()` wrap the `401 EXPIRED_SESSION` in it instead of re-throwing the raw `RequestException`. - `SyncBankingConnectionJob` catches it and routes through the existing expiry handling (extracted to `markExpired()`): marks the connection `Expired`, sends `BankingConnectionExpiredEmail`, logs a skipped attempt, returns **without throwing**. - The provider's HTTP error logger downgrades the expected expiry from `error` → `warning`. `Expired` connections are not re-dispatched by `SyncAllBankingConnectionsJob`, so retries stop and the user is prompted to reconnect. ## Tests - `EnableBankingProviderTest`: `getTransactions` / `getBalances` wrap a `401 EXPIRED_SESSION` as a non-reportable `ExpiredBankingSessionException`. - `SyncRetryAndLoggingTest`: an expired session marks the connection `Expired`, queues the reconnect email, and does not throw. All three failed before the fix. Full `tests/Feature/OpenBanking` suite passes (264 tests). ## Follow-up (not in this PR) The 10 connections already stuck in `Error` from this bug are past the retry cap and won't self-heal. A one-off command to reclassify them to `Expired` and send the reconnect email would recover those users — worth a separate, reviewed change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
2026-06-18 15:48:13 +02:00 · 2026-06-18 15:48:13 +02:00 · c36df98d32
parent 6e6433c6ad
commit c36df98d32
5 changed files with 168 additions and 17 deletions
--- a/app/Exceptions/Banking/ExpiredBankingSessionException.php
+++ b/app/Exceptions/Banking/ExpiredBankingSessionException.php
@ -0,0 +1,17 @@
+<?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,
+        ?Throwable $previous = null,
+    ) {
+        parent::__construct($message, 0, $previous);
+    }
+}
--- a/app/Jobs/SyncBankingConnectionJob.php
+++ b/app/Jobs/SyncBankingConnectionJob.php
@ -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([
--- a/app/Services/Banking/EnableBankingProvider.php
+++ b/app/Services/Banking/EnableBankingProvider.php
@ -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,13 @@ class EnableBankingProvider implements BankingProviderInterface
                previous: $e,
            );
        } catch (RequestException $e) {
+            if ($this->isExpiredSession($e)) {
+                throw new ExpiredBankingSessionException(
+                    'EnableBanking session expired while fetching account transactions.',
+                    previous: $e,
+                );
+            }
+
            if (! $this->isAspspError($e)) {
                throw $e;
            }
@ -140,6 +148,13 @@ class EnableBankingProvider implements BankingProviderInterface
                previous: $e,
            );
        } catch (RequestException $e) {
+            if ($this->isExpiredSession($e)) {
+                throw new ExpiredBankingSessionException(
+                    'EnableBanking session expired while fetching account balances.',
+                    previous: $e,
+                );
+            }
+
            if (! $this->isAspspError($e)) {
                throw $e;
            }
@ -192,6 +207,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 +236,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),
--- a/tests/Feature/OpenBanking/EnableBankingProviderTest.php
+++ b/tests/Feature/OpenBanking/EnableBankingProviderTest.php
@ -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,54 @@ 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->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->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([
--- 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\ExpiredBankingSessionException;
 use App\Exceptions\Banking\TransientBankingProviderException;
 use App\Jobs\SyncAllBankingConnectionsJob;
 use App\Jobs\SyncBankingConnectionJob;
@ -154,6 +155,48 @@ 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.',
+        )
+    );
+
+    $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([