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
This commit is contained in:
SnapOtter
2026-05-01 19:01:05 +08:00
parent ff8dcf63c7
commit 66211ed6a7
9 changed files with 128 additions and 87 deletions
+1
View File
@@ -57,3 +57,4 @@ audit_report.md
.mcp.json .mcp.json
tests/benchmark/bench-limits-results-mac.jsonl tests/benchmark/bench-limits-results-mac.jsonl
.superpowers/ .superpowers/
scripts/setup-cloudflare-email.sh
+2
View File
@@ -100,7 +100,9 @@ app.setErrorHandler((error: Error & { statusCode?: number }, request, reply) =>
{ err: error, url: request.url, method: request.method }, { err: error, url: request.url, method: request.method },
"Unhandled request error", "Unhandled request error",
); );
if (statusCode >= 500) {
captureException(error, request); captureException(error, request);
}
const isProduction = process.env.NODE_ENV === "production"; const isProduction = process.env.NODE_ENV === "production";
reply.status(statusCode).send({ reply.status(statusCode).send({
error: statusCode >= 500 ? "Internal server error" : error.message, error: statusCode >= 500 ? "Internal server error" : error.message,
+4
View File
@@ -72,6 +72,7 @@ export async function apiKeyRoutes(app: FastifyInstance): Promise<void> {
const keyPrefix = computeKeyPrefix(rawKey); const keyPrefix = computeKeyPrefix(rawKey);
const id = randomUUID(); const id = randomUUID();
try {
db.insert(schema.apiKeys) db.insert(schema.apiKeys)
.values({ .values({
id, id,
@@ -83,6 +84,9 @@ export async function apiKeyRoutes(app: FastifyInstance): Promise<void> {
expiresAt, expiresAt,
}) })
.run(); .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 }); auditLog(request.log, "API_KEY_CREATED", { userId: user.id, keyId: id, keyName: name });
+7 -2
View File
@@ -9,7 +9,7 @@
import { spawn } from "node:child_process"; import { spawn } from "node:child_process";
import crypto from "node:crypto"; 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 { join } from "node:path";
import { shutdownDispatcher } from "@snapotter/ai"; import { shutdownDispatcher } from "@snapotter/ai";
import { ANALYTICS_EVENTS, FEATURE_BUNDLES } from "@snapotter/shared"; import { ANALYTICS_EVENTS, FEATURE_BUNDLES } from "@snapotter/shared";
@@ -67,7 +67,12 @@ function getDirSize(dirPath: string): number {
if (!existsSync(dirPath)) return 0; if (!existsSync(dirPath)) return 0;
let total = 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) { for (const entry of entries) {
const fullPath = join(dirPath, entry.name); const fullPath = join(dirPath, entry.name);
if (entry.isDirectory()) { if (entry.isDirectory()) {
+4
View File
@@ -329,6 +329,7 @@ export async function registerPipelineRoutes(app: FastifyInstance): Promise<void
const id = randomUUID(); const id = randomUUID();
try {
db.insert(schema.pipelines) db.insert(schema.pipelines)
.values({ .values({
id, id,
@@ -338,6 +339,9 @@ export async function registerPipelineRoutes(app: FastifyInstance): Promise<void
steps: JSON.stringify(steps), steps: JSON.stringify(steps),
}) })
.run(); .run();
} catch {
return reply.status(409).send({ error: "Failed to save pipeline" });
}
return reply.status(201).send({ return reply.status(201).send({
id, id,
@@ -149,7 +149,7 @@ export function registerOptimizeForWeb(app: FastifyInstance) {
reply.header("Content-Type", result.contentType); reply.header("Content-Type", result.contentType);
reply.header("X-Original-Size", String(fileBuffer.length)); reply.header("X-Original-Size", String(fileBuffer.length));
reply.header("X-Processed-Size", String(result.buffer.length)); reply.header("X-Processed-Size", String(result.buffer.length));
reply.header("X-Output-Filename", result.filename); reply.header("X-Output-Filename", encodeURIComponent(result.filename));
return reply.send(result.buffer); return reply.send(result.buffer);
} catch (err) { } catch (err) {
const message = err instanceof Error ? err.message : "Preview processing failed"; const message = err instanceof Error ? err.message : "Preview processing failed";
+8
View File
@@ -198,6 +198,7 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
// Create DB record // Create DB record
const id = randomUUID(); const id = randomUUID();
try {
db.insert(schema.userFiles) db.insert(schema.userFiles)
.values({ .values({
id, id,
@@ -213,6 +214,9 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
toolChain: null, toolChain: null,
}) })
.run(); .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(); const row = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get();
@@ -555,6 +559,7 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
// Create DB record // Create DB record
const id = randomUUID(); const id = randomUUID();
try {
db.insert(schema.userFiles) db.insert(schema.userFiles)
.values({ .values({
id, id,
@@ -570,6 +575,9 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
toolChain: JSON.stringify(newChain), toolChain: JSON.stringify(newChain),
}) })
.run(); .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(); const row = db.select().from(schema.userFiles).where(eq(schema.userFiles.id, id)).get();
+5 -1
View File
@@ -24,14 +24,18 @@ export function Navbar() {
const [stars, setStars] = useState<string>("Star"); const [stars, setStars] = useState<string>("Star");
useEffect(() => { useEffect(() => {
let cancelled = false;
fetch("https://api.github.com/repos/snapotter-hq/snapotter") fetch("https://api.github.com/repos/snapotter-hq/snapotter")
.then((res) => res.json()) .then((res) => res.json())
.then((data) => { .then((data) => {
if (typeof data.stargazers_count === "number") { if (!cancelled && typeof data.stargazers_count === "number") {
setStars(formatStarCount(data.stargazers_count)); setStars(formatStarCount(data.stargazers_count));
} }
}) })
.catch(() => {}); .catch(() => {});
return () => {
cancelled = true;
};
}, []); }, []);
return ( return (
+33 -20
View File
@@ -1,6 +1,6 @@
// @vitest-environment jsdom // @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 type React from "react";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
@@ -28,69 +28,78 @@ beforeEach(() => {
afterEach(cleanup); afterEach(cleanup);
describe("Navbar", () => { describe("Navbar", () => {
it("renders the brand name", () => { it("renders the brand name", async () => {
await act(async () => {
render(<Navbar />); render(<Navbar />);
});
expect(screen.getByText("SnapOtter")).toBeDefined(); expect(screen.getByText("SnapOtter")).toBeDefined();
}); });
it("renders navigation links", () => { it("renders navigation links", async () => {
await act(async () => {
render(<Navbar />); render(<Navbar />);
});
expect(screen.getAllByText("Features").length).toBeGreaterThan(0); expect(screen.getAllByText("Features").length).toBeGreaterThan(0);
expect(screen.getAllByText("Pricing").length).toBeGreaterThan(0); expect(screen.getAllByText("Pricing").length).toBeGreaterThan(0);
expect(screen.getAllByText("Docs").length).toBeGreaterThan(0); expect(screen.getAllByText("Docs").length).toBeGreaterThan(0);
expect(screen.getAllByText("Contact").length).toBeGreaterThan(0); expect(screen.getAllByText("Contact").length).toBeGreaterThan(0);
}); });
it("renders Book a Demo CTA", () => { it("renders Book a Demo CTA", async () => {
await act(async () => {
render(<Navbar />); render(<Navbar />);
});
const ctas = screen.getAllByText("Book a Demo"); const ctas = screen.getAllByText("Book a Demo");
expect(ctas.length).toBeGreaterThan(0); expect(ctas.length).toBeGreaterThan(0);
}); });
it("fetches GitHub star count on mount", async () => { it("fetches GitHub star count on mount", async () => {
await act(async () => {
render(<Navbar />); render(<Navbar />);
await waitFor(() => {
expect(fetchMock).toHaveBeenCalledWith("https://api.github.com/repos/snapotter-hq/snapotter");
}); });
expect(fetchMock).toHaveBeenCalledWith("https://api.github.com/repos/snapotter-hq/snapotter");
}); });
it("displays formatted star count after fetch", async () => { it("displays formatted star count after fetch", async () => {
await act(async () => {
render(<Navbar />); render(<Navbar />);
await waitFor(() => {
expect(screen.getByText("1.2k")).toBeDefined();
}); });
expect(screen.getByText("1.2k")).toBeDefined();
}); });
it("formats star count correctly for exact thousands", async () => { it("formats star count correctly for exact thousands", async () => {
fetchMock.mockResolvedValue({ fetchMock.mockResolvedValue({
json: () => Promise.resolve({ stargazers_count: 2000 }), json: () => Promise.resolve({ stargazers_count: 2000 }),
}); });
await act(async () => {
render(<Navbar />); render(<Navbar />);
await waitFor(() => {
expect(screen.getByText("2k")).toBeDefined();
}); });
expect(screen.getByText("2k")).toBeDefined();
}); });
it("shows raw count for numbers under 1000", async () => { it("shows raw count for numbers under 1000", async () => {
fetchMock.mockResolvedValue({ fetchMock.mockResolvedValue({
json: () => Promise.resolve({ stargazers_count: 456 }), json: () => Promise.resolve({ stargazers_count: 456 }),
}); });
await act(async () => {
render(<Navbar />); render(<Navbar />);
await waitFor(() => {
expect(screen.getByText("456")).toBeDefined();
}); });
expect(screen.getByText("456")).toBeDefined();
}); });
it("handles star fetch failure gracefully", async () => { it("handles star fetch failure gracefully", async () => {
fetchMock.mockRejectedValue(new Error("Network error")); fetchMock.mockRejectedValue(new Error("Network error"));
await act(async () => {
render(<Navbar />); render(<Navbar />);
await waitFor(() => expect(fetchMock).toHaveBeenCalled()); });
// Star count should not appear - still shows default "Star" text expect(fetchMock).toHaveBeenCalled();
expect(screen.getAllByText("Star on GitHub").length).toBeGreaterThan(0); expect(screen.getAllByText("Star on GitHub").length).toBeGreaterThan(0);
}); });
it("toggles mobile menu on hamburger click", () => { it("toggles mobile menu on hamburger click", async () => {
await act(async () => {
render(<Navbar />); render(<Navbar />);
});
const toggleButton = screen.getByLabelText("Toggle menu"); const toggleButton = screen.getByLabelText("Toggle menu");
expect(screen.queryByText("Book a Demo")).toBeDefined(); expect(screen.queryByText("Book a Demo")).toBeDefined();
fireEvent.click(toggleButton); fireEvent.click(toggleButton);
@@ -98,17 +107,19 @@ describe("Navbar", () => {
expect(mobileLinks.length).toBeGreaterThanOrEqual(2); expect(mobileLinks.length).toBeGreaterThanOrEqual(2);
}); });
it("closes mobile menu when a link is clicked", () => { it("closes mobile menu when a link is clicked", async () => {
await act(async () => {
render(<Navbar />); render(<Navbar />);
});
fireEvent.click(screen.getByLabelText("Toggle menu")); fireEvent.click(screen.getByLabelText("Toggle menu"));
const mobileFeatures = screen.getAllByText("Features"); const mobileFeatures = screen.getAllByText("Features");
fireEvent.click(mobileFeatures[mobileFeatures.length - 1]); 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", () => { it("links Docs to external URL with target=_blank", async () => {
await act(async () => {
render(<Navbar />); render(<Navbar />);
});
const docsLinks = screen.getAllByText("Docs"); const docsLinks = screen.getAllByText("Docs");
const externalDoc = docsLinks.find( const externalDoc = docsLinks.find(
(el) => el.closest("a")?.getAttribute("target") === "_blank", (el) => el.closest("a")?.getAttribute("target") === "_blank",
@@ -117,8 +128,10 @@ describe("Navbar", () => {
expect(externalDoc?.closest("a")?.getAttribute("href")).toBe("https://docs.snapotter.com"); expect(externalDoc?.closest("a")?.getAttribute("href")).toBe("https://docs.snapotter.com");
}); });
it("links GitHub button to correct repo", () => { it("links GitHub button to correct repo", async () => {
await act(async () => {
render(<Navbar />); render(<Navbar />);
});
const githubLinks = screen.getAllByText("Star on GitHub"); const githubLinks = screen.getAllByText("Star on GitHub");
const link = githubLinks[0].closest("a"); const link = githubLinks[0].closest("a");
expect(link?.getAttribute("href")).toBe("https://github.com/snapotter-hq/snapotter"); expect(link?.getAttribute("href")).toBe("https://github.com/snapotter-hq/snapotter");