fix(observability): stop our own page navigations from reporting fake network errors (#780)
## 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.
This commit is contained in:
parent
ab3902af39
commit
5ec62d8625
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
},
|
||||
},
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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() {
|
|||
</p>
|
||||
|
||||
<div className="flex flex-wrap items-center justify-center gap-2">
|
||||
<Button onClick={() => window.location.reload()}>
|
||||
{__('Try again')}
|
||||
</Button>
|
||||
<Button onClick={reloadPage}>{__('Try again')}</Button>
|
||||
|
||||
<Button
|
||||
variant="outline"
|
||||
onClick={() => {
|
||||
window.location.href = dashboard().url;
|
||||
leavePage(dashboard().url);
|
||||
}}
|
||||
>
|
||||
{__('Go to Dashboard')}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,95 @@
|
|||
import { act, render, screen } from '@testing-library/react';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { AiSuggestionCard, type AiSuggestion } from './ai-suggestion-card';
|
||||
|
||||
const post = vi.fn();
|
||||
|
||||
vi.mock('axios', () => ({
|
||||
default: {
|
||||
post: (...args: unknown[]) => post(...args),
|
||||
isAxiosError: () => true,
|
||||
},
|
||||
}));
|
||||
|
||||
const suggestion: AiSuggestion = {
|
||||
id: 'suggestion-1',
|
||||
confidence: 0.9,
|
||||
group_size: 42,
|
||||
sample_descriptions: [],
|
||||
proposed_category: { id: 'category-1', name: 'Groceries' },
|
||||
new_category_name: null,
|
||||
new_category_direction: null,
|
||||
values: [
|
||||
{
|
||||
id: 'value-1',
|
||||
match_field: 'description',
|
||||
match_operator: 'contains',
|
||||
match_token: 'MERCADONA',
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
describe('AiSuggestionCard when the match preview fails', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
post.mockReset();
|
||||
post.mockRejectedValue(new Error('Network Error'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
/**
|
||||
* Renders the card, then re-renders with an edited token so the debounced
|
||||
* preview fires — an untouched card deliberately makes no request.
|
||||
*/
|
||||
async function renderWithEditedToken() {
|
||||
const draft = {
|
||||
include: true,
|
||||
categoryId: 'category-1',
|
||||
values: [
|
||||
{
|
||||
field: 'description',
|
||||
operator: 'contains',
|
||||
token: 'MERCADONA',
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const view = render(
|
||||
<AiSuggestionCard
|
||||
suggestion={suggestion}
|
||||
draft={draft}
|
||||
categories={[]}
|
||||
onChange={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
view.rerender(
|
||||
<AiSuggestionCard
|
||||
suggestion={suggestion}
|
||||
draft={{
|
||||
...draft,
|
||||
values: [{ ...draft.values[0], token: 'LIDL' }],
|
||||
}}
|
||||
categories={[]}
|
||||
onChange={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(500);
|
||||
});
|
||||
|
||||
return view;
|
||||
}
|
||||
|
||||
it('stops showing a count that belongs to the previous token', async () => {
|
||||
await renderWithEditedToken();
|
||||
|
||||
expect(post).toHaveBeenCalled();
|
||||
expect(screen.queryByText('42 matches')).not.toBeInTheDocument();
|
||||
expect(screen.getByText('? matches')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
@ -118,6 +118,7 @@ export function AiSuggestionCard({
|
|||
const [previewData, setPreviewData] = useState<PreviewResponse | null>(
|
||||
null,
|
||||
);
|
||||
const [previewFailed, setPreviewFailed] = useState(false);
|
||||
|
||||
const conditions = useMemo(
|
||||
() => conditionsFor(draft.values),
|
||||
|
|
@ -148,12 +149,19 @@ export function AiSuggestionCard({
|
|||
|
||||
const handle = setTimeout(async () => {
|
||||
setLoading(true);
|
||||
setPreviewFailed(false);
|
||||
try {
|
||||
const { data } = await axios.post<PreviewResponse>(
|
||||
preview().url,
|
||||
{ conditions },
|
||||
);
|
||||
setPreviewData(data);
|
||||
} catch {
|
||||
// Keeping the old response would show a count for the token the
|
||||
// user just replaced, and they decide whether to create the rule
|
||||
// from that number. Admit we don't know instead.
|
||||
setPreviewData(null);
|
||||
setPreviewFailed(true);
|
||||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
|
|
@ -165,6 +173,21 @@ export function AiSuggestionCard({
|
|||
|
||||
const matchCount = previewData?.match_count ?? suggestion.group_size;
|
||||
|
||||
const previewSummary = (() => {
|
||||
if (previewFailed) {
|
||||
return __('We couldn’t check which transactions match.');
|
||||
}
|
||||
|
||||
if (previewData) {
|
||||
return __(':count of :total uncategorized transactions match', {
|
||||
count: previewData.match_count,
|
||||
total: previewData.total_uncategorized,
|
||||
});
|
||||
}
|
||||
|
||||
return __('Loading…');
|
||||
})();
|
||||
|
||||
const selectedCategory = categories.find((c) => c.id === draft.categoryId);
|
||||
const categoryLabel =
|
||||
selectedCategory?.name ??
|
||||
|
|
@ -211,11 +234,17 @@ export function AiSuggestionCard({
|
|||
}
|
||||
|
||||
setLoading(true);
|
||||
setPreviewFailed(false);
|
||||
try {
|
||||
const { data } = await axios.post<PreviewResponse>(preview().url, {
|
||||
conditions,
|
||||
});
|
||||
setPreviewData(data);
|
||||
} catch {
|
||||
// Without this the dialog drops out of its loading state into an empty
|
||||
// table, which reads as "nothing matches" rather than "we couldn't
|
||||
// find out".
|
||||
setPreviewFailed(true);
|
||||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
|
|
@ -245,7 +274,9 @@ export function AiSuggestionCard({
|
|||
<span>{categoryLabel}</span>
|
||||
</span>
|
||||
<span className="shrink-0 text-xs text-muted-foreground">
|
||||
{__(':count matches', { count: matchCount })}
|
||||
{previewFailed
|
||||
? __('? matches')
|
||||
: __(':count matches', { count: matchCount })}
|
||||
</span>
|
||||
<ChevronDown
|
||||
className={`size-4 shrink-0 text-muted-foreground transition-transform ${expanded ? 'rotate-180' : ''}`}
|
||||
|
|
@ -360,17 +391,7 @@ export function AiSuggestionCard({
|
|||
<DialogContent className="max-h-[85vh] gap-0 overflow-hidden p-0 sm:max-w-2xl">
|
||||
<DialogHeader className="space-y-1 p-6 pb-4">
|
||||
<DialogTitle>{__('Matching transactions')}</DialogTitle>
|
||||
<DialogDescription>
|
||||
{previewData
|
||||
? __(
|
||||
':count of :total uncategorized transactions match',
|
||||
{
|
||||
count: previewData.match_count,
|
||||
total: previewData.total_uncategorized,
|
||||
},
|
||||
)
|
||||
: __('Loading…')}
|
||||
</DialogDescription>
|
||||
<DialogDescription>{previewSummary}</DialogDescription>
|
||||
</DialogHeader>
|
||||
|
||||
<div className="max-h-[60vh] overflow-y-auto border-t">
|
||||
|
|
@ -379,6 +400,12 @@ export function AiSuggestionCard({
|
|||
<Loader2 className="size-4 animate-spin" />
|
||||
{__('Loading…')}
|
||||
</div>
|
||||
) : previewFailed ? (
|
||||
<p className="p-8 text-center text-sm text-muted-foreground">
|
||||
{__(
|
||||
'We couldn’t check which transactions match.',
|
||||
)}
|
||||
</p>
|
||||
) : (
|
||||
<Table>
|
||||
<TableHeader className="sticky top-0 bg-background">
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { useLocale } from '@/hooks/use-locale';
|
||||
import { reloadPage } from '@/lib/leave-page';
|
||||
import { __ } from '@/utils/i18n';
|
||||
import { Link } from '@inertiajs/react';
|
||||
import {
|
||||
|
|
@ -1027,7 +1028,7 @@ export function TransactionList({
|
|||
onReEvaluateComplete={() => {
|
||||
setRowSelection({});
|
||||
setTimeout(() => {
|
||||
window.location.reload();
|
||||
reloadPage();
|
||||
}, 500);
|
||||
}}
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
isProviderComplete,
|
||||
} from '@/lib/connect-providers';
|
||||
import { getCsrfToken } from '@/lib/csrf';
|
||||
import { leavePage } from '@/lib/leave-page';
|
||||
import type {
|
||||
BankingConnection,
|
||||
EnableBankingInstitution,
|
||||
|
|
@ -216,7 +217,7 @@ export function useConnectFlow(connections: BankingConnection[]) {
|
|||
}
|
||||
|
||||
const data = await response.json();
|
||||
window.location.href = data.redirect_url;
|
||||
leavePage(data.redirect_url);
|
||||
} catch (e) {
|
||||
setError(
|
||||
e instanceof Error
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import {
|
|||
import { useEncryptionKey } from '@/contexts/encryption-key-context';
|
||||
import { decrypt, importKey } from '@/lib/crypto';
|
||||
import { getStoredKey } from '@/lib/key-storage';
|
||||
import { reloadPage } from '@/lib/leave-page';
|
||||
import { SharedData } from '@/types';
|
||||
import { usePage } from '@inertiajs/react';
|
||||
import axios from 'axios';
|
||||
|
|
@ -67,7 +68,7 @@ export function useDecryptAccountNames() {
|
|||
}
|
||||
}
|
||||
|
||||
window.location.reload();
|
||||
reloadPage();
|
||||
} catch {
|
||||
// Silent failure — migration will retry next session
|
||||
hasRun.current = false;
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { bulkUpdate } from '@/actions/App/Http/Controllers/Api/TransactionContro
|
|||
import { useEncryptionKey } from '@/contexts/encryption-key-context';
|
||||
import { decrypt, importKey } from '@/lib/crypto';
|
||||
import { getStoredKey } from '@/lib/key-storage';
|
||||
import { reloadPage } from '@/lib/leave-page';
|
||||
import { SharedData } from '@/types';
|
||||
import { usePage } from '@inertiajs/react';
|
||||
import axios from 'axios';
|
||||
|
|
@ -135,7 +136,7 @@ export function useDecryptTransactions() {
|
|||
}
|
||||
}
|
||||
|
||||
window.location.reload();
|
||||
reloadPage();
|
||||
} catch {
|
||||
// Silent failure — migration will retry next session
|
||||
hasRun.current = false;
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import { reloadPage } from './leave-page';
|
||||
import { getStorage } from './safe-storage';
|
||||
|
||||
const CHUNK_LOAD_RELOAD_STORAGE_KEY =
|
||||
|
|
@ -51,7 +52,7 @@ export function reloadOnChunkLoadError(
|
|||
}
|
||||
|
||||
markReloadedForAssetSignature(assetSignature, options.storage);
|
||||
(options.reload ?? (() => window.location.reload()))();
|
||||
(options.reload ?? reloadPage)();
|
||||
|
||||
return true;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,113 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
/**
|
||||
* The flag lives at module scope, so every case needs its own instance. Importing
|
||||
* fresh also re-registers the listeners against the current jsdom document.
|
||||
*/
|
||||
async function freshModule() {
|
||||
vi.resetModules();
|
||||
|
||||
return import('./leave-page');
|
||||
}
|
||||
|
||||
describe('leave-page', () => {
|
||||
const realLocation = window.location;
|
||||
|
||||
beforeEach(() => {
|
||||
Object.defineProperty(window, 'location', {
|
||||
configurable: true,
|
||||
writable: true,
|
||||
value: {
|
||||
href: 'https://whisper.money/onboarding',
|
||||
reload: vi.fn(),
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(window, 'location', {
|
||||
configurable: true,
|
||||
writable: true,
|
||||
value: realLocation,
|
||||
});
|
||||
});
|
||||
|
||||
it('is not leaving until something says so', async () => {
|
||||
const { isLeavingPage } = await freshModule();
|
||||
|
||||
expect(isLeavingPage()).toBe(false);
|
||||
});
|
||||
|
||||
it('navigates and records the departure', async () => {
|
||||
const { leavePage, isLeavingPage } = await freshModule();
|
||||
|
||||
leavePage('https://bank.example/authorize');
|
||||
|
||||
expect(window.location.href).toBe('https://bank.example/authorize');
|
||||
expect(isLeavingPage()).toBe(true);
|
||||
});
|
||||
|
||||
it('reloads and records the departure', async () => {
|
||||
const { reloadPage, isLeavingPage } = await freshModule();
|
||||
|
||||
reloadPage();
|
||||
|
||||
expect(window.location.reload).toHaveBeenCalledOnce();
|
||||
expect(isLeavingPage()).toBe(true);
|
||||
});
|
||||
|
||||
it('records departures it did not start, like an external link', async () => {
|
||||
const { isLeavingPage } = await freshModule();
|
||||
|
||||
window.dispatchEvent(new Event('pagehide'));
|
||||
|
||||
expect(isLeavingPage()).toBe(true);
|
||||
});
|
||||
|
||||
it('stops leaving when the back/forward cache restores the page', async () => {
|
||||
const { leavePage, isLeavingPage } = await freshModule();
|
||||
|
||||
leavePage('https://bank.example/authorize');
|
||||
window.dispatchEvent(
|
||||
new PageTransitionEvent('pageshow', {
|
||||
persisted: true,
|
||||
}),
|
||||
);
|
||||
|
||||
expect(isLeavingPage()).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps leaving on the initial load, which is not a restore', async () => {
|
||||
const { leavePage, isLeavingPage } = await freshModule();
|
||||
|
||||
leavePage('https://bank.example/authorize');
|
||||
window.dispatchEvent(
|
||||
new PageTransitionEvent('pageshow', {
|
||||
persisted: false,
|
||||
}),
|
||||
);
|
||||
|
||||
expect(isLeavingPage()).toBe(true);
|
||||
});
|
||||
|
||||
it('stops leaving when the page becomes visible again, as an iOS PWA does', async () => {
|
||||
const { leavePage, isLeavingPage } = await freshModule();
|
||||
|
||||
leavePage('https://bank.example/authorize');
|
||||
|
||||
Object.defineProperty(document, 'visibilityState', {
|
||||
configurable: true,
|
||||
value: 'hidden',
|
||||
});
|
||||
document.dispatchEvent(new Event('visibilitychange'));
|
||||
expect(isLeavingPage()).toBe(true);
|
||||
|
||||
Object.defineProperty(document, 'visibilityState', {
|
||||
configurable: true,
|
||||
value: 'visible',
|
||||
});
|
||||
document.dispatchEvent(new Event('visibilitychange'));
|
||||
|
||||
expect(isLeavingPage()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,64 @@
|
|||
/**
|
||||
* Navigates out of the SPA with a full page load.
|
||||
*
|
||||
* Assigning `window.location` aborts every request still in flight, and browsers
|
||||
* report that abort to XHR as a transport failure rather than a cancellation
|
||||
* (WebKit especially). So an onboarding poll or a hover prefetch that happened to
|
||||
* be running when the user left surfaces as an unhandled `AxiosError: Network
|
||||
* Error` — a bug report for a request that never actually failed.
|
||||
*
|
||||
* Recording the departure lets Sentry tell "the request died with the page" apart
|
||||
* from "the user's connection dropped". Always prefer Inertia's `router`/`Link`
|
||||
* for internal navigation; this is only for leaving the app entirely (a bank's
|
||||
* authorization page, the Stripe billing portal, a locale reload).
|
||||
*/
|
||||
let leaving = false;
|
||||
|
||||
export function leavePage(url: string): void {
|
||||
leaving = true;
|
||||
window.location.href = url;
|
||||
}
|
||||
|
||||
export function reloadPage(): void {
|
||||
leaving = true;
|
||||
window.location.reload();
|
||||
}
|
||||
|
||||
export function isLeavingPage(): boolean {
|
||||
return leaving;
|
||||
}
|
||||
|
||||
// SSR renders this module — pages/welcome.tsx and pages/settings/connections.tsx
|
||||
// import it, and ssr.tsx globs every page — so nothing here may touch `window` at
|
||||
// import time.
|
||||
if (typeof window !== 'undefined') {
|
||||
// Not every departure comes through the functions above: an external <a>, the
|
||||
// back button and closing the tab unload the document too. Those don't get the
|
||||
// eager flag, which is the whole reason the functions exist — by the time
|
||||
// `pagehide` fires the abort may already have been reported.
|
||||
window.addEventListener('pagehide', () => {
|
||||
leaving = true;
|
||||
});
|
||||
|
||||
// The document does not always die once we leave. Back/forward cache restores
|
||||
// it with the flag still set, and an iOS PWA hands the bank redirect to Safari
|
||||
// and stays alive polling (see pages/onboarding/index.tsx) — either way the
|
||||
// user carries on in a live page whose network errors we'd have silenced for
|
||||
// the rest of the session.
|
||||
//
|
||||
// Becoming visible again is the proof that we stayed. It cannot clear a
|
||||
// departure that is genuinely in progress, because a same-window redirect
|
||||
// never hides the page; and if it ever fires early we over-report, which is
|
||||
// the safe direction.
|
||||
window.addEventListener('pageshow', (event) => {
|
||||
if (event.persisted) {
|
||||
leaving = false;
|
||||
}
|
||||
});
|
||||
|
||||
document.addEventListener('visibilitychange', () => {
|
||||
if (document.visibilityState === 'visible') {
|
||||
leaving = false;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
@ -1,5 +1,5 @@
|
|||
import type { Event } from '@sentry/react';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import {
|
||||
isBrowserExtensionNoise,
|
||||
isChunkLoadErrorEvent,
|
||||
|
|
@ -8,6 +8,114 @@ import {
|
|||
isSafariCashbackExtensionNoise,
|
||||
} from './sentry';
|
||||
|
||||
function exceptionEvent(type: string, value: string): Event {
|
||||
return { exception: { values: [{ type, value }] } };
|
||||
}
|
||||
|
||||
describe('isPageLeaveAbortNoise', () => {
|
||||
// The flag lives at module scope and is deliberately one-way, so each case
|
||||
// needs its own instance of the pair rather than a reset helper.
|
||||
async function freshModules() {
|
||||
vi.resetModules();
|
||||
|
||||
return {
|
||||
...(await import('./leave-page')),
|
||||
...(await import('./sentry')),
|
||||
};
|
||||
}
|
||||
|
||||
const realLocation = window.location;
|
||||
|
||||
beforeEach(() => {
|
||||
Object.defineProperty(window, 'location', {
|
||||
configurable: true,
|
||||
writable: true,
|
||||
value: { href: 'https://whisper.money/onboarding' },
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(window, 'location', {
|
||||
configurable: true,
|
||||
writable: true,
|
||||
value: realLocation,
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps network failures while the user is still on the page', async () => {
|
||||
const { isPageLeaveAbortNoise } = await freshModules();
|
||||
|
||||
expect(
|
||||
isPageLeaveAbortNoise(
|
||||
exceptionEvent('AxiosError', 'Network Error'),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['AxiosError', 'Network Error'],
|
||||
['AxiosError', 'Request aborted'],
|
||||
// Inertia's own shape: HttpError appends the request URL to the message,
|
||||
// so a bare "Network error" is not what actually reaches Sentry.
|
||||
[
|
||||
'HttpNetworkError',
|
||||
'Network error (https://whisper.money/onboarding?step=create-account)',
|
||||
],
|
||||
['TypeError', 'Failed to fetch'],
|
||||
['TypeError', 'Load failed'],
|
||||
])(
|
||||
'drops a %s "%s" caused by the navigation we started',
|
||||
async (type, value) => {
|
||||
const { leavePage, isPageLeaveAbortNoise } = await freshModules();
|
||||
|
||||
leavePage('https://bank.example/authorize');
|
||||
|
||||
expect(window.location.href).toBe('https://bank.example/authorize');
|
||||
expect(isPageLeaveAbortNoise(exceptionEvent(type, value))).toBe(
|
||||
true,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('keeps a genuine crash that happens while leaving', async () => {
|
||||
const { leavePage, isPageLeaveAbortNoise } = await freshModules();
|
||||
|
||||
leavePage('https://bank.example/authorize');
|
||||
|
||||
expect(
|
||||
isPageLeaveAbortNoise(
|
||||
exceptionEvent(
|
||||
'TypeError',
|
||||
"undefined is not an object (evaluating 'e.features.cashflow')",
|
||||
),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps a chunk load failure, which starts the same way', async () => {
|
||||
const { leavePage, isPageLeaveAbortNoise } = await freshModules();
|
||||
|
||||
leavePage('https://bank.example/authorize');
|
||||
|
||||
expect(
|
||||
isPageLeaveAbortNoise(
|
||||
exceptionEvent(
|
||||
'TypeError',
|
||||
'Failed to fetch dynamically imported module: https://whisper.money/build/assets/accounts-BO3xxENF.js',
|
||||
),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps an event carrying no exception at all', async () => {
|
||||
const { leavePage, isPageLeaveAbortNoise } = await freshModules();
|
||||
|
||||
leavePage('https://bank.example/authorize');
|
||||
|
||||
expect(isPageLeaveAbortNoise({ message: 'Network Error' })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isChunkLoadErrorEvent', () => {
|
||||
it('drops recoverable Vite dynamic import failures', () => {
|
||||
const event: Event = {
|
||||
|
|
|
|||
|
|
@ -1,8 +1,20 @@
|
|||
import type { Event } from '@sentry/react';
|
||||
import { isChunkLoadError } from './chunk-load-recovery';
|
||||
import { isLeavingPage } from './leave-page';
|
||||
|
||||
const CLONE_ERROR_MESSAGE_PATTERN =
|
||||
/object (can not|could not|couldn't|can't) be cloned/i;
|
||||
// Transport-level failures only, so a genuine crash during a page unload is
|
||||
// still reported. Covers axios/XHR ("Network Error", "Request aborted") and
|
||||
// fetch in both engines ("Failed to fetch" in Blink, "Load failed" in WebKit).
|
||||
//
|
||||
// The optional parenthesised tail is Inertia's: HttpError's constructor appends
|
||||
// the request URL, so its own wrapper arrives as
|
||||
// `Network error (https://whisper.money/onboarding)`. Matching that tail
|
||||
// explicitly rather than loosening to a prefix keeps "Failed to fetch
|
||||
// dynamically imported module: …" out — that is a chunk load, not an abort.
|
||||
const ABORTED_REQUEST_MESSAGE_PATTERN =
|
||||
/^(network error|request aborted|failed to fetch|load failed)( \(.+\))?$/i;
|
||||
const FACEBOOK_IAB_JAVA_OBJECT_GONE_PATTERN =
|
||||
/Error invoking .+: Java object is gone/i;
|
||||
const SAFARI_CASHBACK_EXTENSION_PATTERN = /response\.cashbackReminder/i;
|
||||
|
|
@ -80,6 +92,26 @@ export function isBrowserExtensionNoise(event: Event): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Requests killed by a full page navigation we started ourselves.
|
||||
*
|
||||
* {@link leavePage} aborts whatever was in flight, and the browser hands that
|
||||
* abort to the caller as a transport failure. The request didn't fail — the page
|
||||
* it belonged to went away — so reporting it buries the connection failures that
|
||||
* really did happen to a user who stayed put.
|
||||
*/
|
||||
export function isPageLeaveAbortNoise(event: Event): boolean {
|
||||
if (!isLeavingPage()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return (
|
||||
event.exception?.values?.some((exception) =>
|
||||
ABORTED_REQUEST_MESSAGE_PATTERN.test(exception.value ?? ''),
|
||||
) ?? false
|
||||
);
|
||||
}
|
||||
|
||||
export function isSafariCashbackExtensionNoise(event: Event): boolean {
|
||||
return (
|
||||
event.exception?.values?.some((exception) => {
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import { leavePage } from '@/lib/leave-page';
|
||||
import type { SubscriptionPaymentIssueNotification } from '@/types';
|
||||
import { __ } from '@/utils/i18n';
|
||||
import { toast } from 'sonner';
|
||||
|
|
@ -32,7 +33,7 @@ export function showSubscriptionPaymentIssueToast(
|
|||
action: {
|
||||
label: __('Update payment'),
|
||||
onClick: () => {
|
||||
window.location.href = issue.action_url;
|
||||
leavePage(issue.action_url);
|
||||
},
|
||||
},
|
||||
});
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import AppLayout from '@/layouts/app-layout';
|
|||
import SettingsLayout from '@/layouts/settings/layout';
|
||||
import { CONNECT_PROVIDERS } from '@/lib/connect-providers';
|
||||
import { getCsrfToken } from '@/lib/csrf';
|
||||
import { leavePage } from '@/lib/leave-page';
|
||||
import type { SharedData } from '@/types';
|
||||
import type { BankingConnection } from '@/types/banking';
|
||||
import { __ } from '@/utils/i18n';
|
||||
|
|
@ -102,7 +103,7 @@ export default function ConnectionsPage({ connections }: Props) {
|
|||
const data = await response.json().catch(() => ({}));
|
||||
|
||||
if (typeof data.redirect === 'string') {
|
||||
window.location.href = data.redirect;
|
||||
leavePage(data.redirect);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -112,7 +113,7 @@ export default function ConnectionsPage({ connections }: Props) {
|
|||
}
|
||||
|
||||
const data = await response.json();
|
||||
window.location.href = data.redirect_url;
|
||||
leavePage(data.redirect_url);
|
||||
} catch (e) {
|
||||
toast.error(
|
||||
e instanceof Error
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import {
|
|||
useScrollProgress,
|
||||
useScrollTranslate,
|
||||
} from '@/hooks/use-scroll-progress';
|
||||
import { leavePage } from '@/lib/leave-page';
|
||||
import { readStoredValue, writeStoredValue } from '@/lib/safe-storage';
|
||||
import { cn } from '@/lib/utils';
|
||||
import { dashboard, roadmap } from '@/routes';
|
||||
|
|
@ -1879,7 +1880,7 @@ export default function Welcome({
|
|||
supportedLocales.includes(storedLocale)
|
||||
) {
|
||||
// Redirect to stored preference
|
||||
window.location.href = `/?lang=${storedLocale}`;
|
||||
leavePage(`/?lang=${storedLocale}`);
|
||||
return;
|
||||
} else if (!storedLocale && locale) {
|
||||
// First visit - store the detected locale from session/header
|
||||
|
|
|
|||
Loading…
Reference in New Issue