mirror of
https://github.com/koala73/worldmonitor.git
synced 2026-04-25 17:14:57 +02:00
fix(sentry): triage 4 inbox issues — silence no-user checkout + deck.gl/Safari noise (#3303)
* fix(sentry): triage 4 inbox issues — silence no-user checkout + deck.gl/Safari noise
- checkout.ts: skip Sentry emit when action='no-user' (pre-auth redirect UX,
not an error; Clerk analytics already tracks this funnel). session_expired
and other codes keep emitting so mid-flight auth drops stay visible.
- main.ts beforeSend: gate short-var Safari ReferenceError
(/^Can't find variable: \w{1,2}$/) with empty stack + !hasFirstParty —
WORLDMONITOR-NQ ('ss' injection).
- main.ts beforeSend: extend deck.gl internal null-access gate to cover
\w{1,3}\.isHidden when !hasFirstParty — WORLDMONITOR-NR (Safari 26.4 beta
MVTLayer crash preceded by DeckGLMap map-error breadcrumbs). First-party
SmartPollContext.isHidden regressions still surface.
Resolves WORLDMONITOR-NN / NQ / NR in Sentry (inNextRelease). NP
(Convex funrun permit) resolved w/o code change — emitted by Convex→Sentry
SDK, not our bundle; recurrence = real concurrency signal.
* test(sentry): add regression coverage for checkout-skip policy + deck.gl/short-var filters
Addresses P2 review findings on PR #3303:
- Extract skip-Sentry predicate to src/services/checkout-sentry-policy.ts
(mirrors checkout-no-user-policy.ts). Pins the contract: 'no-user' skips;
everything else (no-token, http-error, missing-checkout-url, exception,
entitlement-timeout) emits. A static-source assertion walks every
reportCheckoutError call site in checkout.ts and asserts its policy
decision — forces explicit declaration on any new action string.
- Add 8 tests to tests/sentry-beforesend.test.mjs for the two new filters:
* \w{1,3}\.isHidden deck.gl gate: suppresses empty-stack + vendor-only,
surfaces first-party (SmartPollContext.isHidden) regressions, and
surfaces 4+ char symbol accesses.
* ^Can't find variable: \w{1,2}$ gate: suppresses empty-stack short-var,
surfaces first-party frames on same message, surfaces 3+ char names.
test:data: 6358 pass (was 6340 → +18 new regression tests).
This commit is contained in:
105
tests/checkout-report-error.test.mts
Normal file
105
tests/checkout-report-error.test.mts
Normal file
@@ -0,0 +1,105 @@
|
||||
/**
|
||||
* Regression tests for the Sentry-emit contract in `reportCheckoutError`.
|
||||
*
|
||||
* The `no-user` checkout path is a pre-auth redirect UX (user clicks upgrade
|
||||
* before signing up), not an engineering failure. Clerk conversion analytics
|
||||
* already tracks that funnel, so `reportCheckoutError` deliberately skips
|
||||
* Sentry capture for `action: 'no-user'`. Every other action MUST still emit,
|
||||
* or mid-flight auth drops / missing tokens / server errors would be invisible
|
||||
* — exactly the class of regression a future refactor could introduce by
|
||||
* renaming or collapsing action strings.
|
||||
*
|
||||
* Tests the exported `shouldSkipSentryForAction` predicate (the pure policy)
|
||||
* and asserts the contract against every action string actually used in
|
||||
* src/services/checkout.ts so a silent drift gets caught.
|
||||
*/
|
||||
|
||||
import { describe, it } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { dirname, resolve } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { shouldSkipSentryForAction, SENTRY_SKIP_ACTIONS } from '../src/services/checkout-sentry-policy.ts';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
|
||||
describe('shouldSkipSentryForAction', () => {
|
||||
it('skips Sentry for the no-user pre-auth redirect', () => {
|
||||
assert.equal(shouldSkipSentryForAction('no-user'), true);
|
||||
});
|
||||
|
||||
it('does NOT skip Sentry for session_expired / no-token (mid-flight auth drop)', () => {
|
||||
// no-token fires when Clerk returns null token mid-flight after a valid
|
||||
// session — a real auth-bridge regression we MUST see.
|
||||
assert.equal(shouldSkipSentryForAction('no-token'), false);
|
||||
});
|
||||
|
||||
it('does NOT skip Sentry for http-error (server / network failures)', () => {
|
||||
assert.equal(shouldSkipSentryForAction('http-error'), false);
|
||||
});
|
||||
|
||||
it('does NOT skip Sentry for missing-checkout-url (malformed Convex response)', () => {
|
||||
assert.equal(shouldSkipSentryForAction('missing-checkout-url'), false);
|
||||
});
|
||||
|
||||
it('does NOT skip Sentry for exception (unhandled throw inside startCheckout)', () => {
|
||||
assert.equal(shouldSkipSentryForAction('exception'), false);
|
||||
});
|
||||
|
||||
it('does NOT skip Sentry for entitlement-timeout (post-success activation failure)', () => {
|
||||
assert.equal(shouldSkipSentryForAction('entitlement-timeout'), false);
|
||||
});
|
||||
|
||||
it('does NOT skip Sentry for an unknown / future action string', () => {
|
||||
// Fail-safe: default must be "emit to Sentry" so adding a new error site
|
||||
// never silently blinds the funnel.
|
||||
assert.equal(shouldSkipSentryForAction('something-we-havent-written-yet'), false);
|
||||
});
|
||||
|
||||
it('has exactly one skip action (guards against scope drift)', () => {
|
||||
// If this grows, the PR that expands it must update this assertion AND
|
||||
// the docstring on SENTRY_SKIP_ACTIONS. Keeping the set tiny limits the
|
||||
// blast radius for future refactors that might rename `action` tags.
|
||||
assert.equal(SENTRY_SKIP_ACTIONS.size, 1);
|
||||
assert.ok(SENTRY_SKIP_ACTIONS.has('no-user'));
|
||||
});
|
||||
});
|
||||
|
||||
describe('reportCheckoutError call sites in src/services/checkout.ts', () => {
|
||||
// Static guard: every `reportCheckoutError(... action: 'X' ...)` call site
|
||||
// in the implementation corresponds to a known skip / no-skip decision.
|
||||
// If someone adds a new action string without adding a matching assertion
|
||||
// in shouldSkipSentryForAction tests above, this test fails — forcing the
|
||||
// author to explicitly declare the Sentry-emit policy for the new action.
|
||||
const src = readFileSync(resolve(__dirname, '../src/services/checkout.ts'), 'utf-8');
|
||||
// Non-greedy multi-line match: `reportCheckoutError(` ... up to 300 chars
|
||||
// ... `action: '<tag>'`. Handles call sites where the first arg is itself
|
||||
// a function call (classifySyntheticCheckoutError('unauthorized')) so the
|
||||
// `action` tag can live on a later line.
|
||||
const actionRegex = /reportCheckoutError\([\s\S]{0,300}?action:\s*'([^']+)'/g;
|
||||
const knownActions = new Set<string>();
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = actionRegex.exec(src)) !== null) {
|
||||
knownActions.add(m[1]);
|
||||
}
|
||||
|
||||
it('finds the expected reportCheckoutError call sites', () => {
|
||||
// Pins actual usage at the time of writing. If a new error site is added,
|
||||
// this assertion forces an accompanying policy decision.
|
||||
assert.deepEqual(
|
||||
[...knownActions].sort(),
|
||||
['exception', 'http-error', 'missing-checkout-url', 'no-token', 'no-user'].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it('no-user is the only call site marked for skip', () => {
|
||||
for (const action of knownActions) {
|
||||
const expected = action === 'no-user';
|
||||
assert.equal(
|
||||
shouldSkipSentryForAction(action),
|
||||
expected,
|
||||
`action='${action}' must ${expected ? 'skip' : 'emit'} Sentry`,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -554,4 +554,68 @@ describe('existing beforeSend filters', () => {
|
||||
]);
|
||||
assert.ok(beforeSend(event) !== null, 'first-party onmessage regression must surface');
|
||||
});
|
||||
|
||||
// WORLDMONITOR-NR: deck.gl/maplibre internal null-access on Layer.isHidden
|
||||
// during render (Safari 26.4 beta, empty stacks preceded by DeckGLMap map-error
|
||||
// breadcrumbs). `\w{1,3}\.isHidden` is gated on !hasFirstParty so a genuine
|
||||
// SmartPollContext.isHidden regression in runtime.ts still surfaces.
|
||||
it('suppresses "evaluating \'Ue.isHidden\'" with empty stack (deck.gl/Safari internal)', () => {
|
||||
const event = makeEvent("undefined is not an object (evaluating 'Ue.isHidden')", 'TypeError', []);
|
||||
assert.equal(beforeSend(event), null, 'deck.gl isHidden null-access with empty stack should be suppressed');
|
||||
});
|
||||
|
||||
it('suppresses Cannot-read-isHidden with only vendor frames', () => {
|
||||
const event = makeEvent("Cannot read properties of undefined (reading 'isHidden')", 'TypeError', [
|
||||
{ filename: '/assets/deck-stack-x1y2z3.js', lineno: 1, function: 'Layer.render' },
|
||||
]);
|
||||
assert.equal(beforeSend(event), null, 'deck.gl vendor-only isHidden crash should be suppressed');
|
||||
});
|
||||
|
||||
it('does NOT suppress ".isHidden" crashes with first-party frames (SmartPollContext regression)', () => {
|
||||
// src/services/runtime.ts owns SmartPollContext.isHidden. A real regression
|
||||
// there would carry a first-party frame — must surface.
|
||||
const event = makeEvent("Cannot read properties of undefined (reading 'isHidden')", 'TypeError', [
|
||||
firstPartyFrame('src/services/runtime.ts', 'SmartPoller.tick'),
|
||||
]);
|
||||
assert.ok(beforeSend(event) !== null, 'first-party SmartPollContext.isHidden regression must reach Sentry');
|
||||
});
|
||||
|
||||
it('does NOT suppress ".isHidden" errors on longer-name symbols (bounded char class)', () => {
|
||||
// Filter is scoped to `\w{1,3}` to match minified short names. A 4+ char
|
||||
// symbol like `myLayer.isHidden` should NOT match this filter (it'd hit
|
||||
// the broader !hasFirstParty network/runtime gate instead, which requires
|
||||
// specific shapes — isHidden isn't on that list).
|
||||
const event = makeEvent("undefined is not an object (evaluating 'myLayer.isHidden')", 'TypeError', []);
|
||||
assert.ok(beforeSend(event) !== null, '4+ char symbol accessing .isHidden must still surface');
|
||||
});
|
||||
|
||||
// WORLDMONITOR-NQ: Safari short-var ReferenceError ("Can't find variable: ss")
|
||||
// from userscript/extension injection. Gated on empty stack + !hasFirstParty +
|
||||
// 1–2 char var name so a real "foo is not defined" from our code still surfaces.
|
||||
it("suppresses \"Can't find variable: ss\" with empty stack", () => {
|
||||
const event = makeEvent("Can't find variable: ss", 'Error', []);
|
||||
assert.equal(beforeSend(event), null, 'Short-var Safari ReferenceError with empty stack should be suppressed');
|
||||
});
|
||||
|
||||
it("suppresses \"Can't find variable: x\" (single char)", () => {
|
||||
const event = makeEvent("Can't find variable: x", 'Error', []);
|
||||
assert.equal(beforeSend(event), null);
|
||||
});
|
||||
|
||||
it("does NOT suppress \"Can't find variable: ss\" when first-party frames are present", () => {
|
||||
// A real minified first-party ReferenceError would carry frames. We never
|
||||
// want to silently drop that.
|
||||
const event = makeEvent("Can't find variable: ss", 'Error', [
|
||||
firstPartyFrame('/assets/panels-DzUv7BBV.js', 'loadTab'),
|
||||
]);
|
||||
assert.ok(beforeSend(event) !== null, 'first-party short-var ReferenceError must surface');
|
||||
});
|
||||
|
||||
it("does NOT suppress longer variable names (3+ chars) — shape outside char class", () => {
|
||||
// Only `\w{1,2}` matches. `foo` is 3 chars, falls through — meaningful
|
||||
// first-party misses (e.g. helper name typo) still surface.
|
||||
const event = makeEvent("Can't find variable: foo", 'Error', []);
|
||||
assert.ok(beforeSend(event) !== null, '3+ char variable names must surface');
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user