From d85a9b885f235b641030d57bcb22054119916144 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:09:05 +0000 Subject: [PATCH] fix(billing): stop writing empty Autumn records on read failure A transient Postgres failure in getAutumnCustomerFields was swallowed and returned bare defaults, so getOrCreateAutumnCustomer then overwrote the stored name, email, and account type with only source metadata. - Retry the identity read once, then return null instead of defaults when it still fails. - Callers skip the Autumn write on null: the billing panel returns no state; usage tracking skips the identity write but still records the meter. - Normalize a drizzle query error before capture (statement only, drop the bound parameters, keep the driver cause) so error tracking no longer fragments per user id. Generated-By: PostHog Desktop Task-Id: b15350b1-ea09-4023-b6ad-e404b05595a4 --- src/features/account/billing-functions.ts | 8 ++ src/integrations/autumn/client.server.test.ts | 110 ++++++++++++++++++ src/integrations/autumn/client.server.ts | 102 +++++++++------- .../observability/operational-error.test.ts | 34 +++++- .../observability/operational-error.ts | 40 +++++++ .../observability/operational-events.ts | 10 +- 6 files changed, 258 insertions(+), 46 deletions(-) create mode 100644 src/integrations/autumn/client.server.test.ts diff --git a/src/features/account/billing-functions.ts b/src/features/account/billing-functions.ts index 809b2c417..70d22292c 100644 --- a/src/features/account/billing-functions.ts +++ b/src/features/account/billing-functions.ts @@ -54,6 +54,14 @@ export const getWorkspaceBillingStateFn = createServerFn({ method: "GET" }).hand } const customerFields = await getAutumnCustomerFields(userId); + + // Null means the read failed (see getAutumnCustomerFields). Skip + // get_or_create so its empty fields never overwrite the stored identity; + // this load goes without fresh balances rather than degrading the record. + if (!customerFields) { + return null; + } + const customer = await getOrCreateAutumnCustomer({ customerId: userId, secretKey, diff --git a/src/integrations/autumn/client.server.test.ts b/src/integrations/autumn/client.server.test.ts new file mode 100644 index 000000000..cc0cf92fa --- /dev/null +++ b/src/integrations/autumn/client.server.test.ts @@ -0,0 +1,110 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +vi.mock("#/db/server", () => ({ + withDb: vi.fn(), +})); + +vi.mock("#/integrations/observability/operational-events", () => ({ + recordOperationalFailure: vi.fn(), +})); + +vi.mock("#/integrations/autumn/rest", () => ({ + getOrCreateAutumnCustomer: vi.fn(async () => ({ balances: {}, id: "u", subscriptions: [] })), + trackAutumnBalance: vi.fn(async () => undefined), +})); + +import { withDb } from "#/db/server"; +import { getAutumnCustomerFields, trackAutumnUsage } from "#/integrations/autumn/client.server"; +import { getOrCreateAutumnCustomer, trackAutumnBalance } from "#/integrations/autumn/rest"; +import { recordOperationalFailure } from "#/integrations/observability/operational-events"; + +const REGISTERED_ROW = { + createdAt: new Date("2026-01-02T03:04:05.000Z"), + email: "person@example.com", + emailVerified: true, + isAnonymous: false, + name: "Ada Example", +}; + +// Runs the caller's handler against a db stub whose query resolves to `rows`, +// mirroring a successful `withDb` attempt. +function readsRows(rows: unknown[]) { + const db = { + select: () => ({ from: () => ({ where: () => ({ limit: () => Promise.resolve(rows) }) }) }), + }; + + return (handler: (db: unknown) => unknown) => handler(db); +} + +afterEach(() => { + vi.clearAllMocks(); +}); + +describe("getAutumnCustomerFields", () => { + it("maps a registered user row into customer fields", async () => { + vi.mocked(withDb).mockImplementationOnce(readsRows([REGISTERED_ROW]) as never); + + await expect(getAutumnCustomerFields("user_1")).resolves.toEqual({ + email: "person@example.com", + name: "Ada Example", + metadata: { + account_type: "registered", + email_verified: true, + source: "thinkex", + user_created_at: "2026-01-02T03:04:05.000Z", + }, + }); + expect(recordOperationalFailure).not.toHaveBeenCalled(); + }); + + it("returns defaults when the user has no row", async () => { + vi.mocked(withDb).mockImplementationOnce(readsRows([]) as never); + + await expect(getAutumnCustomerFields("user_1")).resolves.toEqual({ + metadata: { source: "thinkex" }, + }); + }); + + it("retries once and recovers from a transient read failure", async () => { + vi.mocked(withDb) + .mockRejectedValueOnce(new Error("Internal error.")) + .mockImplementationOnce(readsRows([REGISTERED_ROW]) as never); + + await expect(getAutumnCustomerFields("user_1")).resolves.toMatchObject({ + email: "person@example.com", + }); + expect(withDb).toHaveBeenCalledTimes(2); + expect(recordOperationalFailure).not.toHaveBeenCalled(); + }); + + it("returns null and records one failure when both reads fail", async () => { + const error = new Error("Internal error."); + vi.mocked(withDb).mockRejectedValue(error); + + await expect(getAutumnCustomerFields("user_1")).resolves.toBeNull(); + expect(withDb).toHaveBeenCalledTimes(2); + expect(recordOperationalFailure).toHaveBeenCalledTimes(1); + expect(recordOperationalFailure).toHaveBeenCalledWith({ + distinctId: "user_1", + error, + event: "autumn_customer_fields", + }); + }); +}); + +describe("trackAutumnUsage", () => { + it("skips get_or_create but still tracks usage when the identity read fails", async () => { + vi.mocked(withDb).mockRejectedValue(new Error("Internal error.")); + + await trackAutumnUsage({ + env: { AUTUMN_SECRET_KEY: "am_sk_test" } as never, + event: "workspace_ai_usage", + featureId: "messages", + properties: {}, + userId: "user_1", + }); + + expect(getOrCreateAutumnCustomer).not.toHaveBeenCalled(); + expect(trackAutumnBalance).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/integrations/autumn/client.server.ts b/src/integrations/autumn/client.server.ts index 85a150944..434ce1851 100644 --- a/src/integrations/autumn/client.server.ts +++ b/src/integrations/autumn/client.server.ts @@ -1,7 +1,7 @@ import { eq } from "drizzle-orm"; import { user } from "#/db/schema"; -import { createDbContext } from "#/db/server"; +import { withDb } from "#/db/server"; import { getOrCreateAutumnCustomer, trackAutumnBalance } from "#/integrations/autumn/rest"; import { resolveAutumnSecretKey } from "#/integrations/autumn/secret-key"; import { recordOperationalFailure } from "#/integrations/observability/operational-events"; @@ -23,50 +23,63 @@ const DEFAULT_AUTUMN_CUSTOMER_FIELDS = { }, } as const satisfies AutumnCustomerFields; -export async function getAutumnCustomerFields(userId: string): Promise { - let dbContext: Awaited> | undefined; +// One retry: the read is a single indexed lookup and the failure seen here is a +// transient Postgres internal error, so a fresh connection usually succeeds. +const MAX_READ_ATTEMPTS = 2; - try { - dbContext = await createDbContext(); - - const [row] = await dbContext.db - .select({ - createdAt: user.createdAt, - email: user.email, - emailVerified: user.emailVerified, - isAnonymous: user.isAnonymous, - name: user.name, - }) - .from(user) - .where(eq(user.id, userId)) - .limit(1); - - if (!row) { - return DEFAULT_AUTUMN_CUSTOMER_FIELDS; +/** + * Null means the read failed and the caller must skip the Autumn write: a + * get_or_create with empty identity fields overwrites the stored name, email, and + * account type. Defaults are returned only when the user genuinely has no row. + */ +export async function getAutumnCustomerFields( + userId: string, +): Promise { + let lastError: unknown; + + for (let attempt = 0; attempt < MAX_READ_ATTEMPTS; attempt += 1) { + try { + return await withDb(async (db) => { + const [row] = await db + .select({ + createdAt: user.createdAt, + email: user.email, + emailVerified: user.emailVerified, + isAnonymous: user.isAnonymous, + name: user.name, + }) + .from(user) + .where(eq(user.id, userId)) + .limit(1); + + if (!row) { + return DEFAULT_AUTUMN_CUSTOMER_FIELDS; + } + + const isAnonymous = Boolean(row.isAnonymous); + + return { + ...(isAnonymous ? {} : getNamedCustomerFields(row)), + metadata: { + ...DEFAULT_AUTUMN_CUSTOMER_FIELDS.metadata, + account_type: isAnonymous ? "anonymous" : "registered", + email_verified: row.emailVerified, + user_created_at: row.createdAt.toISOString(), + }, + }; + }); + } catch (error) { + lastError = error; } + } - const isAnonymous = Boolean(row.isAnonymous); - - return { - ...(isAnonymous ? {} : getNamedCustomerFields(row)), - metadata: { - ...DEFAULT_AUTUMN_CUSTOMER_FIELDS.metadata, - account_type: isAnonymous ? "anonymous" : "registered", - email_verified: row.emailVerified, - user_created_at: row.createdAt.toISOString(), - }, - }; - } catch (error) { - recordOperationalFailure({ - distinctId: userId, - error, - event: "autumn_customer_fields", - }); + recordOperationalFailure({ + distinctId: userId, + error: lastError, + event: "autumn_customer_fields", + }); - return DEFAULT_AUTUMN_CUSTOMER_FIELDS; - } finally { - await dbContext?.dispose(); - } + return null; } function getNamedCustomerFields(row: { email: string; name: string }) { @@ -103,7 +116,12 @@ export async function trackAutumnUsage(input: TrackAutumnUsageInput) { try { const customerFields = await getAutumnCustomerFields(input.userId); - await getOrCreateAutumnCustomer({ customerId: input.userId, secretKey, ...customerFields }); + // Null means the read failed (see getAutumnCustomerFields). Skip the + // identity write, but still track usage: the customer already exists for + // anyone metered here, and the meter carries no identity to corrupt. + if (customerFields) { + await getOrCreateAutumnCustomer({ customerId: input.userId, secretKey, ...customerFields }); + } await trackAutumnBalance({ customerId: input.userId, diff --git a/src/integrations/observability/operational-error.test.ts b/src/integrations/observability/operational-error.test.ts index 70042f715..d513930d1 100644 --- a/src/integrations/observability/operational-error.test.ts +++ b/src/integrations/observability/operational-error.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from "vitest"; -import { buildOperationalErrorFields } from "#/integrations/observability/operational-error"; +import { + buildOperationalErrorFields, + normalizeCapturedError, +} from "#/integrations/observability/operational-error"; describe("operational error fields", () => { it("preserves structured Durable Object error details", () => { @@ -19,6 +22,35 @@ describe("operational error fields", () => { }); }); + it("drops bound parameters from a drizzle query error before capture", () => { + const cause = Object.assign(new Error("Internal error."), { name: "DatabaseError" }); + const drizzleError = Object.assign( + new Error('Failed query: select "id" from "user" where "id" = $1\nparams: user_123'), + { + cause, + params: ["user_123"], + query: 'select "id" from "user" where "id" = $1', + stack: "Error: Failed query: ...\nparams: user_123\n at read (client.server.ts:1:1)", + }, + ); + + const normalized = normalizeCapturedError(drizzleError); + + expect(buildOperationalErrorFields(normalized)).toMatchObject({ + error_cause_message: "Internal error.", + error_cause_type: "DatabaseError", + error_message: 'select "id" from "user" where "id" = $1', + error_type: "DrizzleQueryError", + }); + expect(buildOperationalErrorFields(normalized).error_stack).not.toContain("user_123"); + }); + + it("passes non-drizzle errors through unchanged", () => { + const error = new Error("plain failure"); + + expect(normalizeCapturedError(error)).toBe(error); + }); + it("bounds error messages before logging them", () => { const fields = buildOperationalErrorFields(new Error("x".repeat(2_000))); diff --git a/src/integrations/observability/operational-error.ts b/src/integrations/observability/operational-error.ts index 92d091325..0692e357b 100644 --- a/src/integrations/observability/operational-error.ts +++ b/src/integrations/observability/operational-error.ts @@ -30,6 +30,46 @@ export function buildOperationalErrorFields(error: unknown): OperationalErrorFie }; } +/** + * A drizzle `DrizzleQueryError` builds its message from the SQL statement and the + * bound parameters. Those parameters carry request data — a user id among them — + * so every caller produces a distinct message and error tracking splinters a + * single fault into one issue per parameter value. Rebuild it from the statement + * alone, keep the driver error as the cause so the real reason survives, and keep + * the original frames so the origin still resolves. Other errors pass through. + */ +export function normalizeCapturedError(error: unknown): unknown { + if (!isErrorRecord(error)) { + return error; + } + + const statement = getString(error.query); + + if (!statement || !Array.isArray(error.params)) { + return error; + } + + const normalized = new Error(statement, { cause: error.cause }); + normalized.name = "DrizzleQueryError"; + normalized.stack = buildStatementStack(statement, getString(error.stack)); + + return normalized; +} + +function buildStatementStack(statement: string, originalStack: string | undefined) { + const header = `DrizzleQueryError: ${statement}`; + + if (!originalStack) { + return header; + } + + // Drop the original message header (it holds the parameters) but keep the + // stack frames beneath it. + const framesIndex = originalStack.search(/\n\s+at /); + + return framesIndex === -1 ? header : `${header}${originalStack.slice(framesIndex)}`; +} + function getErrorMessage(error: unknown) { if (error instanceof Error) { return error.message; diff --git a/src/integrations/observability/operational-events.ts b/src/integrations/observability/operational-events.ts index be2ac85c6..25eeb9b27 100644 --- a/src/integrations/observability/operational-events.ts +++ b/src/integrations/observability/operational-events.ts @@ -6,7 +6,10 @@ import { type TelemetryRequestDetails, } from "#/integrations/posthog/server-context"; import type { PostHogTelemetryScheduler } from "#/integrations/posthog/scheduler"; -import { buildOperationalErrorFields } from "#/integrations/observability/operational-error"; +import { + buildOperationalErrorFields, + normalizeCapturedError, +} from "#/integrations/observability/operational-error"; type OperationalEventValue = boolean | null | number | string | readonly string[] | undefined; type OperationalEventFields = Record; @@ -49,11 +52,12 @@ export function logOperationalEvent(input: OperationalEventInput) { } export function recordOperationalFailure(input: OperationalFailureInput) { + const error = normalizeCapturedError(input.error); const requestContext = input.requestContext ?? (input.request ? getTelemetryRequestContext(input.request) : getTelemetryRuntimeContext()); logOperationalEvent({ - error: input.error, + error, event: input.event, fields: input.fields, outcome: "error", @@ -62,7 +66,7 @@ export function recordOperationalFailure(input: OperationalFailureInput) { capturePostHogServerException({ distinctId: input.distinctId, - error: input.error, + error, properties: { operation: input.event, ...input.fields,