mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
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
This commit is contained in:
@@ -436,6 +436,8 @@ export function createToolRoute<T>(app: FastifyInstance, config: ToolRouteConfig
|
|||||||
duration_ms: Date.now() - startTime,
|
duration_ms: Date.now() - startTime,
|
||||||
category: TOOLS.find((t) => t.id === config.toolId)?.category ?? "unknown",
|
category: TOOLS.find((t) => t.id === config.toolId)?.category ?? "unknown",
|
||||||
is_ai_tool: getBundleForTool(config.toolId) !== null,
|
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({
|
return reply.status(422).send({
|
||||||
error: "Processing failed",
|
error: "Processing failed",
|
||||||
|
|||||||
@@ -5,13 +5,19 @@ import { z } from "zod";
|
|||||||
import { resolveOutputFormat } from "../../lib/output-format.js";
|
import { resolveOutputFormat } from "../../lib/output-format.js";
|
||||||
import { createToolRoute } from "../tool-factory.js";
|
import { createToolRoute } from "../tool-factory.js";
|
||||||
|
|
||||||
const settingsSchema = z.object({
|
const MAX_DIMENSION = 16383;
|
||||||
width: z.number().positive().optional(),
|
|
||||||
height: z.number().positive().optional(),
|
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"),
|
fit: z.enum(["contain", "cover", "fill", "inside", "outside"]).default("contain"),
|
||||||
withoutEnlargement: z.boolean().default(false),
|
withoutEnlargement: z.boolean().default(false),
|
||||||
percentage: z.number().positive().optional(),
|
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) {
|
export function registerResize(app: FastifyInstance) {
|
||||||
createToolRoute(app, {
|
createToolRoute(app, {
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ import { Toaster } from "sonner";
|
|||||||
import { ConnectionMonitor } from "./components/common/connection-monitor";
|
import { ConnectionMonitor } from "./components/common/connection-monitor";
|
||||||
import { KeyboardShortcutProvider } from "./components/common/keyboard-shortcut-provider";
|
import { KeyboardShortcutProvider } from "./components/common/keyboard-shortcut-provider";
|
||||||
import { useAuth } from "./hooks/use-auth";
|
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";
|
import { useAnalyticsStore } from "./stores/analytics-store";
|
||||||
|
|
||||||
// Lazy-load all pages so each page's JS (and its icons/deps) is only
|
// Lazy-load all pages so each page's JS (and its icons/deps) is only
|
||||||
@@ -190,11 +190,13 @@ export function App() {
|
|||||||
)
|
)
|
||||||
return;
|
return;
|
||||||
void (async () => {
|
void (async () => {
|
||||||
|
setAnalyticsConsent(true);
|
||||||
await initAnalytics(analyticsConfig);
|
await initAnalytics(analyticsConfig);
|
||||||
identify(analyticsConfig.instanceId, {
|
identify(
|
||||||
$set: { version: APP_VERSION },
|
analyticsConfig.instanceId,
|
||||||
$set_once: { instance_id: analyticsConfig.instanceId },
|
{ version: APP_VERSION },
|
||||||
});
|
{ instance_id: analyticsConfig.instanceId },
|
||||||
|
);
|
||||||
})();
|
})();
|
||||||
}, [analyticsConfigLoaded, analyticsConfig, analyticsConsent.analyticsEnabled]);
|
}, [analyticsConfigLoaded, analyticsConfig, analyticsConsent.analyticsEnabled]);
|
||||||
|
|
||||||
|
|||||||
@@ -15,13 +15,10 @@ function scrubString(str: string): string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export async function initAnalytics(config: AnalyticsConfig): Promise<void> {
|
export async function initAnalytics(config: AnalyticsConfig): Promise<void> {
|
||||||
if (initialized || !config.enabled) return;
|
if (initialized || !config.enabled || !consentGranted) return;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const posthogJs = (await import("posthog-js")).default;
|
const posthogJs = (await import("posthog-js")).default;
|
||||||
if (!consentGranted) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
posthog =
|
posthog =
|
||||||
posthogJs.init(config.posthogApiKey, {
|
posthogJs.init(config.posthogApiKey, {
|
||||||
api_host: config.posthogHost,
|
api_host: config.posthogHost,
|
||||||
@@ -45,9 +42,6 @@ export async function initAnalytics(config: AnalyticsConfig): Promise<void> {
|
|||||||
try {
|
try {
|
||||||
if (config.sentryDsn) {
|
if (config.sentryDsn) {
|
||||||
const Sentry = await import("@sentry/react");
|
const Sentry = await import("@sentry/react");
|
||||||
if (!consentGranted) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
Sentry.init({
|
Sentry.init({
|
||||||
dsn: config.sentryDsn,
|
dsn: config.sentryDsn,
|
||||||
sendDefaultPii: false,
|
sendDefaultPii: false,
|
||||||
@@ -110,10 +104,14 @@ export function setAnalyticsConsent(enabled: boolean): void {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
export function identify(instanceId: string, properties: Record<string, unknown>): void {
|
export function identify(
|
||||||
|
instanceId: string,
|
||||||
|
properties: Record<string, unknown>,
|
||||||
|
propertiesSetOnce?: Record<string, unknown>,
|
||||||
|
): void {
|
||||||
if (!posthog || !consentGranted) return;
|
if (!posthog || !consentGranted) return;
|
||||||
try {
|
try {
|
||||||
posthog.identify(instanceId, properties);
|
posthog.identify(instanceId, properties, propertiesSetOnce);
|
||||||
} catch {
|
} catch {
|
||||||
// never throw
|
// never throw
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,10 +8,11 @@ export async function resize(image: Sharp, options: ResizeOptions): Promise<Shar
|
|||||||
throw new Error("Resize percentage must be greater than 0");
|
throw new Error("Resize percentage must be greater than 0");
|
||||||
}
|
}
|
||||||
const metadata = await image.metadata();
|
const metadata = await image.metadata();
|
||||||
const currentWidth = metadata.width ?? 0;
|
if (!metadata.width || !metadata.height) {
|
||||||
const currentHeight = metadata.height ?? 0;
|
throw new Error("Cannot determine image dimensions for percentage resize");
|
||||||
width = Math.round(currentWidth * (percentage / 100));
|
}
|
||||||
height = Math.round(currentHeight * (percentage / 100));
|
width = Math.max(1, Math.round(metadata.width * (percentage / 100)));
|
||||||
|
height = Math.max(1, Math.round(metadata.height * (percentage / 100)));
|
||||||
}
|
}
|
||||||
|
|
||||||
if (width !== undefined && width <= 0) {
|
if (width !== undefined && width <= 0) {
|
||||||
@@ -26,10 +27,11 @@ export async function resize(image: Sharp, options: ResizeOptions): Promise<Shar
|
|||||||
|
|
||||||
if (withoutEnlargement) {
|
if (withoutEnlargement) {
|
||||||
const meta = await image.metadata();
|
const meta = await image.metadata();
|
||||||
const curW = meta.width ?? 0;
|
if (!meta.width || !meta.height) {
|
||||||
const curH = meta.height ?? 0;
|
throw new Error("Cannot determine image dimensions for resize clamping");
|
||||||
if (width !== undefined && width > curW) width = curW;
|
}
|
||||||
if (height !== undefined && height > curH) height = curH;
|
if (width !== undefined && width > meta.width) width = meta.width;
|
||||||
|
if (height !== undefined && height > meta.height) height = meta.height;
|
||||||
}
|
}
|
||||||
|
|
||||||
return image.resize({
|
return image.resize({
|
||||||
|
|||||||
@@ -14,6 +14,8 @@ export interface ToolUsedProperties {
|
|||||||
category: string;
|
category: string;
|
||||||
is_ai_tool: boolean;
|
is_ai_tool: boolean;
|
||||||
params?: Record<string, string | number | boolean>;
|
params?: Record<string, string | number | boolean>;
|
||||||
|
error_code?: string;
|
||||||
|
error_message?: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface SearchProperties {
|
export interface SearchProperties {
|
||||||
|
|||||||
@@ -851,7 +851,7 @@ describe("Tool processing", () => {
|
|||||||
expect(JSON.parse(res.body).error).toMatch(/json/i);
|
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([
|
const { body: payload, contentType } = createMultipartPayload([
|
||||||
{ name: "file", filename: "defaults.png", contentType: "image/png", content: PNG_200x150 },
|
{ name: "file", filename: "defaults.png", contentType: "image/png", content: PNG_200x150 },
|
||||||
{ name: "settings", content: "{}" },
|
{ name: "settings", content: "{}" },
|
||||||
@@ -866,9 +866,7 @@ describe("Tool processing", () => {
|
|||||||
},
|
},
|
||||||
payload,
|
payload,
|
||||||
});
|
});
|
||||||
// All resize fields are optional at the Zod level, but Sharp needs at
|
expect(res.statusCode).toBe(400);
|
||||||
// least width or height — so processing fails with 422
|
|
||||||
expect(res.statusCode).toBe(422);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it("download URL from resize result is accessible", async () => {
|
it("download URL from resize result is accessible", async () => {
|
||||||
|
|||||||
@@ -229,13 +229,12 @@ describe("resize", () => {
|
|||||||
expect(meta.width).toBeGreaterThanOrEqual(0);
|
expect(meta.width).toBeGreaterThanOrEqual(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("percentage=1 on 1x1 yields width rounded to 0 and triggers error", async () => {
|
it("percentage=1 on 1x1 clamps to 1px minimum", async () => {
|
||||||
// 1 * 1/100 = 0.01 -> Math.round -> 0
|
|
||||||
// Then the check width <= 0 should throw
|
|
||||||
const img = sharp(png1x1);
|
const img = sharp(png1x1);
|
||||||
await expect(resize(img, { percentage: 1 })).rejects.toThrow(
|
const result = await resize(img, { percentage: 1 });
|
||||||
"Resize width must be greater than 0",
|
const meta = await result.toBuffer().then((b) => sharp(b).metadata());
|
||||||
);
|
expect(meta.width).toBe(1);
|
||||||
|
expect(meta.height).toBe(1);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -319,7 +319,7 @@ describe("analytics lib", () => {
|
|||||||
setAnalyticsConsent(true);
|
setAnalyticsConsent(true);
|
||||||
await initAnalytics(enabledConfig);
|
await initAnalytics(enabledConfig);
|
||||||
identify("inst-1", { version: "1.0" });
|
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", () => {
|
it("does not throw before initialization", () => {
|
||||||
@@ -382,7 +382,7 @@ describe("analytics lib", () => {
|
|||||||
track("phase1_event");
|
track("phase1_event");
|
||||||
expect(mockCapture).toHaveBeenCalledWith("phase1_event", undefined);
|
expect(mockCapture).toHaveBeenCalledWith("phase1_event", undefined);
|
||||||
identify("inst-1", { phase: 1 });
|
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
|
// Phase 2: Revoke consent mid-session
|
||||||
setAnalyticsConsent(false);
|
setAnalyticsConsent(false);
|
||||||
|
|||||||
Reference in New Issue
Block a user