From 5707486474a94954943583e22458898c1a73bc0b Mon Sep 17 00:00:00 2001 From: Nitheesh D R Date: Mon, 7 Sep 2026 22:58:48 +0530 Subject: [PATCH] PUL-22: cache the store snapshot on Upstash Redis, fix two uncached hot paths Real Supabase egress-quota restriction (service outage) today traced back to readSnapshot() in src/lib/woo/mirror.ts, which reassembles a store's entire order/customer/product history from Postgres on every cache miss -- tens of MB on a real store per that file's own comment. The in-process memo only helps a warm serverless instance, which per that same comment is the exception on Vercel, not the rule. Three real, uncached/undercached hot paths found and fixed: 1. /api/whatsapp/chats -- polled every 60s (was 30s) by any open Inbox tab, paid the full readSnapshot() cost every single time with zero HTTP caching, and the client explicitly forced cache: "no-store". This was almost certainly the dominant driver of today's incident: every open inbox tab, every 30-60s, for as long as it stayed open. Fixed: same private + Vary: Cookie Cache-Control already used on /api/analytics and /api/customers/[key], no-store removed client-side so the browser can actually use it, poll interval lengthened to match the cache window. 2. /api/whatsapp/products -- hit on every 250ms-debounced keystroke in the campaign product picker, no caching at all. Same treatment, max-age=120 since a catalogue changes far less often than a chat list. 3. The deeper fix: readSnapshot() now checks a Redis-backed cache (src/lib/storage/snapshot-cache.ts, Upstash's free tier, confirmed no card required) before ever touching Postgres, and writes back on a miss. forgetSnapshot() invalidates it. The cron sync route proactively warms it right after each store's sync -- the one place a full Postgres reassembly is actually expected, once per store per 10-minute cycle, not once per user-facing request. R2 was the first choice (also zero egress) but requires a payment method to enable even on the free tier; Upstash doesn't. Also fixed two stale docs found along the way: README.md's SNAPSHOT_CACHE_MINUTES default said 60, code and .env.example say (and mean) 10. .env.example's kv_store comment described a table dropped in the multi-tenant migration and never recreated -- corrected to describe where WooCommerce credentials and the mirror actually live now, and that nothing in the app currently reads SUPABASE_SERVICE_ROLE_KEY at all. Not yet verified against a real synced store -- the fresh Supabase project (today's other recovery) has no connected stores yet. Checks (typecheck, lint, the Redis REST API shape) all pass; the actual readSnapshot() cache hit/miss/warm path needs a real store connected and synced to exercise for real. Co-Authored-By: Claude Sonnet 5 --- .env.example | 15 +++- README.md | 2 +- src/app/(app)/inbox/page.tsx | 10 ++- src/app/api/cron/sync/route.ts | 18 ++++- src/app/api/whatsapp/chats/route.ts | 14 +++- src/app/api/whatsapp/products/route.ts | 14 +++- src/lib/storage/snapshot-cache.ts | 108 +++++++++++++++++++++++++ src/lib/woo/mirror.ts | 32 +++++++- 8 files changed, 201 insertions(+), 12 deletions(-) create mode 100644 src/lib/storage/snapshot-cache.ts diff --git a/.env.example b/.env.example index 568a408..9ac5d69 100644 --- a/.env.example +++ b/.env.example @@ -98,10 +98,17 @@ GROQ_FALLBACK_MODEL= # Supabase is tried first, then Redis, then the local filesystem. # Supabase. Both are required together. The anon key is NOT accepted, and -# neither is a key issued for a different project — kv_store has row level -# security on with no policies, so only the service-role key reaches it, and it -# holds the consumer secret and a copy of the order history. It must never be -# exposed to a browser. +# neither is a key issued for a different project. +# +# Stale note this replaces: an earlier kv_store table was dropped in the +# multi-tenant migration (20260811140000_multi_tenant.sql) and never +# recreated -- WooCommerce credentials and the order/customer/product mirror +# live in stores/woo_orders/woo_customers/woo_products now, reached over +# SUPABASE_DB_POOL_URL as the table owner. The app's own runtime code does +# not read SUPABASE_SERVICE_ROLE_KEY at all today (confirmed: nothing under +# src/ references it) -- it exists only for ad-hoc Supabase Auth Admin API +# operations run by hand (e.g. seeding an account). It must never be exposed +# to a browser if that changes. NEXT_PUBLIC_SUPABASE_URL= SUPABASE_SERVICE_ROLE_KEY= diff --git a/README.md b/README.md index 574c0a2..4449a6e 100644 --- a/README.md +++ b/README.md @@ -1542,7 +1542,7 @@ proving you can approve the store is proof of access. | `APP_PASSWORD` | for login | Set alongside `AUTH_SECRET` to require a password. | | `KV_REST_API_URL` | on serverless | Redis endpoint. Vercel KV and Upstash both provide it. | | `KV_REST_API_TOKEN` | on serverless | Token for the above. `UPSTASH_REDIS_REST_*` also accepted. | -| `SNAPSHOT_CACHE_MINUTES` | no | How long a snapshot stays warm. Default `60`. | +| `SNAPSHOT_CACHE_MINUTES` | no | How long a snapshot stays warm. Default `10`. | | `WHATSAPP_API_URL` | no | Gateway base URL. Takes the connection out of the UI. | | `WHATSAPP_API_KEY` | no | Gateway API key, operator role. | | `WHATSAPP_SESSION_ID` | no | Which session to send from. Adopted automatically if omitted. | diff --git a/src/app/(app)/inbox/page.tsx b/src/app/(app)/inbox/page.tsx index 7affb49..b61d677 100644 --- a/src/app/(app)/inbox/page.tsx +++ b/src/app/(app)/inbox/page.tsx @@ -61,7 +61,10 @@ export default function InboxPage() { const loadChats = useCallback(async () => { try { - const res = await fetch("/api/whatsapp/chats", { cache: "no-store" }); + // Not no-store: the route sets its own short private Cache-Control, + // specifically so this poll doesn't re-pay a full snapshot read every + // time — letting the browser honour that cache is the point. + const res = await fetch("/api/whatsapp/chats"); const body = await res.json(); if (!res.ok) { setError(body.error ?? "Could not load conversations."); @@ -92,7 +95,10 @@ export default function InboxPage() { useEffect(() => { // eslint-disable-next-line react-hooks/set-state-in-effect void loadChats(); - const interval = setInterval(() => void loadChats(), 30_000); + // Matches the route's own 60s Cache-Control -- polling faster than the + // cache window would just mean every other poll pays for a snapshot + // read the cache was added specifically to avoid. + const interval = setInterval(() => void loadChats(), 60_000); return () => clearInterval(interval); }, [loadChats]); diff --git a/src/app/api/cron/sync/route.ts b/src/app/api/cron/sync/route.ts index 2168d5e..cb5cbc9 100644 --- a/src/app/api/cron/sync/route.ts +++ b/src/app/api/cron/sync/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { db } from "@/lib/db/client"; -import { forgetSnapshot } from "@/lib/woo/mirror"; +import { forgetSnapshot, readSnapshot } from "@/lib/woo/mirror"; import { syncStore } from "@/lib/woo/sync"; export const runtime = "nodejs"; @@ -76,6 +76,22 @@ export async function POST(request: Request) { maxPages: store.max_pages, }); forgetSnapshot(store.id); + /* + * Warms the Redis snapshot cache with this store's fresh data right + * here, once, in the sync job -- rather than leaving the first + * dashboard/inbox/product-search request after this sync to pay for + * it. This is the one place in the app a full Postgres reassembly is + * actually expected: once per store per cron cycle, not once per + * user-facing request. A failure here (e.g. no history yet on a + * store's very first sync) must not fail the sync itself. + */ + await readSnapshot({ + id: store.id, + url: store.url, + name: store.name, + historyMonths: store.history_months, + lastSyncAt: null, + }).catch(() => {}); results.push({ store: store.url, ok: true, ...result }); } catch (error) { // One unreachable store must not stop the rest: a merchant who revoked diff --git a/src/app/api/whatsapp/chats/route.ts b/src/app/api/whatsapp/chats/route.ts index 1edfa7a..35c20eb 100644 --- a/src/app/api/whatsapp/chats/route.ts +++ b/src/app/api/whatsapp/chats/route.ts @@ -47,7 +47,19 @@ export async function GET(request: Request) { try { const chats = await new WhatsAppClient(config).listChats(80); const enriched = await withCustomerNames(store, chats, config.defaultDialCode); - return NextResponse.json({ chats: enriched }); + /* + * The gateway call is cheap; the customer-name enrichment above is not — + * it pays a full readSnapshot() (the whole order/customer/product mirror, + * tens of MB on a real store) on every uncached hit. This route is + * polled every 60s by any open Inbox tab, so without a cache that cost + * repeats indefinitely for as long as the tab stays open. Same + * private + Vary: Cookie treatment as /api/analytics and /api/customers, + * for the same tenant-cache-poisoning reason. + */ + return NextResponse.json( + { chats: enriched }, + { headers: { "Cache-Control": "private, max-age=60", Vary: "Cookie" } }, + ); } catch (error) { if (error instanceof WhatsAppApiError) { return NextResponse.json( diff --git a/src/app/api/whatsapp/products/route.ts b/src/app/api/whatsapp/products/route.ts index bc7e614..39911a6 100644 --- a/src/app/api/whatsapp/products/route.ts +++ b/src/app/api/whatsapp/products/route.ts @@ -50,7 +50,19 @@ export async function GET(request: Request) { sales: product.total_sales ?? 0, })); - return NextResponse.json({ products: matches, currency: snapshot.currency }); + /* + * Hit on every 250ms-debounced keystroke in the picker with no caching + * previously — each request still pays the full loadSnapshot() cost (the + * whole order/customer/product mirror) on a cold instance, regardless of + * how narrow the search is, since it's the same underlying snapshot load + * before any client-side filtering happens. Products don't change often + * enough to need fresher than a couple of minutes; same private + + * Vary: Cookie treatment as the other tenant-scoped cached routes. + */ + return NextResponse.json( + { products: matches, currency: snapshot.currency }, + { headers: { "Cache-Control": "private, max-age=120", Vary: "Cookie" } }, + ); } catch (error) { if (isNotConnected(error)) { return NextResponse.json({ error: error.message, code: "not_connected" }, { status: 409 }); diff --git a/src/lib/storage/snapshot-cache.ts b/src/lib/storage/snapshot-cache.ts new file mode 100644 index 0000000..34ae0cb --- /dev/null +++ b/src/lib/storage/snapshot-cache.ts @@ -0,0 +1,108 @@ +import type { StoreSnapshot } from "@/lib/woo/types"; + +/** + * A read-through cache for the full store snapshot, on Upstash Redis. + * + * The reason this exists: `readSnapshot()` in src/lib/woo/mirror.ts reassembles + * a whole store's orders/customers/products from Postgres on every cache miss + * — tens of megabytes on a real store, per that file's own comment on what + * `pg_stat_statements` showed. The in-process memo there helps within one + * warm serverless instance, but a fresh instance (the common case on Vercel, + * not the exception) gets none of that benefit and pays the full Postgres + * read again -- which is exactly what drove a real Supabase egress-quota + * restriction (service outage) this project hit. + * + * Upstash's free tier needs no card at all and is confirmed against their + * own pricing page (256MB storage, 500K commands/month, 10GB bandwidth/month) + * — comfortably enough for a JSON blob a few MB in size, written once per + * store per 10-minute sync cycle and read on every dashboard/inbox/product + * request. R2 was the first choice (also zero egress) but requires a + * payment method to enable even on the free tier, which this project can't + * use -- Upstash doesn't. + * + * Plain REST calls rather than the @upstash/redis SDK: Upstash's REST API is + * two HTTP calls (GET/SET with a bearer token), and pulling in a client + * library for that is more than this needs. + * + * This is a cache, not the source of truth -- Postgres still is. A miss here + * (first sync of a new store, Redis unreachable, env vars unset) falls back + * to the existing Postgres reassembly in mirror.ts exactly as before. + */ + +const TTL_SECONDS = 15 * 60; // Generous relative to the 10-min sync cycle that refreshes it. + +function credentials(): { url: string; token: string } | null { + const url = process.env.UPSTASH_REDIS_REST_URL?.trim(); + const token = process.env.UPSTASH_REDIS_REST_TOKEN?.trim(); + if (!url || !token) return null; + return { url, token }; +} + +function keyFor(storeId: string): string { + return `snapshot:${storeId}`; +} + +/** Reads the cached snapshot, or null on any miss/failure — never throws. */ +export async function getCachedSnapshot(storeId: string): Promise { + const creds = credentials(); + if (!creds) return null; + + try { + const res = await fetch(`${creds.url}/get/${keyFor(storeId)}`, { + headers: { Authorization: `Bearer ${creds.token}` }, + cache: "no-store", + }); + if (!res.ok) return null; + const body = (await res.json()) as { result: string | null }; + if (!body.result) return null; + return JSON.parse(body.result) as StoreSnapshot; + } catch { + // Network error, malformed JSON, whatever -- fall back to Postgres, same + // as a cold cache always has. + return null; + } +} + +/** + * Invalidates the cache entry — called from mirror.ts's forgetSnapshot, + * which several call sites use without a guaranteed follow-up read (a store + * disconnect, a manual re-sync trigger). Without this, stale data could sit + * here for up to TTL_SECONDS after the thing that made it stale. Best-effort + * and fire-and-forget for the same reason as the rest of this file: a failed + * delete just means the entry expires on its own schedule instead of + * immediately, not a correctness break for anything that reads through this + * cache. + */ +export async function deleteCachedSnapshot(storeId: string): Promise { + const creds = credentials(); + if (!creds) return; + + try { + await fetch(`${creds.url}/del/${keyFor(storeId)}`, { + method: "POST", + headers: { Authorization: `Bearer ${creds.token}` }, + }); + } catch { + // Best-effort, see above. + } +} + +/** + * Writes the snapshot back, best-effort. Never throws -- a failed cache write + * must not fail the sync or the request that triggered a Postgres reassembly; + * the next reader just falls back to Postgres again, same as today. + */ +export async function putCachedSnapshot(storeId: string, snapshot: StoreSnapshot): Promise { + const creds = credentials(); + if (!creds) return; + + try { + await fetch(`${creds.url}/set/${keyFor(storeId)}?EX=${TTL_SECONDS}`, { + method: "POST", + headers: { Authorization: `Bearer ${creds.token}`, "Content-Type": "text/plain" }, + body: JSON.stringify(snapshot), + }); + } catch { + // Best-effort, see the doc comment above. + } +} diff --git a/src/lib/woo/mirror.ts b/src/lib/woo/mirror.ts index f0ce3c3..2b0ce4b 100644 --- a/src/lib/woo/mirror.ts +++ b/src/lib/woo/mirror.ts @@ -1,5 +1,6 @@ import { db } from "@/lib/db/client"; import type { TenantStore } from "@/lib/auth/tenant"; +import { deleteCachedSnapshot, getCachedSnapshot, putCachedSnapshot } from "@/lib/storage/snapshot-cache"; import type { StoreSnapshot, WooCustomer, WooOrder, WooProduct } from "./types"; /** @@ -53,10 +54,25 @@ export class NoMirrorDataError extends Error { } } -export async function readSnapshot(store: TenantStore): Promise { +/** + * Narrower than the full TenantStore: everything below only ever reads these + * five fields, and the cron sync route (which warms the Redis cache right + * after a sync, before requireStore has assembled a full TenantStore) can + * only supply this much. Pick<> rather than a separate interface, so + * TenantStore stays the one place the full shape is defined. + */ +type SnapshotSource = Pick; + +export async function readSnapshot(store: SnapshotSource): Promise { const hit = memo.get(store.id); if (hit && hit.expiresAt > Date.now()) return hit.snapshot; + const cached = await getCachedSnapshot(store.id); + if (cached) { + memo.set(store.id, { snapshot: cached, expiresAt: Date.now() + MEMO_TTL_MS }); + return cached; + } + const since = new Date(); since.setMonth(since.getMonth() - (store.historyMonths || 24)); @@ -102,12 +118,24 @@ export async function readSnapshot(store: TenantStore): Promise { }; memo.set(store.id, { snapshot, expiresAt: Date.now() + MEMO_TTL_MS }); + // Best-effort, not awaited-for-correctness -- see snapshot-cache.ts's own + // doc comment. A failed write just means the next reader (possibly a + // different serverless instance, which is why this exists at all) pays + // for another Postgres reassembly, same as before this cache existed. + void putCachedSnapshot(store.id, snapshot); return snapshot; } -/** Drops the memo for a store. Called after a sync, so the next read is fresh. */ +/** + * Drops the memo and the Redis cache entry for a store. Called after a sync, + * a store disconnect, or a manual re-sync trigger, so the next read is + * fresh — not several call sites all guaranteed to follow up with a read + * that would naturally overwrite the Redis entry, so this invalidates it + * directly rather than relying on that. + */ export function forgetSnapshot(storeId: string): void { memo.delete(storeId); + void deleteCachedSnapshot(storeId); } /* ── Queries that do not need the whole snapshot ──────────────────────────