From 5ec62d86250211e2cb9d2b86da3ff22b095ad629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Tue, 11 Aug 2026 18:12:00 +0200 Subject: [PATCH] fix(observability): stop our own page navigations from reporting fake network errors (#780) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## The issue `AxiosError: Network Error` (PHP-LARAVEL-28) is the noisiest issue in this project — 192 events / 75 users, `handled: no` — and it has been archived as ambient connectivity noise several times, including by me. The event distribution says otherwise: | URL | events (last 24) | |---|---| | `/onboarding` | 15 | | `/register` | 3 | | `/dashboard`, `/accounts`, `/accounts/{id}`, `/settings/connections`, `/` | 1 each | Every one of those is a page that triggers a full-page navigation out of the SPA, and the events skew heavily to Safari/macOS. ## The mechanism Assigning `window.location` aborts every request still in flight. Browsers report that abort to XHR through `onerror`, as a transport failure rather than a cancellation — which is why it arrives as `Network Error` and not `Request aborted`. Inertia rethrows it, so it lands as an unhandled rejection. The onboarding bank-connection step is the perfect generator: it polls every 4s (`usePoll` in `pages/onboarding/index.tsx:124`) and *then* sends the user to their bank with `window.location.href` (`hooks/use-connect-flow.ts:219`). A poll dying mid-flight is close to guaranteed. So this is a bug report for a request that never failed. The point isn't the volume — it's that until now a real "the user's connection dropped and their action silently did nothing" was indistinguishable from our own navigations. ## The change `leavePage()` / `reloadPage()` record the departure; a `beforeSend` predicate drops transport-level failures while that flag is set. It follows the five sibling noise predicates already in `lib/sentry.ts`, so Inertia's internals stay untouched. 13 call sites converted. Commits are one-per-finding from the two reviews, and the two interesting ones are corrections to my own first attempt: - **`b7dc8dc6` — the first version was inert.** `HttpError`'s constructor appends the request URL (`super(url ? \`${message} (${url})\` : message)`), so Inertia's XHR client — the default in v3.6.1, which we moved to in #769 — rejects with `Network error (https://whisper.money/onboarding?step=create-account)`, not `Network error`. My anchored regex matched neither. The 192 sampled events read `AxiosError: Network Error` only because they were produced by **v2**, which went through axios. Verified with a runtime probe against the installed package, and the test row now carries the real string (it fails against the previous pattern). - **`09ea3b01` — the flag outlived the navigation.** I had it one-way on the assumption it dies with the document. Two flows here keep the document alive, both in the Safari/iOS population this issue skews to: an **iOS PWA** hands the bank redirect to Safari and stays alive polling (`pages/onboarding/index.tsx:121` documents this — it's why the poll exists), and **bfcache** restores the heap when the user presses Back from the bank or Stripe. Either way the user carries on in a live page with reporting silenced for the rest of the session. Now cleared by a persisted `pageshow` or by becoming visible again; neither can clear a departure genuinely in progress, since a same-window redirect never hides the page. ## Why `beforeSend` and not `router.on('networkError')` Inertia does expose a cancellable event, and `preventDefault()` would stop the rejection at the source. Don't — it also skips `onPrefetchError`, which is where the cleanup lives: ```js onPrefetchError(error) { prefetchedRequests.removeFromInFlight(params); reject(error) } ``` A stale `inFlightRequests` entry isn't cosmetic: `add()` early-returns whenever `findInFlight` hits, and `get()` returns it with a promise that never settles — so that URL becomes un-prefetchable for the session and a `Link` consuming it hangs. We have `prefetch` on the sidebar and user menu. It would also suppress genuine errors unconditionally, which is the opposite of the goal. ## Deliberately not done **Feedback on a real network failure.** When an Inertia visit genuinely fails, the user still gets nothing — they click and nothing happens. A toast needs to exclude background prefetch and poll requests; v3 does make that possible (`visit.prefetch` / `visit.poll` on `router.on('start')`), so this is a scope call, not an impossibility. Worth its own PR. **A bad bank `redirect_url`.** Fails at the browser level, so it was never a JS event — we lose nothing here, but we also have no in-app feedback and the only trace is a connection stranded in `Pending`. Separate ticket. ## Verification 369/369 JS tests, lint, format, `dry` (4.81% vs 5.40% threshold), `build` and `build:ssr` all green. Each behavioural test was confirmed to **fail** with its fix reverted — including the stale-count one, which reproduces the wrong number on screen. SSR needed care: `config/inertia.php` has it enabled and `ssr.tsx` globs pages that import this module, so the listeners sit behind `typeof window` and I verified the module imports cleanly in a node environment. ## Why this is a draft Two things I'd rather you weighed: 1. **Suppressing errors fails silently.** If the gate is ever wrong we go blind to a class of real failures, and no error tells us we stopped getting errors — the same shape as the observability gap noted on #723. The risk is bounded by the resets and the narrow message pattern, but the direction of failure is under-reporting. 2. **My confidence here was already miscalibrated once.** The first version passed every local check and one reviewer's independent verification, and was still completely inert for the target traffic. That argues for a human look rather than auto-merge. The parts I'd merge without hesitation are `6a7059d4` (the onboarding wrong-count fix, independent of all this) and `b7dc8dc6`. If you want the observability change split from the onboarding one, say so and I'll separate them. Prod verification after deploy: PHP-LARAVEL-28 should stop accruing new events on `/onboarding`, while `Network Error` events from users who *stayed* on a page should keep arriving. If both go quiet, the gate is too wide. --- lang/es.json | 2 + resources/js/app.tsx | 5 +- .../js/components/app-error-boundary.tsx | 9 +- .../onboarding/ai-suggestion-card.test.tsx | 95 +++++++++++++++ .../onboarding/ai-suggestion-card.tsx | 51 ++++++-- .../transactions/transaction-list.tsx | 3 +- resources/js/hooks/use-connect-flow.ts | 3 +- .../js/hooks/use-decrypt-account-names.ts | 3 +- .../js/hooks/use-decrypt-transactions.ts | 3 +- resources/js/lib/chunk-load-recovery.ts | 3 +- resources/js/lib/leave-page.test.ts | 113 ++++++++++++++++++ resources/js/lib/leave-page.ts | 64 ++++++++++ resources/js/lib/sentry.test.ts | 110 ++++++++++++++++- resources/js/lib/sentry.ts | 32 +++++ .../lib/subscription-payment-issue-toast.ts | 3 +- resources/js/pages/settings/connections.tsx | 5 +- resources/js/pages/welcome.tsx | 3 +- 17 files changed, 479 insertions(+), 28 deletions(-) create mode 100644 resources/js/components/onboarding/ai-suggestion-card.test.tsx create mode 100644 resources/js/lib/leave-page.test.ts create mode 100644 resources/js/lib/leave-page.ts diff --git a/lang/es.json b/lang/es.json index a78bfb53..65ba4436 100644 --- a/lang/es.json +++ b/lang/es.json @@ -2186,6 +2186,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 check which transactions match.": "No hemos podido comprobar qué transacciones coinciden.", "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.", @@ -2215,6 +2216,7 @@ "or": "o", "Create rule": "Crear regla", "Ignore rule": "Ignorar regla", + "? matches": "? coincidencias", ":count matches": ":count coincidencias", "Flex Web Service Token": "Token del Flex Web Service", "Flex Query ID": "Query ID de Flex", diff --git a/resources/js/app.tsx b/resources/js/app.tsx index fb2b4049..9201d2b1 100644 --- a/resources/js/app.tsx +++ b/resources/js/app.tsx @@ -26,11 +26,13 @@ import { SyncProvider } from './contexts/sync-context'; import { initializeTheme } from './hooks/use-appearance'; import { initializeChartColorScheme } from './hooks/use-chart-color-scheme'; import { installChunkLoadRecovery } from './lib/chunk-load-recovery'; +import { leavePage } from './lib/leave-page'; import { initializePostHog } from './lib/posthog'; import { isBrowserExtensionNoise, isChunkLoadErrorEvent, isFacebookInAppBrowserJavaBridgeNoise, + isPageLeaveAbortNoise, isPostMessageDataCloneNoise, isSafariCashbackExtensionNoise, } from './lib/sentry'; @@ -49,6 +51,7 @@ Sentry.init({ beforeSend(event) { if ( isChunkLoadErrorEvent(event) || + isPageLeaveAbortNoise(event) || isBrowserExtensionNoise(event) || isPostMessageDataCloneNoise(event) || isFacebookInAppBrowserJavaBridgeNoise(event) || @@ -110,7 +113,7 @@ function showExpiredConnectionsToast( action: { label: __('Reconnect'), onClick: () => { - window.location.href = firstConnection.reconnect_url; + leavePage(firstConnection.reconnect_url); }, }, }, diff --git a/resources/js/components/app-error-boundary.tsx b/resources/js/components/app-error-boundary.tsx index 4c93848a..c2c225e8 100644 --- a/resources/js/components/app-error-boundary.tsx +++ b/resources/js/components/app-error-boundary.tsx @@ -1,5 +1,6 @@ import { Button } from '@/components/ui/button'; import { reloadOnChunkLoadError } from '@/lib/chunk-load-recovery'; +import { leavePage, reloadPage } from '@/lib/leave-page'; import { getStorage } from '@/lib/safe-storage'; import { dashboard } from '@/routes'; import { __ } from '@/utils/i18n'; @@ -49,7 +50,7 @@ export function AppErrorBoundary({ children }: { children: ReactNode }) { */ function AppErrorFallback() { useEffect(() => { - const reload = () => window.location.reload(); + const reload = reloadPage; window.addEventListener('popstate', reload); @@ -69,14 +70,12 @@ function AppErrorFallback() {

- +