From 979a833978c35252efb8eff20e3af916a37e9a02 Mon Sep 17 00:00:00 2001 From: SnapOtter Date: Wed, 6 May 2026 23:21:45 +0800 Subject: [PATCH] fix: resolve analytics data gaps and resize validation failures - Fix resize 20% failure rate: add Zod refine requiring at least one dimension, enforce integer/max constraints, clamp percentage scaling to minimum 1px, and guard against missing metadata in withoutEnlargement - Fix PostHog init race condition: move consent check before async import so frontend events (search, pageview) are no longer silently dropped - Fix identify() passing nested $set/$set_once wrappers instead of flat properties, so version person property now appears on PostHog profiles - Add error_code and error_message to failed tool_used analytics events for debugging tool failures from PostHog --- apps/api/src/routes/tool-factory.ts | 2 ++ apps/api/src/routes/tools/resize.ts | 20 ++++++++++++------- apps/web/src/App.tsx | 12 ++++++----- apps/web/src/lib/analytics.ts | 16 +++++++-------- .../image-engine/src/operations/resize.ts | 18 +++++++++-------- packages/shared/src/analytics/events.ts | 2 ++ tests/integration/api.test.ts | 6 ++---- tests/unit/image-engine/operations.test.ts | 11 +++++----- tests/unit/web/analytics.test.ts | 4 ++-- 9 files changed, 50 insertions(+), 41 deletions(-) diff --git a/apps/api/src/routes/tool-factory.ts b/apps/api/src/routes/tool-factory.ts index 87055957..3abcd5d1 100644 --- a/apps/api/src/routes/tool-factory.ts +++ b/apps/api/src/routes/tool-factory.ts @@ -436,6 +436,8 @@ export function createToolRoute(app: FastifyInstance, config: ToolRouteConfig duration_ms: Date.now() - startTime, category: TOOLS.find((t) => t.id === config.toolId)?.category ?? "unknown", is_ai_tool: getBundleForTool(config.toolId) !== null, + error_code: err instanceof Error ? err.constructor.name : "UnknownError", + error_message: message.slice(0, 200), }); return reply.status(422).send({ error: "Processing failed", diff --git a/apps/api/src/routes/tools/resize.ts b/apps/api/src/routes/tools/resize.ts index 2d12f315..60bf0b38 100644 --- a/apps/api/src/routes/tools/resize.ts +++ b/apps/api/src/routes/tools/resize.ts @@ -5,13 +5,19 @@ import { z } from "zod"; import { resolveOutputFormat } from "../../lib/output-format.js"; import { createToolRoute } from "../tool-factory.js"; -const settingsSchema = z.object({ - width: z.number().positive().optional(), - height: z.number().positive().optional(), - fit: z.enum(["contain", "cover", "fill", "inside", "outside"]).default("contain"), - withoutEnlargement: z.boolean().default(false), - percentage: z.number().positive().optional(), -}); +const MAX_DIMENSION = 16383; + +const settingsSchema = z + .object({ + width: z.number().int().positive().max(MAX_DIMENSION).optional(), + height: z.number().int().positive().max(MAX_DIMENSION).optional(), + fit: z.enum(["contain", "cover", "fill", "inside", "outside"]).default("contain"), + withoutEnlargement: z.boolean().default(false), + percentage: z.number().positive().optional(), + }) + .refine((s) => s.width !== undefined || s.height !== undefined || s.percentage !== undefined, { + message: "At least one of width, height, or percentage is required", + }); export function registerResize(app: FastifyInstance) { createToolRoute(app, { diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index c3740656..cfc52b47 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -5,7 +5,7 @@ import { Toaster } from "sonner"; import { ConnectionMonitor } from "./components/common/connection-monitor"; import { KeyboardShortcutProvider } from "./components/common/keyboard-shortcut-provider"; import { useAuth } from "./hooks/use-auth"; -import { identify, initAnalytics } from "./lib/analytics"; +import { identify, initAnalytics, setAnalyticsConsent } from "./lib/analytics"; import { useAnalyticsStore } from "./stores/analytics-store"; // Lazy-load all pages so each page's JS (and its icons/deps) is only @@ -190,11 +190,13 @@ export function App() { ) return; void (async () => { + setAnalyticsConsent(true); await initAnalytics(analyticsConfig); - identify(analyticsConfig.instanceId, { - $set: { version: APP_VERSION }, - $set_once: { instance_id: analyticsConfig.instanceId }, - }); + identify( + analyticsConfig.instanceId, + { version: APP_VERSION }, + { instance_id: analyticsConfig.instanceId }, + ); })(); }, [analyticsConfigLoaded, analyticsConfig, analyticsConsent.analyticsEnabled]); diff --git a/apps/web/src/lib/analytics.ts b/apps/web/src/lib/analytics.ts index cd92bab1..4545884c 100644 --- a/apps/web/src/lib/analytics.ts +++ b/apps/web/src/lib/analytics.ts @@ -15,13 +15,10 @@ function scrubString(str: string): string { } export async function initAnalytics(config: AnalyticsConfig): Promise { - if (initialized || !config.enabled) return; + if (initialized || !config.enabled || !consentGranted) return; try { const posthogJs = (await import("posthog-js")).default; - if (!consentGranted) { - return; - } posthog = posthogJs.init(config.posthogApiKey, { api_host: config.posthogHost, @@ -45,9 +42,6 @@ export async function initAnalytics(config: AnalyticsConfig): Promise { try { if (config.sentryDsn) { const Sentry = await import("@sentry/react"); - if (!consentGranted) { - return; - } Sentry.init({ dsn: config.sentryDsn, sendDefaultPii: false, @@ -110,10 +104,14 @@ export function setAnalyticsConsent(enabled: boolean): void { } } -export function identify(instanceId: string, properties: Record): void { +export function identify( + instanceId: string, + properties: Record, + propertiesSetOnce?: Record, +): void { if (!posthog || !consentGranted) return; try { - posthog.identify(instanceId, properties); + posthog.identify(instanceId, properties, propertiesSetOnce); } catch { // never throw } diff --git a/packages/image-engine/src/operations/resize.ts b/packages/image-engine/src/operations/resize.ts index 7b0d221d..4f2fe668 100644 --- a/packages/image-engine/src/operations/resize.ts +++ b/packages/image-engine/src/operations/resize.ts @@ -8,10 +8,11 @@ export async function resize(image: Sharp, options: ResizeOptions): Promise curW) width = curW; - if (height !== undefined && height > curH) height = curH; + if (!meta.width || !meta.height) { + throw new Error("Cannot determine image dimensions for resize clamping"); + } + if (width !== undefined && width > meta.width) width = meta.width; + if (height !== undefined && height > meta.height) height = meta.height; } return image.resize({ diff --git a/packages/shared/src/analytics/events.ts b/packages/shared/src/analytics/events.ts index 0dac225e..89f87248 100644 --- a/packages/shared/src/analytics/events.ts +++ b/packages/shared/src/analytics/events.ts @@ -14,6 +14,8 @@ export interface ToolUsedProperties { category: string; is_ai_tool: boolean; params?: Record; + error_code?: string; + error_message?: string; } export interface SearchProperties { diff --git a/tests/integration/api.test.ts b/tests/integration/api.test.ts index 4f12341a..0a81983b 100644 --- a/tests/integration/api.test.ts +++ b/tests/integration/api.test.ts @@ -851,7 +851,7 @@ describe("Tool processing", () => { expect(JSON.parse(res.body).error).toMatch(/json/i); }); - it("returns 422 with empty settings (no dimensions given)", async () => { + it("returns 400 with empty settings (no dimensions given)", async () => { const { body: payload, contentType } = createMultipartPayload([ { name: "file", filename: "defaults.png", contentType: "image/png", content: PNG_200x150 }, { name: "settings", content: "{}" }, @@ -866,9 +866,7 @@ describe("Tool processing", () => { }, payload, }); - // All resize fields are optional at the Zod level, but Sharp needs at - // least width or height — so processing fails with 422 - expect(res.statusCode).toBe(422); + expect(res.statusCode).toBe(400); }); it("download URL from resize result is accessible", async () => { diff --git a/tests/unit/image-engine/operations.test.ts b/tests/unit/image-engine/operations.test.ts index 6a076bd3..84762ac7 100644 --- a/tests/unit/image-engine/operations.test.ts +++ b/tests/unit/image-engine/operations.test.ts @@ -229,13 +229,12 @@ describe("resize", () => { expect(meta.width).toBeGreaterThanOrEqual(0); }); - it("percentage=1 on 1x1 yields width rounded to 0 and triggers error", async () => { - // 1 * 1/100 = 0.01 -> Math.round -> 0 - // Then the check width <= 0 should throw + it("percentage=1 on 1x1 clamps to 1px minimum", async () => { const img = sharp(png1x1); - await expect(resize(img, { percentage: 1 })).rejects.toThrow( - "Resize width must be greater than 0", - ); + const result = await resize(img, { percentage: 1 }); + const meta = await result.toBuffer().then((b) => sharp(b).metadata()); + expect(meta.width).toBe(1); + expect(meta.height).toBe(1); }); }); diff --git a/tests/unit/web/analytics.test.ts b/tests/unit/web/analytics.test.ts index 6a500f88..db410297 100644 --- a/tests/unit/web/analytics.test.ts +++ b/tests/unit/web/analytics.test.ts @@ -319,7 +319,7 @@ describe("analytics lib", () => { setAnalyticsConsent(true); await initAnalytics(enabledConfig); identify("inst-1", { version: "1.0" }); - expect(mockIdentify).toHaveBeenCalledWith("inst-1", { version: "1.0" }); + expect(mockIdentify).toHaveBeenCalledWith("inst-1", { version: "1.0" }, undefined); }); it("does not throw before initialization", () => { @@ -382,7 +382,7 @@ describe("analytics lib", () => { track("phase1_event"); expect(mockCapture).toHaveBeenCalledWith("phase1_event", undefined); identify("inst-1", { phase: 1 }); - expect(mockIdentify).toHaveBeenCalledWith("inst-1", { phase: 1 }); + expect(mockIdentify).toHaveBeenCalledWith("inst-1", { phase: 1 }, undefined); // Phase 2: Revoke consent mid-session setAnalyticsConsent(false);