fix(onboarding): don't trap users on the syncing step when a bank sync fails (#745)

Reported by a user who spent over an hour and a half on the onboarding
syncing step, on two devices, without ever getting into the app.

## What was happening

When Enable Banking rate limits a connection, `SyncBankingConnectionJob`
records the error and backs off, but leaves the connection **Active with
`last_synced_at` still NULL** — the exact shape `syncStatus` treated as
"still syncing". The step polled that endpoint forever, and there was no
deadline or escape on the client.

Confirmed in production for the reporter: a Trade Republic connection
returned `429` on every scheduled sync for two days straight. They
unblocked themselves by deleting the connection — `onboarded_at` was set
four seconds later. Three other users are in the same state right now,
one of them from today.

## What changed

- **`syncStatus` no longer waits on a connection that already failed**,
and reports it as `failed` separately from `pending`.
- **The step says so instead of pretending it worked.** Advancing
silently dropped the user on a "You are all set!" screen with an empty
dashboard and no way to find out why. Now they get a short explanation
and a Continue button.
- **Client deadline raised to 5 minutes** — the previous 2 was under the
worst case of a healthy first sync (3 attempts of a 120s job, 30s apart)
— and the status request now has a timeout, so a hanging poll can't
stall the step either. A failing status check keeps polling until the
deadline rather than skipping the user ahead.
- **`AccountMappingController` clears the stale error** when it
reactivates a connection, so a genuine sync in flight isn't reported as
failed.

## Testing

- Feature tests for the rate-limited case, for a healthy connection
syncing alongside a failed one, and for the failed flag; a
`rateLimited()` factory state replaces the hand-rolled attributes.
- Vitest coverage for the three client branches: failed, deadline
reached, and the normal advance.
- Browser QA against the real app, reproducing the production state: the
honest screen appears instead of the spinner, Continue moves on, the
user completes onboarding and reaches the app; and a genuinely pending
sync still spins and then advances by itself once the connection syncs.
No console errors.

## Demo


https://github.com/user-attachments/assets/e54e029d-db7a-47b0-8b93-0da5569aa363


<!-- PLACEHOLDER: drag the QA video here -->

## Not in this PR

`settings/connections.tsx` has the same `active && !last_synced_at` =
"syncing" heuristic, so a rate-limited connection shows a permanent
green "Syncing" badge there and its error is never rendered (the error
block is gated on `status === 'error'`). Same bug class, separate
surface — worth a follow-up.

app/Http/Controllers/OnboardingController.php

@@ -5,6 +5,7 @@ namespace App\Http\Controllers;
 use App\Enums\BankingConnectionStatus;
 use App\Jobs\CategorizeOnboardingTransactionsJob;
 use App\Models\Bank;
+use App\Models\BankingConnection;
 use App\Models\Category;
 use App\Models\Transaction;
 use Illuminate\Http\JsonResponse;
@@ -75,15 +76,31 @@ class OnboardingController extends Controller
         ]);
     }
 
+    /**
+     * Report whether the onboarding sync step should keep waiting.
+     *
+     * A connection that already recorded an error will never set last_synced_at
+     * (rate limits keep the status Active while only storing the message), so it
+     * must not hold the user on the syncing step forever. It is reported as
+     * failed instead, so the step can say so rather than claim everything worked.
+     */
     public function syncStatus(Request $request): JsonResponse
     {
-        $pending = $request->user()
+        $unsynced = $request->user()
             ->bankingConnections()
-            ->where('status', BankingConnectionStatus::Active)
+            ->whereIn('status', [BankingConnectionStatus::Active, BankingConnectionStatus::Error])
             ->whereNull('last_synced_at')
-            ->exists();
+            ->get(['status', 'error_message']);
 
-        return response()->json(['pending' => $pending]);
+        $pending = $unsynced->contains(
+            fn (BankingConnection $connection): bool => $connection->status === BankingConnectionStatus::Active
+                && $connection->error_message === null
+        );
+
+        return response()->json([
+            'pending' => $pending,
+            'failed' => ! $pending && $unsynced->isNotEmpty(),
+        ]);
     }
 
     public function complete(Request $request): RedirectResponse

app/Http/Controllers/OpenBanking/AccountMappingController.php

@@ -126,6 +126,8 @@ class AccountMappingController extends Controller
         $connection->update([
             'status' => BankingConnectionStatus::Active,
             'pending_accounts_data' => null,
+            'error_message' => null,
+            'consecutive_sync_failures' => 0,
         ]);
 
         SyncBankingConnectionJob::dispatch($connection);

database/factories/BankingConnectionFactory.php

@@ -171,4 +171,18 @@ class BankingConnectionFactory extends Factory
             'consecutive_sync_failures' => 1,
         ]);
     }
+
+    /**
+     * A connection the provider rate limited: still Active, backing off, and
+     * carrying an error message instead of a sync.
+     */
+    public function rateLimited(): static
+    {
+        return $this->state(fn (array $attributes) => [
+            'status' => BankingConnectionStatus::Active,
+            'last_synced_at' => null,
+            'rate_limited_until' => now()->addHour(),
+            'error_message' => 'Rate limit exceeded. Please wait a few minutes and try again.',
+        ]);
+    }
 }

lang/es.json

@@ -1983,6 +1983,7 @@
     "Your :provider connection has expired.": "Tu conexión con :provider ha caducado.",
     "Your :provider connection needs attention": "Tu conexión con :provider necesita atención",
     "Your Accounts": "Tus Cuentas",
+    "Your bank is taking longer than expected. We’ll keep trying in the background and your transactions will show up automatically.": "Tu banco está tardando más de lo previsto. Seguiremos intentándolo en segundo plano y tus transacciones aparecerán automáticamente.",
     "Your Categories Include:": "Tus Categorías Incluyen:",
     "Your Data is Truly Private": "Tus Datos Son Verdaderamente Privados",
     "Your Data, Your Privacy": "Tus Datos, Tu Privacidad",
@@ -2178,6 +2179,7 @@
     "No thanks": "No, gracias",
     "AI suggestions need more data": "Las sugerencias de IA necesitan más datos",
     "Once you have at least :count transactions, you can generate rule suggestions from Settings → Automation rules.": "Cuando tengas al menos :count transacciones, podrás generar sugerencias de reglas desde Ajustes → Reglas de automatización.",
+    "We couldn’t finish importing right now": "No hemos podido terminar la importación",
     "We couldn’t generate suggestions": "No pudimos generar sugerencias",
     "Something went wrong. You can try again or skip for now.": "Algo salió mal. Puedes intentarlo de nuevo u omitirlo por ahora.",
     "Try again": "Intentar de nuevo",

resources/js/components/onboarding/step-syncing.test.tsx

@@ -0,0 +1,81 @@
+import { act, render, screen } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { StepSyncing } from './step-syncing';
+
+const { get } = vi.hoisted(() => ({ get: vi.fn() }));
+
+vi.mock('axios', () => ({
+    default: { get, isAxiosError: () => false },
+}));
+
+const reload = vi.fn();
+
+vi.mock('@inertiajs/react', () => ({
+    router: { reload: (...args: unknown[]) => reload(...args) },
+}));
+
+describe('StepSyncing', () => {
+    beforeEach(() => {
+        vi.useFakeTimers();
+    });
+
+    afterEach(() => {
+        vi.useRealTimers();
+        get.mockReset();
+        reload.mockReset();
+    });
+
+    it('stops spinning and offers to continue when a connection failed', async () => {
+        get.mockResolvedValue({ data: { pending: false, failed: true } });
+
+        render(<StepSyncing onComplete={vi.fn()} />);
+
+        await act(async () => {
+            await vi.advanceTimersByTimeAsync(0);
+        });
+
+        expect(
+            screen.getByText('We couldn’t finish importing right now'),
+        ).toBeInTheDocument();
+        expect(reload).not.toHaveBeenCalled();
+    });
+
+    it('gives up on a sync that never resolves instead of polling forever', async () => {
+        get.mockResolvedValue({ data: { pending: true, failed: false } });
+
+        render(<StepSyncing onComplete={vi.fn()} />);
+
+        await act(async () => {
+            await vi.advanceTimersByTimeAsync(0);
+        });
+        expect(
+            screen.getByText('This will only take a moment.'),
+        ).toBeInTheDocument();
+
+        await act(async () => {
+            await vi.advanceTimersByTimeAsync(5 * 60_000 + 3_000);
+        });
+
+        expect(
+            screen.getByText('We couldn’t finish importing right now'),
+        ).toBeInTheDocument();
+        expect(
+            screen.queryByText('This will only take a moment.'),
+        ).not.toBeInTheDocument();
+    });
+
+    it('advances on its own once every connection has synced', async () => {
+        get.mockResolvedValue({ data: { pending: false, failed: false } });
+
+        render(<StepSyncing onComplete={vi.fn()} />);
+
+        await act(async () => {
+            await vi.advanceTimersByTimeAsync(0);
+        });
+
+        expect(reload).toHaveBeenCalled();
+        expect(
+            screen.queryByText('We couldn’t finish importing right now'),
+        ).not.toBeInTheDocument();
+    });
+});

resources/js/components/onboarding/step-syncing.tsx

@@ -1,10 +1,19 @@
+import { StepButton } from '@/components/onboarding/step-button';
+import { StepHeader } from '@/components/onboarding/step-header';
 import { syncStatus } from '@/routes/onboarding';
 import { __ } from '@/utils/i18n';
 import { router } from '@inertiajs/react';
 import axios from 'axios';
-import { Loader2 } from 'lucide-react';
+import { CloudOff, Loader2 } from 'lucide-react';
 import { useCallback, useEffect, useRef, useState } from 'react';
 
+// Client-side give-up: the sync keeps running on the queue, but this guarantees
+// the spinner resolves even if the worker dies. Beyond the worst case a healthy
+// first sync can take (3 attempts of a 120s job, 30s apart).
+const MAX_POLL_MS = 5 * 60_000;
+
+const POLL_INTERVAL_MS = 3_000;
+
 const MESSAGES = [
     'Importing your balances...',
     'Fetching your transactions...',
@@ -24,6 +33,7 @@ interface StepSyncingProps {
 export function StepSyncing({ onComplete }: StepSyncingProps) {
     const [messageIndex, setMessageIndex] = useState(0);
     const [isPending, setIsPending] = useState<boolean | null>(null);
+    const [hasStalled, setHasStalled] = useState(false);
     const onCompleteRef = useRef(onComplete);
     onCompleteRef.current = onComplete;
 
@@ -40,28 +50,49 @@ export function StepSyncing({ onComplete }: StepSyncingProps) {
     useEffect(() => {
         let cancelled = false;
         let pollTimer: ReturnType<typeof setTimeout>;
+        const deadline = Date.now() + MAX_POLL_MS;
+
+        const stall = () => {
+            setIsPending(false);
+            setHasStalled(true);
+        };
+
+        // A failing status check says nothing about the sync itself, so both the
+        // still-pending and the request-failed paths keep polling until the
+        // deadline rather than dropping the user into the next step early.
+        const keepPolling = () => {
+            if (Date.now() > deadline) {
+                stall();
+
+                return;
+            }
+
+            setIsPending(true);
+            pollTimer = setTimeout(() => check(), POLL_INTERVAL_MS);
+        };
 
         const check = async () => {
             try {
-                const { data } = await axios.get<{ pending: boolean }>(
-                    syncStatus().url,
-                );
+                const { data } = await axios.get<{
+                    pending: boolean;
+                    failed: boolean;
+                }>(syncStatus().url, { timeout: POLL_INTERVAL_MS });
 
                 if (cancelled) {
                     return;
                 }
 
-                if (!data.pending) {
+                if (data.failed) {
+                    stall();
+                } else if (data.pending) {
+                    keepPolling();
+                } else {
                     setIsPending(false);
                     advance();
-                } else {
-                    setIsPending(true);
-                    pollTimer = setTimeout(() => check(), 3000);
                 }
             } catch {
                 if (!cancelled) {
-                    // On error, advance anyway to not block the user
-                    advance();
+                    keepPolling();
                 }
             }
         };
@@ -87,6 +118,22 @@ export function StepSyncing({ onComplete }: StepSyncingProps) {
         return () => clearInterval(interval);
     }, [isPending]);
 
+    if (hasStalled) {
+        return (
+            <div className="flex animate-in flex-col items-center gap-6 pb-4 duration-500 fade-in slide-in-from-bottom-4">
+                <StepHeader
+                    icon={CloudOff}
+                    iconContainerClassName="bg-gradient-to-br from-violet-500 to-purple-600"
+                    title={__('We couldn’t finish importing right now')}
+                    description={__(
+                        'Your bank is taking longer than expected. We’ll keep trying in the background and your transactions will show up automatically.',
+                    )}
+                />
+                <StepButton text={__('Continue')} onClick={advance} />
+            </div>
+        );
+    }
+
     // Don't render anything until we know sync is pending
     if (!isPending) {
         return null;

tests/Feature/Onboarding/OnboardingSyncStatusTest.php

@@ -54,6 +54,46 @@ it('returns pending false when unsynced connection has an error status', functio
         ->assertJson(['pending' => false]);
 });
 
+it('reports a rate limited connection as failed instead of pending', function () {
+    $user = User::factory()->create(['onboarded_at' => null]);
+
+    BankingConnection::factory()->for($user)->rateLimited()->create();
+
+    $this->actingAs($user)
+        ->getJson('/onboarding/sync-status')
+        ->assertOk()
+        ->assertJson(['pending' => false, 'failed' => true]);
+});
+
+it('keeps waiting while a healthy connection syncs alongside a failed one', function () {
+    $user = User::factory()->create(['onboarded_at' => null]);
+
+    BankingConnection::factory()->for($user)->rateLimited()->create();
+    BankingConnection::factory()->for($user)->create([
+        'status' => BankingConnectionStatus::Active,
+        'last_synced_at' => null,
+    ]);
+
+    $this->actingAs($user)
+        ->getJson('/onboarding/sync-status')
+        ->assertOk()
+        ->assertJson(['pending' => true, 'failed' => false]);
+});
+
+it('does not report a synced connection as failed', function () {
+    $user = User::factory()->create(['onboarded_at' => null]);
+
+    BankingConnection::factory()->for($user)->create([
+        'status' => BankingConnectionStatus::Active,
+        'last_synced_at' => now(),
+    ]);
+
+    $this->actingAs($user)
+        ->getJson('/onboarding/sync-status')
+        ->assertOk()
+        ->assertJson(['pending' => false, 'failed' => false]);
+});
+
 it('returns pending false when unsynced connection is revoked', function () {
     $user = User::factory()->create(['onboarded_at' => null]);