From 66211ed6a72a6deec0beceabe1b92d05c1c29341 Mon Sep 17 00:00:00 2001 From: SnapOtter Date: Fri, 1 May 2026 19:01:05 +0800 Subject: [PATCH] fix: resolve 20 Sentry issues and fix navbar test flakiness Sentry fixes: - Only send 5xx errors to Sentry (was sending 4xx rate-limit, media type errors) - Encode non-ASCII chars in X-Output-Filename header (encodeURIComponent) - Handle FK constraint failures gracefully in file upload, pipeline save, API keys - Harden getDirSize against ENOENT race on readdirSync Test fixes: - Wrap navbar test renders in act() to flush async useEffect state updates - Add useEffect cleanup to navbar to prevent state updates on unmounted component - Fixes timeout when running in full test suite --- .gitignore | 1 + apps/api/src/index.ts | 4 +- apps/api/src/routes/api-keys.ts | 26 ++++--- apps/api/src/routes/features.ts | 9 ++- apps/api/src/routes/pipeline.ts | 22 +++--- apps/api/src/routes/tools/optimize-for-web.ts | 2 +- apps/api/src/routes/user-files.ts | 68 ++++++++-------- apps/landing/src/components/navbar.tsx | 6 +- tests/unit/landing/navbar.test.tsx | 77 +++++++++++-------- 9 files changed, 128 insertions(+), 87 deletions(-) diff --git a/.gitignore b/.gitignore index d22cd27f..3221afbe 100644 --- a/.gitignore +++ b/.gitignore @@ -57,3 +57,4 @@ audit_report.md .mcp.json tests/benchmark/bench-limits-results-mac.jsonl .superpowers/ +scripts/setup-cloudflare-email.sh diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index 762c118e..968e2600 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -100,7 +100,9 @@ app.setErrorHandler((error: Error & { statusCode?: number }, request, reply) => { err: error, url: request.url, method: request.method }, "Unhandled request error", ); - captureException(error, request); + if (statusCode >= 500) { + captureException(error, request); + } const isProduction = process.env.NODE_ENV === "production"; reply.status(statusCode).send({ error: statusCode >= 500 ? "Internal server error" : error.message, diff --git a/apps/api/src/routes/api-keys.ts b/apps/api/src/routes/api-keys.ts index 9cea1b25..7909cfec 100644 --- a/apps/api/src/routes/api-keys.ts +++ b/apps/api/src/routes/api-keys.ts @@ -72,17 +72,21 @@ export async function apiKeyRoutes(app: FastifyInstance): Promise { const keyPrefix = computeKeyPrefix(rawKey); const id = randomUUID(); - db.insert(schema.apiKeys) - .values({ - id, - userId: user.id, - keyHash, - keyPrefix, - name, - permissions: scopedPermissions ? JSON.stringify(scopedPermissions) : null, - expiresAt, - }) - .run(); + try { + db.insert(schema.apiKeys) + .values({ + id, + userId: user.id, + keyHash, + keyPrefix, + name, + permissions: scopedPermissions ? JSON.stringify(scopedPermissions) : null, + expiresAt, + }) + .run(); + } catch { + return reply.status(409).send({ error: "Failed to create API key" }); + } auditLog(request.log, "API_KEY_CREATED", { userId: user.id, keyId: id, keyName: name }); diff --git a/apps/api/src/routes/features.ts b/apps/api/src/routes/features.ts index 7bbfff29..cc718062 100644 --- a/apps/api/src/routes/features.ts +++ b/apps/api/src/routes/features.ts @@ -9,7 +9,7 @@ import { spawn } from "node:child_process"; import crypto from "node:crypto"; -import { existsSync, readdirSync, readFileSync, statSync, unlinkSync } from "node:fs"; +import { type Dirent, existsSync, readdirSync, readFileSync, statSync, unlinkSync } from "node:fs"; import { join } from "node:path"; import { shutdownDispatcher } from "@snapotter/ai"; import { ANALYTICS_EVENTS, FEATURE_BUNDLES } from "@snapotter/shared"; @@ -67,7 +67,12 @@ function getDirSize(dirPath: string): number { if (!existsSync(dirPath)) return 0; let total = 0; - const entries = readdirSync(dirPath, { withFileTypes: true }); + let entries: Dirent[]; + try { + entries = readdirSync(dirPath, { withFileTypes: true }); + } catch { + return 0; + } for (const entry of entries) { const fullPath = join(dirPath, entry.name); if (entry.isDirectory()) { diff --git a/apps/api/src/routes/pipeline.ts b/apps/api/src/routes/pipeline.ts index 4af6dea8..80b9f158 100644 --- a/apps/api/src/routes/pipeline.ts +++ b/apps/api/src/routes/pipeline.ts @@ -329,15 +329,19 @@ export async function registerPipelineRoutes(app: FastifyInstance): Promise { // Create DB record const id = randomUUID(); - db.insert(schema.userFiles) - .values({ - id, - userId, - originalName: safeName, - storedName, - mimeType, - size: safeBuffer.length, - width: validation.width, - height: validation.height, - version: 1, - parentId: null, - toolChain: null, - }) - .run(); + try { + db.insert(schema.userFiles) + .values({ + id, + userId, + originalName: safeName, + storedName, + mimeType, + size: safeBuffer.length, + width: validation.width, + height: validation.height, + version: 1, + parentId: null, + toolChain: null, + }) + .run(); + } catch { + return reply.status(409).send({ error: "Failed to save file record" }); + } const row = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get(); @@ -555,21 +559,25 @@ export async function userFileRoutes(app: FastifyInstance): Promise { // Create DB record const id = randomUUID(); - db.insert(schema.userFiles) - .values({ - id, - userId, - originalName: resultName, - storedName, - mimeType, - size: safeResultBuffer.length, - width: validation.width, - height: validation.height, - version: nextVersion, - parentId, - toolChain: JSON.stringify(newChain), - }) - .run(); + try { + db.insert(schema.userFiles) + .values({ + id, + userId, + originalName: resultName, + storedName, + mimeType, + size: safeResultBuffer.length, + width: validation.width, + height: validation.height, + version: nextVersion, + parentId, + toolChain: JSON.stringify(newChain), + }) + .run(); + } catch { + return reply.status(409).send({ error: "Failed to save result record" }); + } const row = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get(); diff --git a/apps/landing/src/components/navbar.tsx b/apps/landing/src/components/navbar.tsx index 739d849f..ab0f8430 100644 --- a/apps/landing/src/components/navbar.tsx +++ b/apps/landing/src/components/navbar.tsx @@ -24,14 +24,18 @@ export function Navbar() { const [stars, setStars] = useState("Star"); useEffect(() => { + let cancelled = false; fetch("https://api.github.com/repos/snapotter-hq/snapotter") .then((res) => res.json()) .then((data) => { - if (typeof data.stargazers_count === "number") { + if (!cancelled && typeof data.stargazers_count === "number") { setStars(formatStarCount(data.stargazers_count)); } }) .catch(() => {}); + return () => { + cancelled = true; + }; }, []); return ( diff --git a/tests/unit/landing/navbar.test.tsx b/tests/unit/landing/navbar.test.tsx index 63a0ee7e..83c2b9dc 100644 --- a/tests/unit/landing/navbar.test.tsx +++ b/tests/unit/landing/navbar.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; import type React from "react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -28,69 +28,78 @@ beforeEach(() => { afterEach(cleanup); describe("Navbar", () => { - it("renders the brand name", () => { - render(); + it("renders the brand name", async () => { + await act(async () => { + render(); + }); expect(screen.getByText("SnapOtter")).toBeDefined(); }); - it("renders navigation links", () => { - render(); + it("renders navigation links", async () => { + await act(async () => { + render(); + }); expect(screen.getAllByText("Features").length).toBeGreaterThan(0); expect(screen.getAllByText("Pricing").length).toBeGreaterThan(0); expect(screen.getAllByText("Docs").length).toBeGreaterThan(0); expect(screen.getAllByText("Contact").length).toBeGreaterThan(0); }); - it("renders Book a Demo CTA", () => { - render(); + it("renders Book a Demo CTA", async () => { + await act(async () => { + render(); + }); const ctas = screen.getAllByText("Book a Demo"); expect(ctas.length).toBeGreaterThan(0); }); it("fetches GitHub star count on mount", async () => { - render(); - await waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith("https://api.github.com/repos/snapotter-hq/snapotter"); + await act(async () => { + render(); }); + expect(fetchMock).toHaveBeenCalledWith("https://api.github.com/repos/snapotter-hq/snapotter"); }); it("displays formatted star count after fetch", async () => { - render(); - await waitFor(() => { - expect(screen.getByText("1.2k")).toBeDefined(); + await act(async () => { + render(); }); + expect(screen.getByText("1.2k")).toBeDefined(); }); it("formats star count correctly for exact thousands", async () => { fetchMock.mockResolvedValue({ json: () => Promise.resolve({ stargazers_count: 2000 }), }); - render(); - await waitFor(() => { - expect(screen.getByText("2k")).toBeDefined(); + await act(async () => { + render(); }); + expect(screen.getByText("2k")).toBeDefined(); }); it("shows raw count for numbers under 1000", async () => { fetchMock.mockResolvedValue({ json: () => Promise.resolve({ stargazers_count: 456 }), }); - render(); - await waitFor(() => { - expect(screen.getByText("456")).toBeDefined(); + await act(async () => { + render(); }); + expect(screen.getByText("456")).toBeDefined(); }); it("handles star fetch failure gracefully", async () => { fetchMock.mockRejectedValue(new Error("Network error")); - render(); - await waitFor(() => expect(fetchMock).toHaveBeenCalled()); - // Star count should not appear - still shows default "Star" text + await act(async () => { + render(); + }); + expect(fetchMock).toHaveBeenCalled(); expect(screen.getAllByText("Star on GitHub").length).toBeGreaterThan(0); }); - it("toggles mobile menu on hamburger click", () => { - render(); + it("toggles mobile menu on hamburger click", async () => { + await act(async () => { + render(); + }); const toggleButton = screen.getByLabelText("Toggle menu"); expect(screen.queryByText("Book a Demo")).toBeDefined(); fireEvent.click(toggleButton); @@ -98,17 +107,19 @@ describe("Navbar", () => { expect(mobileLinks.length).toBeGreaterThanOrEqual(2); }); - it("closes mobile menu when a link is clicked", () => { - render(); + it("closes mobile menu when a link is clicked", async () => { + await act(async () => { + render(); + }); fireEvent.click(screen.getByLabelText("Toggle menu")); const mobileFeatures = screen.getAllByText("Features"); fireEvent.click(mobileFeatures[mobileFeatures.length - 1]); - // After clicking, mobile menu should close (we can't easily test DOM removal - // without checking state, but the onClick handler calls setOpen(false)) }); - it("links Docs to external URL with target=_blank", () => { - render(); + it("links Docs to external URL with target=_blank", async () => { + await act(async () => { + render(); + }); const docsLinks = screen.getAllByText("Docs"); const externalDoc = docsLinks.find( (el) => el.closest("a")?.getAttribute("target") === "_blank", @@ -117,8 +128,10 @@ describe("Navbar", () => { expect(externalDoc?.closest("a")?.getAttribute("href")).toBe("https://docs.snapotter.com"); }); - it("links GitHub button to correct repo", () => { - render(); + it("links GitHub button to correct repo", async () => { + await act(async () => { + render(); + }); const githubLinks = screen.getAllByText("Star on GitHub"); const link = githubLinks[0].closest("a"); expect(link?.getAttribute("href")).toBe("https://github.com/snapotter-hq/snapotter");