diff --git a/.github/workflows/deno-tests.yml b/.github/workflows/deno-tests.yml new file mode 100644 index 0000000..4c288cb --- /dev/null +++ b/.github/workflows/deno-tests.yml @@ -0,0 +1,35 @@ +name: Deno Tests + +on: + pull_request: + paths: + - "supabase/functions/**" + - ".github/workflows/deno-tests.yml" + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + + - uses: denoland/setup-deno@v2 + with: + deno-version: v2.x + + # deno.lock is a real, committed lockfile (it also pins + # scripts/backfill-mailerlite.ts's esm.sh imports) — deliberately not + # using --no-lock here so this step verifies those hashes. Deno also + # rewrites the lockfile's npm-workspace section to reflect the root + # package.json on every resolve; that's expected and harmless since + # CI never commits the checkout back. + # + # No --allow-net: rc-client.test.ts fully replaces globalThis.fetch + # rather than making real network calls, so no permissions are needed + # today. If a future test hits a real network/filesystem/env, add the + # specific flag it needs rather than granting broadly up front. + - run: deno test supabase/functions diff --git a/.github/workflows/lint-typecheck.yml b/.github/workflows/lint-typecheck.yml new file mode 100644 index 0000000..2748290 --- /dev/null +++ b/.github/workflows/lint-typecheck.yml @@ -0,0 +1,26 @@ +name: Lint & Typecheck + +on: + pull_request: + +permissions: + contents: read + +jobs: + lint-typecheck: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + + - uses: actions/setup-node@v4 + with: + node-version: "22" + cache: "npm" + + - run: npm ci + + - run: npm run lint + + - run: npm run typecheck diff --git a/docs/payments.md b/docs/payments.md index 0969be0..44795ad 100644 --- a/docs/payments.md +++ b/docs/payments.md @@ -72,10 +72,9 @@ The webhook is the realtime path. The drift health check is the out-of-band back **What it does:** -1. Lists every RevenueCat customer for the project (paginated via `starting_after`, 100 per page). -2. Reads `id, entitlement` from every `profiles` row. -3. Classifies each user into one of three drift categories. Healthy users are not reported. -4. If drift > 0, emits **one** Sentry event with a stable fingerprint so consecutive runs collapse into a single issue. +1. Reads `id, entitlement` from every `profiles` row, then looks up each user's RevenueCat state individually (`GET /v2/projects/{id}/customers/{customer_id}`) — see the "Why no orphan category?" note below for why this isn't a bulk RC customer listing. +2. Classifies each user into one of three drift categories. Healthy users are not reported. +3. If drift > 0, emits **one** Sentry event with a stable fingerprint so consecutive runs collapse into a single issue. **Drift categories:** @@ -93,11 +92,13 @@ The Sentry event's `extra` payload includes the first 50 affected ids per catego Every run prints one heartbeat to the Edge Function logs regardless of outcome: ```json -{"event":"drift_check_complete","drift_count":0,"count_missing":0,"count_stale":0,"supabase_profile_count":47,"run_at":"..."} +{"event":"drift_check_complete","drift_count":0,"count_missing":0,"count_stale":0,"failed_count":0,"supabase_profile_count":47,"run_at":"..."} ``` `mcp__supabase__get_logs --service edge-function` is the fastest way to find it. A missing heartbeat means the cron job didn't run, which is itself a signal worth investigating. +**Partial coverage (`rc_drift_check_partial_failure`):** each per-customer RC lookup gets one retry on a transient failure (5xx, network error, timeout) before being given up on; a customer that still fails isn't retried further within that run — it's excluded from drift classification and counted in `failed_count`. If any customers failed, a separate Sentry warning fires with fingerprint `rc-drift-check-partial-failure` (distinct from `rc-entitlement-drift`, so the two never suppress each other). No action needed beyond noting it — the next scheduled run re-checks every profile from scratch, so a skipped customer is covered again in ≤6 hours. Only worth investigating if `failed_count` is persistently non-zero across consecutive runs (points to a sustained RC-side or network problem, not a blip). + **Operator runbook — drift event fires:** 1. Open the Sentry issue. Look at `tags.count_missing` first; that's the urgent class. diff --git a/supabase/functions/rc-entitlement-drift-check/index.ts b/supabase/functions/rc-entitlement-drift-check/index.ts index b8bc9f2..85efef5 100644 --- a/supabase/functions/rc-entitlement-drift-check/index.ts +++ b/supabase/functions/rc-entitlement-drift-check/index.ts @@ -1,41 +1,22 @@ import { serve } from "https://deno.land/std@0.168.0/http/server.ts"; import { createClient } from "https://esm.sh/@supabase/supabase-js@2"; import * as Sentry from "npm:@sentry/node"; +import { + classifyProfilesDrift, + resolvePremiumEntitlementId, +} from "./rc-client.ts"; Sentry.init({ dsn: Deno.env.get("SENTRY_DSN"), tracesSampleRate: 0, }); -const RC_API_BASE = "https://api.revenuecat.com/v2"; const JOB_NAME = "rc-entitlement-drift-check"; const MAX_EXTRA_IDS = 50; -interface RcEntitlement { - id: string; - lookup_key: string; -} - -interface RcCustomerActiveEntitlement { - entitlement_id: string; - // expires_at is intentionally not checked client-side — we trust RC's - // server-side active filter, which keeps in-grace-period entitlements active. - expires_at: number | null; -} - -interface RcCustomer { - id: string; - active_entitlements?: { - items?: RcCustomerActiveEntitlement[]; - }; -} - -interface RcListResponse { - items: T[]; - next_page: string | null; -} +serve(handler); -serve(async (req) => { +async function handler(req: Request): Promise { const runAt = new Date().toISOString(); try { @@ -133,27 +114,26 @@ serve(async (req) => { // unreliable. The webhook's `revenuecat_webhook_user_not_found` // captures one half of that gap (RC event for an unknown profile) and // orphans from already-deleted accounts are not operationally urgent. - const driftPremiumMissing: string[] = []; - const driftPremiumStale: string[] = []; - - for (const profile of profiles) { - const rcActivePremium = await isCustomerActivePremium( - profile.id, + const { driftPremiumMissing, driftPremiumStale, driftCheckFailed } = + await classifyProfilesDrift( + profiles, projectId, rcKey, premiumEntitlementId, + (profileId, error) => { + console.error(JSON.stringify({ + event: "rc_customer_check_failed", + job: JOB_NAME, + profile_id: profileId, + error, + })); + }, ); - if (profile.entitlement === "premium" && !rcActivePremium) { - driftPremiumStale.push(profile.id); - } else if (profile.entitlement === "free" && rcActivePremium) { - driftPremiumMissing.push(profile.id); - } - } - const countMissing = driftPremiumMissing.length; const countStale = driftPremiumStale.length; const driftCount = countMissing + countStale; + const countFailed = driftCheckFailed.length; // 6. Single Sentry event when drift > 0; stable fingerprint collapses // consecutive runs into one issue. No event on the healthy path — @@ -180,6 +160,30 @@ serve(async (req) => { await Sentry.flush(2000); } + // 7. Separate signal for runs with incomplete coverage (RC lookups that + // failed even after retry). Distinct fingerprint from the drift event + // above so the two never collapse into or suppress each other — a run + // can have both real drift AND some unchecked profiles at once. The + // next scheduled run re-checks every profile from scratch, so no + // backfill is needed once failed_count returns to 0. + if (countFailed > 0) { + Sentry.captureMessage("rc_drift_check_partial_failure", { + level: "warning", + fingerprint: ["rc-drift-check-partial-failure"], + tags: { + function: JOB_NAME, + context: "rc_drift_check_partial_failure", + failed_count: String(countFailed), + profile_count: String(profiles.length), + }, + extra: { + failed_profile_ids: driftCheckFailed.slice(0, MAX_EXTRA_IDS), + run_at: runAt, + }, + }); + await Sentry.flush(2000); + } + console.log(JSON.stringify({ event: "drift_check_complete", job: JOB_NAME, @@ -187,11 +191,12 @@ serve(async (req) => { drift_count: driftCount, count_missing: countMissing, count_stale: countStale, + failed_count: countFailed, supabase_profile_count: profiles.length, })); return new Response( - JSON.stringify({ drift_count: driftCount }), + JSON.stringify({ drift_count: driftCount, failed_count: countFailed }), { status: 200, headers: { "Content-Type": "application/json" } }, ); } finally { @@ -217,53 +222,4 @@ serve(async (req) => { { status: 500, headers: { "Content-Type": "application/json" } }, ); } -}); - -async function resolvePremiumEntitlementId( - projectId: string, - rcKey: string, -): Promise { - // MapVault has one entitlement today (lookup_key="premium"). If the project - // ever has more than 100 entitlements, this needs pagination via - // starting_after — but well before that point, the data model has changed - // enough that the drift check itself should be re-evaluated. - const res = await fetch( - `${RC_API_BASE}/projects/${projectId}/entitlements?limit=100`, - { - headers: { Authorization: `Bearer ${rcKey}` }, - signal: AbortSignal.timeout(10_000), - }, - ); - if (!res.ok) { - throw new Error(`rc_list_entitlements_${res.status}`); - } - const data = (await res.json()) as RcListResponse; - return data.items.find((e) => e.lookup_key === "premium")?.id ?? null; -} - -async function isCustomerActivePremium( - customerId: string, - projectId: string, - rcKey: string, - premiumEntitlementId: string, -): Promise { - const res = await fetch( - `${RC_API_BASE}/projects/${projectId}/customers/${customerId}`, - { - headers: { Authorization: `Bearer ${rcKey}` }, - signal: AbortSignal.timeout(10_000), - }, - ); - // 404 = customer has no RC record at all. That's not "drift" by itself; - // a free-tier Supabase user who never went near payments will look like - // this, which is the healthy state. Treat as "not active premium". - if (res.status === 404) { - return false; - } - if (!res.ok) { - throw new Error(`rc_get_customer_${res.status}`); - } - const customer = (await res.json()) as RcCustomer; - const items = customer.active_entitlements?.items ?? []; - return items.some((e) => e.entitlement_id === premiumEntitlementId); } diff --git a/supabase/functions/rc-entitlement-drift-check/rc-client.test.ts b/supabase/functions/rc-entitlement-drift-check/rc-client.test.ts new file mode 100644 index 0000000..85cfdad --- /dev/null +++ b/supabase/functions/rc-entitlement-drift-check/rc-client.test.ts @@ -0,0 +1,235 @@ +// deno-lint-ignore-file no-explicit-any +import { assertEquals } from "https://deno.land/std@0.168.0/testing/asserts.ts"; +import { + checkCustomerActivePremiumWithRetry, + classifyProfilesDrift, + type ProfileEntitlement, +} from "./rc-client.ts"; + +const PROJECT_ID = "proj_test"; +const RC_KEY = "rc_key_test"; +const PREMIUM_ENTITLEMENT_ID = "ent_premium"; + +function jsonResponse(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +function customerBody(active: boolean) { + return { + id: "cust_1", + active_entitlements: { + items: active ? [{ entitlement_id: PREMIUM_ENTITLEMENT_ID, expires_at: null }] : [], + }, + }; +} + +function withStubbedFetch( + responses: Array<() => Response | Promise>, + run: () => Promise, +) { + const originalFetch = globalThis.fetch; + let call = 0; + globalThis.fetch = (async (..._args: unknown[]) => { + const factory = responses[call] ?? responses[responses.length - 1]; + call += 1; + return factory(); + }) as typeof fetch; + + return run().finally(() => { + globalThis.fetch = originalFetch; + }); +} + +Deno.test("retries once on 503 and succeeds", async () => { + await withStubbedFetch( + [ + () => jsonResponse(503, {}), + () => jsonResponse(200, customerBody(true)), + ], + async () => { + const result = await checkCustomerActivePremiumWithRetry( + "cust_1", + PROJECT_ID, + RC_KEY, + PREMIUM_ENTITLEMENT_ID, + ); + assertEquals(result, { ok: true, active: true }); + }, + ); +}); + +Deno.test("exhausts retries on repeated 503 and returns ok:false without throwing", async () => { + await withStubbedFetch( + [ + () => jsonResponse(503, {}), + () => jsonResponse(503, {}), + ], + async () => { + const result = await checkCustomerActivePremiumWithRetry( + "cust_1", + PROJECT_ID, + RC_KEY, + PREMIUM_ENTITLEMENT_ID, + ); + assertEquals(result.ok, false); + }, + ); +}); + +Deno.test("retries once on a network error (TypeError) and succeeds", async () => { + let calls = 0; + await withStubbedFetch( + [ + () => { + calls += 1; + throw new TypeError("network error"); + }, + () => { + calls += 1; + return jsonResponse(200, customerBody(true)); + }, + ], + async () => { + const result = await checkCustomerActivePremiumWithRetry( + "cust_1", + PROJECT_ID, + RC_KEY, + PREMIUM_ENTITLEMENT_ID, + ); + assertEquals(result, { ok: true, active: true }); + assertEquals(calls, 2); + }, + ); +}); + +Deno.test("retries once on a timeout (DOMException) and succeeds", async () => { + let calls = 0; + await withStubbedFetch( + [ + () => { + calls += 1; + throw new DOMException("timed out", "TimeoutError"); + }, + () => { + calls += 1; + return jsonResponse(200, customerBody(true)); + }, + ], + async () => { + const result = await checkCustomerActivePremiumWithRetry( + "cust_1", + PROJECT_ID, + RC_KEY, + PREMIUM_ENTITLEMENT_ID, + ); + assertEquals(result, { ok: true, active: true }); + assertEquals(calls, 2); + }, + ); +}); + +Deno.test("404 short-circuits to not-active with no retry", async () => { + let calls = 0; + await withStubbedFetch( + [ + () => { + calls += 1; + return jsonResponse(404, {}); + }, + ], + async () => { + const result = await checkCustomerActivePremiumWithRetry( + "cust_1", + PROJECT_ID, + RC_KEY, + PREMIUM_ENTITLEMENT_ID, + ); + assertEquals(result, { ok: true, active: false }); + assertEquals(calls, 1); + }, + ); +}); + +Deno.test("4xx fails fast without retry", async () => { + let calls = 0; + await withStubbedFetch( + [ + () => { + calls += 1; + return jsonResponse(401, {}); + }, + ], + async () => { + const result = await checkCustomerActivePremiumWithRetry( + "cust_1", + PROJECT_ID, + RC_KEY, + PREMIUM_ENTITLEMENT_ID, + ); + assertEquals(result.ok, false); + assertEquals(calls, 1); + }, + ); +}); + +function withStubbedFetchByCustomer( + responsesByCustomer: Record Response>>, + run: () => Promise, +) { + const originalFetch = globalThis.fetch; + const callCounts: Record = {}; + globalThis.fetch = ((url: string | URL) => { + const customerId = url.toString().split("/").pop()!; + const responses = responsesByCustomer[customerId] ?? []; + const call = callCounts[customerId] ?? 0; + callCounts[customerId] = call + 1; + const factory = responses[call] ?? responses[responses.length - 1]; + return Promise.resolve(factory()); + }) as typeof fetch; + + return run().finally(() => { + globalThis.fetch = originalFetch; + }); +} + +// Drives the actual production code path (`classifyProfilesDrift`, called +// directly by index.ts's handler) rather than re-implementing the loop here, +// so a regression in the real isolation logic (e.g. someone reintroducing a +// throw instead of a `continue`) would be caught by this test. +Deno.test("classifyProfilesDrift isolates a failing customer instead of aborting the batch", async () => { + const profiles: ProfileEntitlement[] = [ + { id: "cust_a", entitlement: "free" }, + { id: "cust_b", entitlement: "free" }, + { id: "cust_c", entitlement: "premium" }, + ]; + const failed: Array<{ id: string; error: string }> = []; + + await withStubbedFetchByCustomer( + { + cust_a: [() => jsonResponse(503, {}), () => jsonResponse(200, customerBody(true))], + cust_b: [() => jsonResponse(503, {}), () => jsonResponse(503, {})], + cust_c: [() => jsonResponse(404, {})], + }, + async () => { + const classification = await classifyProfilesDrift( + profiles, + PROJECT_ID, + RC_KEY, + PREMIUM_ENTITLEMENT_ID, + (id, error) => failed.push({ id, error }), + ); + + // cust_a: free + active after retry -> missing. cust_b: retries + // exhausted -> isolated, not classified as drift either way. cust_c: + // premium + 404 (not active) -> stale. + assertEquals(classification.driftPremiumMissing, ["cust_a"]); + assertEquals(classification.driftPremiumStale, ["cust_c"]); + assertEquals(classification.driftCheckFailed, ["cust_b"]); + }, + ); + + assertEquals(failed.map((f) => f.id), ["cust_b"]); +}); diff --git a/supabase/functions/rc-entitlement-drift-check/rc-client.ts b/supabase/functions/rc-entitlement-drift-check/rc-client.ts new file mode 100644 index 0000000..b85b28a --- /dev/null +++ b/supabase/functions/rc-entitlement-drift-check/rc-client.ts @@ -0,0 +1,203 @@ +// RevenueCat v2 API client for the drift-check job, kept dependency-free +// (no Sentry/Supabase imports) so rc-client.test.ts can exercise the retry +// and fault-isolation logic without pulling in the full edge function runtime. + +const RC_API_BASE = "https://api.revenuecat.com/v2"; +const RC_CUSTOMER_CHECK_MAX_ATTEMPTS = 2; +const RC_CUSTOMER_CHECK_RETRY_DELAY_MS = 300; + +interface RcEntitlement { + id: string; + lookup_key: string; +} + +interface RcCustomerActiveEntitlement { + entitlement_id: string; + // expires_at is intentionally not checked client-side — we trust RC's + // server-side active filter, which keeps in-grace-period entitlements active. + expires_at: number | null; +} + +interface RcCustomer { + id: string; + active_entitlements?: { + items?: RcCustomerActiveEntitlement[]; + }; +} + +interface RcListResponse { + items: T[]; + next_page: string | null; +} + +export async function resolvePremiumEntitlementId( + projectId: string, + rcKey: string, +): Promise { + // MapVault has one entitlement today (lookup_key="premium"). If the project + // ever has more than 100 entitlements, this needs pagination via + // starting_after — but well before that point, the data model has changed + // enough that the drift check itself should be re-evaluated. + const res = await fetch( + `${RC_API_BASE}/projects/${projectId}/entitlements?limit=100`, + { + headers: { Authorization: `Bearer ${rcKey}` }, + signal: AbortSignal.timeout(10_000), + }, + ); + if (!res.ok) { + throw new Error(`rc_list_entitlements_${res.status}`); + } + const data = (await res.json()) as RcListResponse; + return data.items.find((e) => e.lookup_key === "premium")?.id ?? null; +} + +// Thrown only for 5xx responses so the retry wrapper below can tell a +// transient RC-side failure apart from a permanent one (4xx) without +// retrying errors that will never succeed. +class RcTransientHttpError extends Error { + constructor(status: number) { + super(`rc_get_customer_${status}`); + this.name = "RcTransientHttpError"; + } +} + +async function isCustomerActivePremium( + customerId: string, + projectId: string, + rcKey: string, + premiumEntitlementId: string, +): Promise { + const res = await fetch( + `${RC_API_BASE}/projects/${projectId}/customers/${customerId}`, + { + headers: { Authorization: `Bearer ${rcKey}` }, + signal: AbortSignal.timeout(10_000), + }, + ); + // 404 = customer has no RC record at all. That's not "drift" by itself; + // a free-tier Supabase user who never went near payments will look like + // this, which is the healthy state. Treat as "not active premium". + if (res.status === 404) { + return false; + } + if (!res.ok) { + if (res.status >= 500) { + throw new RcTransientHttpError(res.status); + } + throw new Error(`rc_get_customer_${res.status}`); + } + const customer = (await res.json()) as RcCustomer; + const items = customer.active_entitlements?.items ?? []; + return items.some((e) => e.entitlement_id === premiumEntitlementId); +} + +// Network failures and timeouts are just as transient as a 5xx — retry both, +// but not a 4xx (auth/bad-request are permanent; retrying wastes the run's +// time budget on something that will never succeed). +function isTransientRcError(err: unknown): boolean { + if (err instanceof RcTransientHttpError) return true; + if (err instanceof TypeError) return true; // fetch network failure + if (err instanceof DOMException) { + return err.name === "TimeoutError" || err.name === "AbortError"; + } + return false; +} + +export type CustomerCheckResult = + | { ok: true; active: boolean } + | { ok: false; error: string }; + +// One retry (two attempts total) for transient RC failures on a single +// customer, so one flaky lookup can't take down drift detection for every +// other profile in the run — the caller isolates a `{ ok: false }` result +// per-profile instead of letting it throw. See MAPVAULT-J. +export async function checkCustomerActivePremiumWithRetry( + customerId: string, + projectId: string, + rcKey: string, + premiumEntitlementId: string, +): Promise { + let lastError: unknown; + + for (let attempt = 1; attempt <= RC_CUSTOMER_CHECK_MAX_ATTEMPTS; attempt++) { + try { + const active = await isCustomerActivePremium( + customerId, + projectId, + rcKey, + premiumEntitlementId, + ); + return { ok: true, active }; + } catch (err) { + lastError = err; + if ( + !isTransientRcError(err) || + attempt === RC_CUSTOMER_CHECK_MAX_ATTEMPTS + ) { + break; + } + const jitter = Math.random() * 150; + await new Promise((resolve) => + setTimeout(resolve, RC_CUSTOMER_CHECK_RETRY_DELAY_MS + jitter) + ); + } + } + + const message = lastError instanceof Error + ? lastError.message + : String(lastError); + return { ok: false, error: message }; +} + +export interface ProfileEntitlement { + id: string; + entitlement: string; +} + +export interface DriftClassification { + driftPremiumMissing: string[]; + driftPremiumStale: string[]; + driftCheckFailed: string[]; +} + +// Walks every profile, checking RC state one at a time (see index.ts for why +// this is sequential rather than batched) and classifying drift. A profile +// whose RC check fails even after retry is isolated into `driftCheckFailed` +// rather than aborting the rest of the run — the fix for MAPVAULT-J. Kept +// here (rather than inline in index.ts) so it can be exercised directly by +// tests with a stubbed `fetch`, without needing to stub Supabase/Sentry too. +export async function classifyProfilesDrift( + profiles: ProfileEntitlement[], + projectId: string, + rcKey: string, + premiumEntitlementId: string, + onCheckFailed?: (profileId: string, error: string) => void, +): Promise { + const driftPremiumMissing: string[] = []; + const driftPremiumStale: string[] = []; + const driftCheckFailed: string[] = []; + + for (const profile of profiles) { + const result = await checkCustomerActivePremiumWithRetry( + profile.id, + projectId, + rcKey, + premiumEntitlementId, + ); + + if (!result.ok) { + onCheckFailed?.(profile.id, result.error); + driftCheckFailed.push(profile.id); + continue; + } + + if (profile.entitlement === "premium" && !result.active) { + driftPremiumStale.push(profile.id); + } else if (profile.entitlement === "free" && result.active) { + driftPremiumMissing.push(profile.id); + } + } + + return { driftPremiumMissing, driftPremiumStale, driftCheckFailed }; +}