From 0311e72a9693fc466805694a2b0ac358b69d98e9 Mon Sep 17 00:00:00 2001 From: Ben Senescu <44480372+bensenescu@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:23:03 -0400 Subject: [PATCH] PostHog reports every server error as affecting as many users as it has events (#534) --- src/middleware/errorHandling.ts | 39 +++++++++++++++++++---- src/server/billing/autumn-webhook.ts | 14 +++++--- src/server/gdpr/storage-erasure.ts | 2 ++ src/server/lib/posthog.ts | 9 +++++- src/server/mcp/instrumentation.test.ts | 1 + src/server/mcp/instrumentation.ts | 13 +++++--- src/server/workflows/SiteAuditWorkflow.ts | 16 ++++++---- 7 files changed, 73 insertions(+), 21 deletions(-) diff --git a/src/middleware/errorHandling.ts b/src/middleware/errorHandling.ts index c4ed8d2..83cf2e4 100644 --- a/src/middleware/errorHandling.ts +++ b/src/middleware/errorHandling.ts @@ -2,8 +2,32 @@ import { createMiddleware } from "@tanstack/react-start"; import { getRequest } from "@tanstack/react-start/server"; import { waitUntil } from "cloudflare:workers"; import { shouldCaptureAppErrorCode } from "@/shared/error-codes"; +import { getAuth } from "@/lib/auth"; import { AppError, asAppError, toClientError } from "@/server/lib/errors"; import { captureServerError } from "@/server/lib/posthog"; +import { isHostedServerAuthMode } from "@/server/lib/runtime-env"; + +// This middleware wraps ensureUserMiddleware, so the resolved user context isn't +// in scope when a downstream handler throws. Re-read the session instead — but +// not via resolveHostedContext, which can create an organization as a side +// effect. disableRefresh keeps this a pure read: it runs inside waitUntil, where +// a refreshed Set-Cookie is discarded anyway. Any throw here would drop the whole +// exception report, so every failure degrades to an anonymous capture. +async function resolveErrorDistinctId( + headers: Headers, +): Promise { + if (!(await isHostedServerAuthMode())) return undefined; + try { + const session = await getAuth().api.getSession({ + headers, + query: { disableRefresh: true }, + }); + if (session?.user?.analyticsOptedOut === true) return undefined; + return session?.user?.id; + } catch { + return undefined; + } +} // TanStack's serverFn validator throws a plain Error whose message is the // JSON-serialized standard-schema issue list. Treat those as input validation, @@ -46,13 +70,16 @@ export const errorHandlingMiddleware = createMiddleware({ const url = new URL(request.url); console.error("server.function error:", error); + const properties = { + errorCode: appError?.code ?? "INTERNAL_ERROR", + method: request.method, + path: url.pathname, + ...appError?.details, + }; waitUntil( - captureServerError(error, { - errorCode: appError?.code ?? "INTERNAL_ERROR", - method: request.method, - path: url.pathname, - ...appError?.details, - }), + resolveErrorDistinctId(request.headers).then((distinctId) => + captureServerError(error, properties, distinctId), + ), ); } diff --git a/src/server/billing/autumn-webhook.ts b/src/server/billing/autumn-webhook.ts index 432487e..dd8b09d 100644 --- a/src/server/billing/autumn-webhook.ts +++ b/src/server/billing/autumn-webhook.ts @@ -62,10 +62,16 @@ export async function handleAutumnWebhookRequest(request: Request) { console.error("Autumn billing.updated sync failed", customerId, error, { cause: error instanceof Error ? error.cause : undefined, }); - await captureServerError(error, { - source: "autumn_webhook", - customer_id: customerId, - }); + // Webhooks carry no user; the Autumn customer id is the organization id, + // which at least makes "users affected" count organizations instead of events. + await captureServerError( + error, + { + source: "autumn_webhook", + customer_id: customerId, + }, + customerId, + ); return json({ error: "Webhook processing failed" }, 500); } diff --git a/src/server/gdpr/storage-erasure.ts b/src/server/gdpr/storage-erasure.ts index 336cb9d..cad9cc0 100644 --- a/src/server/gdpr/storage-erasure.ts +++ b/src/server/gdpr/storage-erasure.ts @@ -303,6 +303,8 @@ export async function handleGdprStorageErasure( // Raw fetch handlers run outside the server-function middleware, so // nothing else reports failures here. console.error("gdpr.storage-erasure failed:", error); + // Deliberately anonymous: this request erases the user, so keying the + // exception to their distinct id would recreate the person profile. await captureServerError(error, { source: "gdpr_storage_erasure" }); return Response.json({ error: "Erasure failed" }, { status: 500 }); } diff --git a/src/server/lib/posthog.ts b/src/server/lib/posthog.ts index 995c914..c73141a 100644 --- a/src/server/lib/posthog.ts +++ b/src/server/lib/posthog.ts @@ -16,9 +16,16 @@ function getServerPostHogClient(): PostHog | null { }); } +/** + * `distinctId` must be the better-auth user id, the same identity + * identifyAnalyticsUser and captureServerEvent send, so exceptions land on the + * existing person profile. Omitting it makes posthog-node mint a fresh anonymous + * id per event, so every error reports as many users affected as it has events. + */ export async function captureServerError( error: unknown, properties: Record = {}, + distinctId?: string, ) { if (!(await isHostedServerAuthMode())) { return; @@ -28,7 +35,7 @@ export async function captureServerError( if (!client) return; try { - await client.captureExceptionImmediate(error, undefined, { + await client.captureExceptionImmediate(error, distinctId, { source: "server", ...properties, }); diff --git a/src/server/mcp/instrumentation.test.ts b/src/server/mcp/instrumentation.test.ts index 5a2c392..5191134 100644 --- a/src/server/mcp/instrumentation.test.ts +++ b/src/server/mcp/instrumentation.test.ts @@ -95,6 +95,7 @@ describe("instrumentMcpToolHandler", () => { await expect(wrapped({}, toolContext)).rejects.toThrow("upstream exploded"); expect(mocks.captureServerError).toHaveBeenCalledTimes(1); expect(mocks.captureServerError.mock.calls[0][0]).toBe(boom); + expect(mocks.captureServerError.mock.calls[0][2]).toBe("user-1"); }); it("rethrows expected errors without reporting them", async () => { diff --git a/src/server/mcp/instrumentation.ts b/src/server/mcp/instrumentation.ts index e6a2076..d4745ff 100644 --- a/src/server/mcp/instrumentation.ts +++ b/src/server/mcp/instrumentation.ts @@ -106,6 +106,7 @@ export function instrumentMcpToolHandler( tool: toolName, issues: formatValidationIssues(validation.error), }, + context.auth.userId, ), ); } @@ -183,10 +184,14 @@ export function instrumentMcpToolHandler( if (shouldCaptureAppErrorCode(appError?.code)) { console.error(`mcp.tool error (${toolName}):`, error); waitUntil( - captureServerError(error, { - errorCode: appError?.code ?? "INTERNAL_ERROR", - tool: toolName, - }), + captureServerError( + error, + { + errorCode: appError?.code ?? "INTERNAL_ERROR", + tool: toolName, + }, + context.auth.userId, + ), ); } throw error; diff --git a/src/server/workflows/SiteAuditWorkflow.ts b/src/server/workflows/SiteAuditWorkflow.ts index 35b1ef1..49e05fe 100644 --- a/src/server/workflows/SiteAuditWorkflow.ts +++ b/src/server/workflows/SiteAuditWorkflow.ts @@ -81,12 +81,16 @@ export class SiteAuditWorkflow extends WorkflowEntrypoint { "Durable Object reset because its code was updated", ); if (!isDeployReset) { - await captureServerError(error, { - source: "site_audit_workflow", - audit_id: auditId, - organization_id: billingCustomer.organizationId, - project_id: projectId, - }); + await captureServerError( + error, + { + source: "site_audit_workflow", + audit_id: auditId, + organization_id: billingCustomer.organizationId, + project_id: projectId, + }, + billingCustomer.userId, + ); } const errorInfo = classifyAuditError(error); await pgStep(step, "mark-failed", DB_STEP, async () => {