PostHog reports every server error as affecting as many users as it has events (#534)

This commit is contained in:
Ben Senescu 2026-08-26 10:23:03 -04:00 committed by GitHub
parent 8752269149
commit 0311e72a96
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 73 additions and 21 deletions

View File

@ -2,8 +2,32 @@ import { createMiddleware } from "@tanstack/react-start";
import { getRequest } from "@tanstack/react-start/server"; import { getRequest } from "@tanstack/react-start/server";
import { waitUntil } from "cloudflare:workers"; import { waitUntil } from "cloudflare:workers";
import { shouldCaptureAppErrorCode } from "@/shared/error-codes"; import { shouldCaptureAppErrorCode } from "@/shared/error-codes";
import { getAuth } from "@/lib/auth";
import { AppError, asAppError, toClientError } from "@/server/lib/errors"; import { AppError, asAppError, toClientError } from "@/server/lib/errors";
import { captureServerError } from "@/server/lib/posthog"; 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<string | undefined> {
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 // TanStack's serverFn validator throws a plain Error whose message is the
// JSON-serialized standard-schema issue list. Treat those as input validation, // 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); const url = new URL(request.url);
console.error("server.function error:", error); console.error("server.function error:", error);
const properties = {
errorCode: appError?.code ?? "INTERNAL_ERROR",
method: request.method,
path: url.pathname,
...appError?.details,
};
waitUntil( waitUntil(
captureServerError(error, { resolveErrorDistinctId(request.headers).then((distinctId) =>
errorCode: appError?.code ?? "INTERNAL_ERROR", captureServerError(error, properties, distinctId),
method: request.method, ),
path: url.pathname,
...appError?.details,
}),
); );
} }

View File

@ -62,10 +62,16 @@ export async function handleAutumnWebhookRequest(request: Request) {
console.error("Autumn billing.updated sync failed", customerId, error, { console.error("Autumn billing.updated sync failed", customerId, error, {
cause: error instanceof Error ? error.cause : undefined, cause: error instanceof Error ? error.cause : undefined,
}); });
await captureServerError(error, { // Webhooks carry no user; the Autumn customer id is the organization id,
source: "autumn_webhook", // which at least makes "users affected" count organizations instead of events.
customer_id: customerId, await captureServerError(
}); error,
{
source: "autumn_webhook",
customer_id: customerId,
},
customerId,
);
return json({ error: "Webhook processing failed" }, 500); return json({ error: "Webhook processing failed" }, 500);
} }

View File

@ -303,6 +303,8 @@ export async function handleGdprStorageErasure(
// Raw fetch handlers run outside the server-function middleware, so // Raw fetch handlers run outside the server-function middleware, so
// nothing else reports failures here. // nothing else reports failures here.
console.error("gdpr.storage-erasure failed:", error); 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" }); await captureServerError(error, { source: "gdpr_storage_erasure" });
return Response.json({ error: "Erasure failed" }, { status: 500 }); return Response.json({ error: "Erasure failed" }, { status: 500 });
} }

View File

@ -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( export async function captureServerError(
error: unknown, error: unknown,
properties: Record<string, string | null | undefined> = {}, properties: Record<string, string | null | undefined> = {},
distinctId?: string,
) { ) {
if (!(await isHostedServerAuthMode())) { if (!(await isHostedServerAuthMode())) {
return; return;
@ -28,7 +35,7 @@ export async function captureServerError(
if (!client) return; if (!client) return;
try { try {
await client.captureExceptionImmediate(error, undefined, { await client.captureExceptionImmediate(error, distinctId, {
source: "server", source: "server",
...properties, ...properties,
}); });

View File

@ -95,6 +95,7 @@ describe("instrumentMcpToolHandler", () => {
await expect(wrapped({}, toolContext)).rejects.toThrow("upstream exploded"); await expect(wrapped({}, toolContext)).rejects.toThrow("upstream exploded");
expect(mocks.captureServerError).toHaveBeenCalledTimes(1); expect(mocks.captureServerError).toHaveBeenCalledTimes(1);
expect(mocks.captureServerError.mock.calls[0][0]).toBe(boom); 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 () => { it("rethrows expected errors without reporting them", async () => {

View File

@ -106,6 +106,7 @@ export function instrumentMcpToolHandler<TArgs>(
tool: toolName, tool: toolName,
issues: formatValidationIssues(validation.error), issues: formatValidationIssues(validation.error),
}, },
context.auth.userId,
), ),
); );
} }
@ -183,10 +184,14 @@ export function instrumentMcpToolHandler<TArgs>(
if (shouldCaptureAppErrorCode(appError?.code)) { if (shouldCaptureAppErrorCode(appError?.code)) {
console.error(`mcp.tool error (${toolName}):`, error); console.error(`mcp.tool error (${toolName}):`, error);
waitUntil( waitUntil(
captureServerError(error, { captureServerError(
errorCode: appError?.code ?? "INTERNAL_ERROR", error,
tool: toolName, {
}), errorCode: appError?.code ?? "INTERNAL_ERROR",
tool: toolName,
},
context.auth.userId,
),
); );
} }
throw error; throw error;

View File

@ -81,12 +81,16 @@ export class SiteAuditWorkflow extends WorkflowEntrypoint<Env, AuditParams> {
"Durable Object reset because its code was updated", "Durable Object reset because its code was updated",
); );
if (!isDeployReset) { if (!isDeployReset) {
await captureServerError(error, { await captureServerError(
source: "site_audit_workflow", error,
audit_id: auditId, {
organization_id: billingCustomer.organizationId, source: "site_audit_workflow",
project_id: projectId, audit_id: auditId,
}); organization_id: billingCustomer.organizationId,
project_id: projectId,
},
billingCustomer.userId,
);
} }
const errorInfo = classifyAuditError(error); const errorInfo = classifyAuditError(error);
await pgStep(step, "mark-failed", DB_STEP, async () => { await pgStep(step, "mark-failed", DB_STEP, async () => {