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.
171 lines
6.5 KiB
TypeScript
171 lines
6.5 KiB
TypeScript
/**
|
|
* Locks the user-facing copy + retryability + code mapping for the
|
|
* client-side checkout error taxonomy. A change to any user-facing
|
|
* message should fail this test — the copy lives in one place for a
|
|
* reason and must not drift.
|
|
*/
|
|
|
|
import { describe, it } from 'node:test';
|
|
import assert from 'node:assert/strict';
|
|
|
|
import {
|
|
classifyHttpCheckoutError,
|
|
classifySyntheticCheckoutError,
|
|
classifyThrownCheckoutError,
|
|
} from '../src/services/checkout-errors.ts';
|
|
|
|
describe('classifyHttpCheckoutError', () => {
|
|
it('maps 401 to unauthorized', () => {
|
|
const err = classifyHttpCheckoutError(401);
|
|
assert.equal(err.code, 'unauthorized');
|
|
assert.equal(err.retryable, true);
|
|
assert.equal(err.httpStatus, 401);
|
|
});
|
|
|
|
it('maps 409 with ACTIVE_SUBSCRIPTION_EXISTS to duplicate_subscription', () => {
|
|
const err = classifyHttpCheckoutError(409, {
|
|
error: 'ACTIVE_SUBSCRIPTION_EXISTS',
|
|
message: 'Active Pro Monthly sub exists for user X',
|
|
});
|
|
assert.equal(err.code, 'duplicate_subscription');
|
|
assert.equal(err.retryable, false);
|
|
});
|
|
|
|
it('maps 409 without known error code to invalid_product (4xx)', () => {
|
|
const err = classifyHttpCheckoutError(409, { error: 'SOMETHING_ELSE' });
|
|
assert.equal(err.code, 'invalid_product');
|
|
});
|
|
|
|
it('maps 400 to invalid_product', () => {
|
|
const err = classifyHttpCheckoutError(400);
|
|
assert.equal(err.code, 'invalid_product');
|
|
assert.equal(err.retryable, false);
|
|
});
|
|
|
|
it('maps 404 to invalid_product', () => {
|
|
const err = classifyHttpCheckoutError(404);
|
|
assert.equal(err.code, 'invalid_product');
|
|
});
|
|
|
|
it('maps 500 to service_unavailable', () => {
|
|
const err = classifyHttpCheckoutError(500);
|
|
assert.equal(err.code, 'service_unavailable');
|
|
assert.equal(err.retryable, true);
|
|
});
|
|
|
|
it('maps 503 to service_unavailable', () => {
|
|
const err = classifyHttpCheckoutError(503);
|
|
assert.equal(err.code, 'service_unavailable');
|
|
});
|
|
|
|
it('maps 502 to service_unavailable', () => {
|
|
const err = classifyHttpCheckoutError(502);
|
|
assert.equal(err.code, 'service_unavailable');
|
|
});
|
|
|
|
it('maps unexpected status (e.g. 302) to unknown', () => {
|
|
const err = classifyHttpCheckoutError(302);
|
|
assert.equal(err.code, 'unknown');
|
|
});
|
|
|
|
it('preserves serverMessage from body.message when present', () => {
|
|
const err = classifyHttpCheckoutError(500, {
|
|
message: 'Internal relay failure: convex action timeout at node-3',
|
|
});
|
|
assert.equal(err.serverMessage, 'Internal relay failure: convex action timeout at node-3');
|
|
// User never sees the server string.
|
|
assert.notEqual(err.userMessage, err.serverMessage);
|
|
assert.equal(err.userMessage, 'Checkout is temporarily unavailable. Please try again in a moment.');
|
|
});
|
|
|
|
it('falls back to body.error when body.message is absent', () => {
|
|
const err = classifyHttpCheckoutError(400, { error: 'INVALID_PRODUCT_ID' });
|
|
assert.equal(err.serverMessage, 'INVALID_PRODUCT_ID');
|
|
});
|
|
|
|
it('leaves serverMessage undefined when body is empty', () => {
|
|
const err = classifyHttpCheckoutError(500);
|
|
assert.equal(err.serverMessage, undefined);
|
|
});
|
|
|
|
it('never exposes raw server text in userMessage', () => {
|
|
const err = classifyHttpCheckoutError(500, {
|
|
message: 'leaked-internal-id-42: db.prod-us-east-1 connection refused',
|
|
});
|
|
assert.ok(!err.userMessage.includes('leaked'));
|
|
assert.ok(!err.userMessage.includes('db.prod'));
|
|
});
|
|
});
|
|
|
|
describe('classifyThrownCheckoutError', () => {
|
|
it('classifies network errors as service_unavailable', () => {
|
|
const err = classifyThrownCheckoutError(new TypeError('Failed to fetch'));
|
|
assert.equal(err.code, 'service_unavailable');
|
|
assert.equal(err.retryable, true);
|
|
assert.equal(err.serverMessage, 'Failed to fetch');
|
|
});
|
|
|
|
it('classifies AbortError (timeout) as service_unavailable', () => {
|
|
const abort = new Error('The operation was aborted');
|
|
abort.name = 'AbortError';
|
|
const err = classifyThrownCheckoutError(abort);
|
|
assert.equal(err.code, 'service_unavailable');
|
|
});
|
|
|
|
it('handles non-Error throws by coercing to string', () => {
|
|
const err = classifyThrownCheckoutError('string thrown directly');
|
|
assert.equal(err.code, 'service_unavailable');
|
|
assert.equal(err.serverMessage, 'string thrown directly');
|
|
});
|
|
|
|
it('handles null/undefined caught values', () => {
|
|
const err = classifyThrownCheckoutError(undefined);
|
|
assert.equal(err.code, 'service_unavailable');
|
|
});
|
|
});
|
|
|
|
describe('classifySyntheticCheckoutError', () => {
|
|
it('maps unauthorized to retryable unauthorized code', () => {
|
|
const err = classifySyntheticCheckoutError('unauthorized');
|
|
assert.equal(err.code, 'unauthorized');
|
|
assert.equal(err.retryable, true);
|
|
assert.equal(err.userMessage, 'Please sign in to continue your purchase.');
|
|
});
|
|
|
|
it('maps session_expired to retryable session_expired code', () => {
|
|
const err = classifySyntheticCheckoutError('session_expired');
|
|
assert.equal(err.code, 'session_expired');
|
|
assert.equal(err.retryable, true);
|
|
});
|
|
|
|
it('does not include serverMessage for synthetic errors (no server involved)', () => {
|
|
const err = classifySyntheticCheckoutError('unauthorized');
|
|
assert.equal(err.serverMessage, undefined);
|
|
});
|
|
});
|
|
|
|
describe('user copy invariants', () => {
|
|
it('user copy is non-empty for every code', () => {
|
|
const codes = [401, 409, 400, 500, 503, 302] as const;
|
|
for (const status of codes) {
|
|
const err = classifyHttpCheckoutError(status);
|
|
assert.ok(err.userMessage.length > 0, `user message should not be empty for ${status}`);
|
|
}
|
|
});
|
|
|
|
it('user copy never contains raw server-generated artifacts', () => {
|
|
// Pass a server message laden with artifacts we'd never want the
|
|
// user to see; assert the userMessage stays clean regardless.
|
|
const hostile = 'Error: stack trace\n at foo.js:10\n at bar.js:42\n at DB.query(prod-us-east-1)';
|
|
const codes = [401, 409, 400, 500, 503] as const;
|
|
for (const status of codes) {
|
|
const err = classifyHttpCheckoutError(status, { message: hostile });
|
|
// Node/Chrome stack-frame pattern (4 spaces + "at " + identifier).
|
|
assert.ok(!/\s{2,}at\s/.test(err.userMessage), `user copy must not include stack frames`);
|
|
assert.ok(!err.userMessage.includes('Error:'), `user copy must not include raw Error: prefix`);
|
|
assert.ok(!err.userMessage.includes('prod-us-east-1'), `user copy must not include infra identifiers`);
|
|
assert.ok(!err.userMessage.includes(hostile), `user copy must not include the raw server message`);
|
|
}
|
|
});
|
|
});
|