mirror of
https://github.com/koala73/worldmonitor.git
synced 2026-04-25 17:14:57 +02:00
* fix(checkout): error taxonomy + inline error surfaces (no /pro _blank tabs)
Previously, `startCheckout()` on the main dashboard had 4 failure
branches that all did `window.open('https://worldmonitor.app/pro', '_blank')`:
- no Clerk user
- no token after retry
- non-OK HTTP response
- thrown exception
That's a UX trap — especially on mobile and Tauri desktop, where a
spawned blank tab disorients the user, leaves the original dashboard
untouched, and provides no inline feedback about what went wrong.
Errors fell silently except for a console.error.
This PR introduces Primitive B (client-side checkout error taxonomy)
and wires each failure branch to the right surface:
- `unauthorized` → save intent + `openSignIn()` inline (no tab).
Post-signin Clerk listener auto-resumes the
exact checkout the user just attempted.
- `session_expired` → inline red toast.
- `duplicate_sub.` → continues routing to billing portal (PR-7 will
add the confirmation dialog).
- `invalid_product` → inline toast.
- `service_unav.` → inline toast (catches 5xx + all thrown errors
including aborts, network, timeouts).
- `unknown` → inline toast (defensive bucket).
Raw server-generated strings NEVER reach the user — the taxonomy
maps status+body to a small fixed set of codes each with stable
user copy. Raw details (body.message, err.stack, http status) go
to Sentry via `extra` so engineers can investigate, but the UI
never discloses server internals.
`fallbackToPricingPage` parameter is preserved for backwards
compat. Semantics now:
- true → same-tab `window.location.assign('/pro')` (in-product
upsells that expect users to leave for marketing page)
- false → inline toast (default for dashboard-origin retries and
for `resumePendingCheckout`/failure-banner callers)
Never `window.open(..., '_blank')`. Grep of src/services/checkout.ts
for `window.open` = 0 hits post-PR.
Changes:
- `src/services/checkout-errors.ts` (new) — Primitive B: pure
module mapping HTTP + synthetic + thrown errors to CheckoutError
{code, userMessage, serverMessage?, httpStatus?, retryable}.
- `src/components/checkout-error-toast.ts` (new) — red transient
toast mirroring showCheckoutSuccess styling. Takes ONLY typed
userMessage — can never render raw server text.
- `src/services/checkout.ts` — wires all 4 branches through the
classifier + centralizes Sentry reporting in `reportCheckoutError`
and surface rendering in `renderCheckoutErrorSurface`. No-user
branch saves both PENDING and LAST_CHECKOUT_ATTEMPT keys before
opening sign-in so the Clerk listener can auto-resume.
Tests:
- `tests/checkout-error-classification.test.mts` — 22 cases:
all status→code mappings, body-shape edge cases (message vs
error, both missing), thrown error types (TypeError, AbortError,
non-Error throws), synthetic codes, user-copy invariants
(non-empty, no stack-frame artifacts, no raw server text).
PR-3 of the 14-PR rollout at
docs/plans/2026-04-21-002-feat-harden-auth-checkout-flow-ux-plan.md.
Unblocks PR-7 (duplicate-subscription dialog reuses this taxonomy).
Stacked on PR-2 (`feat/checkout-attempt-lifecycle`) — needs that
branch merged first to avoid rebase. Contains PR-2 commits.
Typecheck + scoped lint clean. test:data 6095/6095 passing.
* fix(checkout): downgrade Sentry noise + surface silent 200-OK failures
Two issues that made the checkout error path useless for both users and
engineers:
1. reportCheckoutError captured everything at level:error, including
unauthorized + session_expired. Those fire on every free-tier sign-up
click and every expired session — not engineering problems, but they
were drowning Sentry in non-actionable noise and triggering alerts.
Route those two codes through level:info instead; real errors stay
at level:error.
2. When /api/create-checkout returned 200 with no checkout_url (server
contract violation), the code silently 'return false'd. User saw
nothing, Sentry saw nothing. Now classifies as service_unavailable
with a distinct action tag so it's filterable in Sentry and the user
sees an actionable toast.
* fix(checkout): honor fallbackToPricingPage on no-user path
Reviewer flagged contract change — the no-user branch had been silently
hardcoded to always call openSignIn() regardless of the
fallbackToPricingPage parameter. Restores the original contract:
default (true) routes signed-out panel upsells to /pro (marketing page
with full pricing context); callers pass `fallbackToPricingPage: false`
to opt into inline sign-in (what the failure-retry banner wants).
* fix(checkout): reopen auth inline on 401/unauthorized from create-checkout
HTTP 401 from /api/create-checkout classified as 'unauthorized' but only
showed a toast — dead end for expired/invalid Clerk sessions. Now saves
pending intent and calls openSignIn() so the post-auth Clerk listener
can auto-resume the exact checkout. session_expired routed the same way
(synthetic no-token path already did; HTTP 401 now matches).
* fix(checkout): don't persist pending intent when redirecting to /pro
Reviewer flagged a cross-session leak: my previous no-user fix saved
PENDING_CHECKOUT_KEY BEFORE redirecting to /pro. The /pro page has its
own URL-param intent mechanism, so the sessionStorage save was unused
there — but it lived on in the dashboard's sessionStorage. Days later
when the same user signs in on the dashboard for unrelated reasons,
Clerk's auto-resume listener reads that stale intent and pops a
checkout the user didn't ask for.
Only persist pending/attempt state on the INLINE sign-in path
(fallbackToPricingPage=false). The /pro redirect path fires a
fire-and-forget navigation; /pro owns its own intent lifecycle.
* test(checkout): regression test for cross-page intent leak
Reviewer flagged residual risk on the no-write-on-/pro-redirect fix:
"no test covering the exact sequence signed-out dashboard click →
/pro redirect → no purchase → later unrelated sign-in".
Extract decideNoUserPathOutcome() into checkout-no-user-policy.ts as a
pure helper (no Clerk / Dodo / Convex deps), then test against:
1. default fallbackToPricingPage=true returns persist:false
2. explicit fallbackToPricingPage=false returns persist:true
3. simulated cross-page sequence: redirect outcome + sessionStorage
never written → resume path finds null → no auto-resume fires
4. inline path correctly persists for resume
5. redirect URL is the canonical absolute /pro
Wires the helper into checkout.ts no-user branch so regression is
caught at build/test time if anyone re-introduces the persist-then-
redirect pattern.
* docs(checkout): clarify 401-only re-auth (not 403)
Reviewer flagged that the comment said "401 / 403 should reopen sign-in"
but the classifier only maps 401 to unauthorized. 403 means valid auth
but forbidden (banned account, plan-tier mismatch) — reopening sign-in
wouldn't change the outcome and would confuse users. Tighten the
comment to match the classifier: 401 only, with explicit explanation
of why 403 is intentionally NOT routed to re-auth.
147 lines
5.3 KiB
TypeScript
147 lines
5.3 KiB
TypeScript
/**
|
|
* Regression test for the cross-page checkout intent leak that the
|
|
* PR-3 reviewer flagged on commit b4e8fb3a1.
|
|
*
|
|
* Scenario covered:
|
|
* 1. Signed-out dashboard click on a paid panel upsell (the common
|
|
* "click locked feature" path) → default fallbackToPricingPage=true
|
|
* → user is redirected to /pro.
|
|
* 2. User does nothing on /pro (closes tab, navigates away, etc.).
|
|
* 3. Hours/days later, the same browser tab signs in on the dashboard
|
|
* for an UNRELATED reason (e.g., responding to a notification).
|
|
* 4. Without this fix, the dashboard's post-sign-in auto-resume
|
|
* listener reads the stale `wm-pending-checkout` key and pops a
|
|
* Dodo overlay for a checkout the user never asked to resume.
|
|
*
|
|
* The fix: the redirect-to-/pro path must NOT write
|
|
* `wm-pending-checkout`. Only the inline-sign-in path (
|
|
* fallbackToPricingPage=false) writes pending state, scoped to the
|
|
* dashboard's own resume flow.
|
|
*
|
|
* Tested via the pure `decideNoUserPathOutcome` policy helper because
|
|
* the surrounding `startCheckout` requires the full Clerk + Dodo SDK
|
|
* dependency tree. The test simulates the storage state that the
|
|
* caller would create based on the policy outcome, then exercises the
|
|
* resume path against a sessionStorage that has NEVER been written —
|
|
* proving auto-resume gets nothing.
|
|
*/
|
|
|
|
import { describe, it, beforeEach, before, after } from 'node:test';
|
|
import assert from 'node:assert/strict';
|
|
|
|
class MemoryStorage {
|
|
private readonly store = new Map<string, string>();
|
|
getItem(key: string): string | null {
|
|
return this.store.has(key) ? (this.store.get(key) as string) : null;
|
|
}
|
|
setItem(key: string, value: string): void {
|
|
this.store.set(key, String(value));
|
|
}
|
|
removeItem(key: string): void {
|
|
this.store.delete(key);
|
|
}
|
|
clear(): void {
|
|
this.store.clear();
|
|
}
|
|
}
|
|
|
|
const PENDING_CHECKOUT_KEY = 'wm-pending-checkout';
|
|
let _sessionStorage: MemoryStorage;
|
|
|
|
before(() => {
|
|
_sessionStorage = new MemoryStorage();
|
|
Object.defineProperty(globalThis, 'sessionStorage', {
|
|
configurable: true,
|
|
value: _sessionStorage,
|
|
});
|
|
Object.defineProperty(globalThis, 'window', {
|
|
configurable: true,
|
|
value: {
|
|
location: { href: 'https://worldmonitor.app/', pathname: '/', search: '', hash: '' },
|
|
history: { replaceState: () => {} },
|
|
},
|
|
});
|
|
});
|
|
|
|
after(() => {
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
delete (globalThis as any).sessionStorage;
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
delete (globalThis as any).window;
|
|
});
|
|
|
|
beforeEach(() => {
|
|
_sessionStorage.clear();
|
|
});
|
|
|
|
const { decideNoUserPathOutcome } = await import('../src/services/checkout-no-user-policy.ts');
|
|
|
|
describe('decideNoUserPathOutcome', () => {
|
|
it('default (fallbackToPricingPage=true) returns redirect-pro with persist=false', () => {
|
|
const outcome = decideNoUserPathOutcome(true);
|
|
assert.equal(outcome.kind, 'redirect-pro');
|
|
assert.equal(outcome.persist, false);
|
|
if (outcome.kind === 'redirect-pro') {
|
|
assert.equal(outcome.redirectUrl, 'https://worldmonitor.app/pro');
|
|
}
|
|
});
|
|
|
|
it('explicit opt-out (fallbackToPricingPage=false) returns inline-signin with persist=true', () => {
|
|
const outcome = decideNoUserPathOutcome(false);
|
|
assert.equal(outcome.kind, 'inline-signin');
|
|
assert.equal(outcome.persist, true);
|
|
});
|
|
});
|
|
|
|
describe('cross-page redirect leak regression', () => {
|
|
it('signed-out → /pro redirect → no purchase → later sign-in: no auto-resume', () => {
|
|
// Step 1: signed-out dashboard click. The policy decides /pro
|
|
// redirect; CALLER must respect persist=false and skip
|
|
// savePendingCheckoutIntent.
|
|
const outcome = decideNoUserPathOutcome(true);
|
|
assert.equal(outcome.kind, 'redirect-pro');
|
|
assert.equal(outcome.persist, false);
|
|
// Simulate caller correctly skipping the persist:
|
|
// (deliberately do NOT call sessionStorage.setItem)
|
|
|
|
// Step 2: user does nothing on /pro. Dashboard sessionStorage is
|
|
// untouched.
|
|
|
|
// Step 3: later sign-in on the dashboard. Resume path checks the
|
|
// pending key.
|
|
const stalePending = _sessionStorage.getItem(PENDING_CHECKOUT_KEY);
|
|
|
|
// Step 4: must be null — no auto-resume can fire.
|
|
assert.equal(
|
|
stalePending,
|
|
null,
|
|
'PENDING_CHECKOUT_KEY must not be written on the redirect-to-/pro path',
|
|
);
|
|
});
|
|
|
|
it('inline sign-in path DOES persist intent (resume needs it)', () => {
|
|
const outcome = decideNoUserPathOutcome(false);
|
|
assert.equal(outcome.persist, true);
|
|
// Simulate caller correctly persisting on the inline path:
|
|
_sessionStorage.setItem(
|
|
PENDING_CHECKOUT_KEY,
|
|
JSON.stringify({ productId: 'pdt_X' }),
|
|
);
|
|
const restored = _sessionStorage.getItem(PENDING_CHECKOUT_KEY);
|
|
assert.notEqual(restored, null);
|
|
});
|
|
|
|
it('redirect-pro outcome carries the canonical /pro URL (not relative)', () => {
|
|
// Regression guard: an absolute URL is required because the
|
|
// dashboard origin and /pro origin are the same in prod
|
|
// (worldmonitor.app) but the helper is also used from sub-origin
|
|
// contexts; relative would resolve unexpectedly.
|
|
const outcome = decideNoUserPathOutcome(true);
|
|
if (outcome.kind === 'redirect-pro') {
|
|
assert.match(outcome.redirectUrl, /^https:\/\/worldmonitor\.app\/pro$/);
|
|
} else {
|
|
assert.fail('expected redirect-pro outcome');
|
|
}
|
|
});
|
|
});
|