diff --git a/src/__tests__/email-change.test.ts b/src/__tests__/email-change.test.ts index 755f687..fffaa57 100644 --- a/src/__tests__/email-change.test.ts +++ b/src/__tests__/email-change.test.ts @@ -171,3 +171,97 @@ describe("POST /v1/email-change/verify", () => { ); }); }); + +describe("POST /v1/email-change - Database failure handling", () => { + it("returns 500 when validateApiKey DB query fails", async () => { + const { queryWithRetry } = await import("../services/db.js"); + + vi.mocked(queryWithRetry).mockRejectedValue(new Error("Connection pool exhausted")); + + const res = await request(app).post("/v1/email-change").send({ + apiKey: "df_pro_xxx", + newEmail: "new@example.com" + }); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: "Internal server error" }); + }); + + it("returns 500 when email existence check fails", async () => { + const { queryWithRetry } = await import("../services/db.js"); + + let callCount = 0; + vi.mocked(queryWithRetry).mockImplementation((async (sql: string) => { + callCount++; + // First call (validateApiKey) succeeds + if (callCount === 1 && sql.includes("SELECT") && sql.includes("key =")) { + return { rows: [{ key: "df_pro_xxx", email: "old@example.com", tier: "pro" }], rowCount: 1 }; + } + // Second call (email check) fails + throw new Error("DB connection lost"); + }) as any); + + const res = await request(app).post("/v1/email-change").send({ + apiKey: "df_pro_xxx", + newEmail: "new@example.com" + }); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: "Internal server error" }); + }); + + it("returns 500 when createPendingVerification fails", async () => { + const { createPendingVerification } = await import("../services/verification.js"); + + vi.mocked(createPendingVerification).mockRejectedValue(new Error("DB insert failed")); + + const res = await request(app).post("/v1/email-change").send({ + apiKey: "df_pro_xxx", + newEmail: "new@example.com" + }); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: "Internal server error" }); + }); +}); + +describe("POST /v1/email-change/verify - Database failure handling", () => { + it("returns 500 when validateApiKey DB query fails", async () => { + const { queryWithRetry } = await import("../services/db.js"); + + vi.mocked(queryWithRetry).mockRejectedValue(new Error("Connection timeout")); + + const res = await request(app).post("/v1/email-change/verify").send({ + apiKey: "df_pro_xxx", + newEmail: "new@example.com", + code: "123456" + }); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: "Internal server error" }); + }); + + it("returns 500 when UPDATE query fails", async () => { + const { queryWithRetry } = await import("../services/db.js"); + + let callCount = 0; + vi.mocked(queryWithRetry).mockImplementation((async (sql: string) => { + callCount++; + // First call (validateApiKey) succeeds + if (callCount === 1 && sql.includes("SELECT") && sql.includes("key =")) { + return { rows: [{ key: "df_pro_xxx", email: "old@example.com", tier: "pro" }], rowCount: 1 }; + } + // Second call (UPDATE) fails + throw new Error("UPDATE failed - constraint violation"); + }) as any); + + const res = await request(app).post("/v1/email-change/verify").send({ + apiKey: "df_pro_xxx", + newEmail: "new@example.com", + code: "123456" + }); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: "Internal server error" }); + }); +}); diff --git a/src/__tests__/error-handler.test.ts b/src/__tests__/error-handler.test.ts new file mode 100644 index 0000000..5dd6fe5 --- /dev/null +++ b/src/__tests__/error-handler.test.ts @@ -0,0 +1,118 @@ +import { describe, it, expect, vi } from "vitest"; +import express, { Request, Response, NextFunction } from "express"; +import request from "supertest"; + +const mockLogger = { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }; +vi.mock("../services/logger.js", () => ({ + default: mockLogger, +})); + +describe("Global error handler", () => { + it("returns 500 JSON for unhandled errors in API routes", async () => { + const app = express(); + + // Add request ID middleware (like in main app) + app.use((req, _res, next) => { + (req as any).requestId = "test-req-id"; + next(); + }); + + // Add a test route that throws an error + app.get("/v1/test-error", (_req: Request, _res: Response) => { + throw new Error("Test unhandled error"); + }); + + // Add global error handler (same as in src/index.ts) + app.use((err: unknown, req: Request, res: Response, _next: NextFunction) => { + const reqId = (req as any).requestId || "unknown"; + mockLogger.error({ err, requestId: reqId, method: req.method, path: req.path }, "Unhandled route error"); + if (!res.headersSent) { + const isApi = req.path.startsWith("/v1/") || req.path.startsWith("/health"); + if (isApi) { + res.status(500).json({ error: "Internal server error" }); + } else { + res.status(500).send("Internal server error"); + } + } + }); + + const res = await request(app).get("/v1/test-error"); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: "Internal server error" }); + expect(res.headers["content-type"]).toMatch(/json/); + expect(mockLogger.error).toHaveBeenCalledWith( + expect.objectContaining({ + method: "GET", + path: "/v1/test-error", + }), + "Unhandled route error" + ); + }); + + it("returns 500 text for unhandled errors in non-API routes", async () => { + const app = express(); + + // Add request ID middleware + app.use((req, _res, next) => { + (req as any).requestId = "test-req-id"; + next(); + }); + + // Add a test route that throws an error + app.get("/test-error-page", (_req: Request, _res: Response) => { + throw new Error("Test page error"); + }); + + // Add global error handler + app.use((err: unknown, req: Request, res: Response, _next: NextFunction) => { + const reqId = (req as any).requestId || "unknown"; + mockLogger.error({ err, requestId: reqId, method: req.method, path: req.path }, "Unhandled route error"); + if (!res.headersSent) { + const isApi = req.path.startsWith("/v1/") || req.path.startsWith("/health"); + if (isApi) { + res.status(500).json({ error: "Internal server error" }); + } else { + res.status(500).send("Internal server error"); + } + } + }); + + const res = await request(app).get("/test-error-page"); + + expect(res.status).toBe(500); + expect(res.text).toBe("Internal server error"); + expect(res.headers["content-type"]).toMatch(/text/); + expect(mockLogger.error).toHaveBeenCalled(); + }); + + it("does not send response if headers already sent", async () => { + const app = express(); + app.use(express.json()); + + app.get("/v1/test-headers-sent", (_req: Request, res: Response, next: NextFunction) => { + res.status(200).json({ ok: true }); + next(new Error("Too late")); + }); + + // Add global error handler + app.use((err: unknown, req: Request, res: Response, _next: NextFunction) => { + const reqId = (req as any).requestId || "unknown"; + mockLogger.error({ err, requestId: reqId, method: req.method, path: req.path }, "Unhandled route error"); + if (!res.headersSent) { + const isApi = req.path.startsWith("/v1/") || req.path.startsWith("/health"); + if (isApi) { + res.status(500).json({ error: "Internal server error" }); + } else { + res.status(500).send("Internal server error"); + } + } + }); + + const res = await request(app).get("/v1/test-headers-sent"); + + // Should get the 200 response, error handler does nothing + expect(res.status).toBe(200); + expect(res.body).toEqual({ ok: true }); + }); +}); diff --git a/src/__tests__/recover.test.ts b/src/__tests__/recover.test.ts index aebde21..6074ad8 100644 --- a/src/__tests__/recover.test.ts +++ b/src/__tests__/recover.test.ts @@ -2,6 +2,11 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import express from "express"; import request from "supertest"; +vi.mock("../services/db.js"); +vi.mock("../services/logger.js", () => ({ + default: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, +})); + let app: express.Express; beforeEach(async () => { @@ -13,6 +18,7 @@ beforeEach(async () => { const { createPendingVerification, verifyCode } = await import("../services/verification.js"); const { sendVerificationEmail } = await import("../services/email.js"); const { getAllKeys } = await import("../services/keys.js"); + const { queryWithRetry } = await import("../services/db.js"); vi.mocked(createPendingVerification).mockResolvedValue({ email: "test@test.com", code: "654321", createdAt: "", expiresAt: "", attempts: 0 }); vi.mocked(verifyCode).mockResolvedValue({ status: "ok" }); @@ -20,6 +26,8 @@ beforeEach(async () => { vi.mocked(getAllKeys).mockReturnValue([ { key: "existing-key", tier: "pro" as const, email: "found@test.com", createdAt: "2025-01-01" }, ]); + // Default DB mock: returns empty (no user found in DB fallback) + vi.mocked(queryWithRetry).mockResolvedValue({ rows: [], rowCount: 0 } as any); const { recoverRouter } = await import("../routes/recover.js"); app = express(); @@ -94,3 +102,40 @@ describe("POST /recover/verify", () => { expect(res.body.apiKey).toBeUndefined(); }); }); + +describe("POST /recover - Database failure handling", () => { + it("returns 500 when queryWithRetry throws in DB fallback", async () => { + const { queryWithRetry } = await import("../services/db.js"); + const { getAllKeys } = await import("../services/keys.js"); + + // Mock cache miss + DB failure + vi.mocked(getAllKeys).mockReturnValue([]); + vi.mocked(queryWithRetry).mockRejectedValue(new Error("Connection pool exhausted")); + + const res = await request(app).post("/recover").send({ email: "test@example.com" }); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: "Internal server error" }); + }); +}); + +describe("POST /recover/verify - Database failure handling", () => { + it("returns 500 when queryWithRetry throws in DB fallback", async () => { + const { queryWithRetry } = await import("../services/db.js"); + const { getAllKeys } = await import("../services/keys.js"); + const { verifyCode } = await import("../services/verification.js"); + + // Mock cache miss + verifyCode success + DB failure + vi.mocked(getAllKeys).mockReturnValue([]); + vi.mocked(verifyCode).mockResolvedValue({ status: "ok" }); + vi.mocked(queryWithRetry).mockRejectedValue(new Error("Unexpected DB schema error")); + + const res = await request(app).post("/recover/verify").send({ + email: "test@example.com", + code: "123456" + }); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: "Internal server error" }); + }); +}); diff --git a/src/index.ts b/src/index.ts index 0659e2d..b786f79 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -import express, { Request, Response } from "express"; +import express, { Request, Response, NextFunction } from "express"; import { randomUUID } from "crypto"; import { AuthenticatedRequest } from "./types.js"; import "./types.js"; // Augments Express.Request with requestId, acquirePdfSlot, releasePdfSlot @@ -221,7 +221,29 @@ app.use((req, res) => { } }); - +// Global error handler — must be after all routes +app.use((err: unknown, req: Request, res: Response, _next: NextFunction) => { + const reqId = (req as any).requestId || "unknown"; + + // Check if this is a JSON parse error from express.json() + if (err instanceof SyntaxError && 'status' in err && (err as any).status === 400 && 'body' in err) { + logger.warn({ err, requestId: reqId, method: req.method, path: req.path }, "Invalid JSON body"); + if (!res.headersSent) { + res.status(400).json({ error: "Invalid JSON in request body" }); + } + return; + } + + logger.error({ err, requestId: reqId, method: req.method, path: req.path }, "Unhandled route error"); + if (!res.headersSent) { + const isApi = req.path.startsWith("/v1/") || req.path.startsWith("/health"); + if (isApi) { + res.status(500).json({ error: "Internal server error" }); + } else { + res.status(500).send("Internal server error"); + } + } +}); async function start() { // Initialize PostgreSQL diff --git a/src/routes/email-change.ts b/src/routes/email-change.ts index 72dcfb0..c64e4a8 100644 --- a/src/routes/email-change.ts +++ b/src/routes/email-change.ts @@ -71,48 +71,54 @@ async function validateApiKey(apiKey: string) { * description: Too many attempts */ router.post("/", emailChangeLimiter, async (req: Request, res: Response) => { - const { apiKey, newEmail } = req.body || {}; + try { + const { apiKey, newEmail } = req.body || {}; - if (!apiKey || typeof apiKey !== "string") { - res.status(400).json({ error: "apiKey is required." }); - return; + if (!apiKey || typeof apiKey !== "string") { + res.status(400).json({ error: "apiKey is required." }); + return; + } + + if (!newEmail || typeof newEmail !== "string") { + res.status(400).json({ error: "newEmail is required." }); + return; + } + + const cleanEmail = newEmail.trim().toLowerCase(); + + if (!EMAIL_RE.test(cleanEmail)) { + res.status(400).json({ error: "Invalid email format." }); + return; + } + + const keyRow = await validateApiKey(apiKey); + if (!keyRow) { + res.status(403).json({ error: "Invalid API key." }); + return; + } + + // Check if email is already taken by another key + const existing = await queryWithRetry( + `SELECT key FROM api_keys WHERE email = $1 AND key != $2`, + [cleanEmail, apiKey] + ); + if (existing.rows.length > 0) { + res.status(409).json({ error: "This email is already associated with another account." }); + return; + } + + const pending = await createPendingVerification(cleanEmail); + + sendVerificationEmail(cleanEmail, pending.code).catch(err => { + logger.error({ err, email: cleanEmail }, "Failed to send email change verification"); + }); + + res.json({ status: "verification_sent", message: "A verification code has been sent to your new email address." }); + } catch (err: unknown) { + const reqId = (req as any).requestId || "unknown"; + logger.error({ err, requestId: reqId }, "Unhandled error in POST /email-change"); + res.status(500).json({ error: "Internal server error" }); } - - if (!newEmail || typeof newEmail !== "string") { - res.status(400).json({ error: "newEmail is required." }); - return; - } - - const cleanEmail = newEmail.trim().toLowerCase(); - - if (!EMAIL_RE.test(cleanEmail)) { - res.status(400).json({ error: "Invalid email format." }); - return; - } - - const keyRow = await validateApiKey(apiKey); - if (!keyRow) { - res.status(403).json({ error: "Invalid API key." }); - return; - } - - // Check if email is already taken by another key - const existing = await queryWithRetry( - `SELECT key FROM api_keys WHERE email = $1 AND key != $2`, - [cleanEmail, apiKey] - ); - if (existing.rows.length > 0) { - res.status(409).json({ error: "This email is already associated with another account." }); - return; - } - - const pending = await createPendingVerification(cleanEmail); - - sendVerificationEmail(cleanEmail, pending.code).catch(err => { - logger.error({ err, email: cleanEmail }, "Failed to send email change verification"); - }); - - res.json({ status: "verification_sent", message: "A verification code has been sent to your new email address." }); }); /** @@ -161,43 +167,49 @@ router.post("/", emailChangeLimiter, async (req: Request, res: Response) => { * description: Too many failed attempts */ router.post("/verify", async (req: Request, res: Response) => { - const { apiKey, newEmail, code } = req.body || {}; + try { + const { apiKey, newEmail, code } = req.body || {}; - if (!apiKey || !newEmail || !code) { - res.status(400).json({ error: "apiKey, newEmail, and code are required." }); - return; - } - - const cleanEmail = newEmail.trim().toLowerCase(); - const cleanCode = String(code).trim(); - - const keyRow = await validateApiKey(apiKey); - if (!keyRow) { - res.status(403).json({ error: "Invalid API key." }); - return; - } - - const result = await verifyCode(cleanEmail, cleanCode); - - switch (result.status) { - case "ok": { - await queryWithRetry( - `UPDATE api_keys SET email = $1 WHERE key = $2`, - [cleanEmail, apiKey] - ); - logger.info({ apiKey: apiKey.slice(0, 10) + "...", newEmail: cleanEmail }, "Email changed"); - res.json({ status: "ok", newEmail: cleanEmail }); - break; + if (!apiKey || !newEmail || !code) { + res.status(400).json({ error: "apiKey, newEmail, and code are required." }); + return; } - case "expired": - res.status(410).json({ error: "Verification code has expired. Please request a new one." }); - break; - case "max_attempts": - res.status(429).json({ error: "Too many failed attempts. Please request a new code." }); - break; - case "invalid": - res.status(400).json({ error: "Invalid verification code." }); - break; + + const cleanEmail = newEmail.trim().toLowerCase(); + const cleanCode = String(code).trim(); + + const keyRow = await validateApiKey(apiKey); + if (!keyRow) { + res.status(403).json({ error: "Invalid API key." }); + return; + } + + const result = await verifyCode(cleanEmail, cleanCode); + + switch (result.status) { + case "ok": { + await queryWithRetry( + `UPDATE api_keys SET email = $1 WHERE key = $2`, + [cleanEmail, apiKey] + ); + logger.info({ apiKey: apiKey.slice(0, 10) + "...", newEmail: cleanEmail }, "Email changed"); + res.json({ status: "ok", newEmail: cleanEmail }); + break; + } + case "expired": + res.status(410).json({ error: "Verification code has expired. Please request a new one." }); + break; + case "max_attempts": + res.status(429).json({ error: "Too many failed attempts. Please request a new code." }); + break; + case "invalid": + res.status(400).json({ error: "Invalid verification code." }); + break; + } + } catch (err: unknown) { + const reqId = (req as any).requestId || "unknown"; + logger.error({ err, requestId: reqId }, "Unhandled error in POST /email-change/verify"); + res.status(500).json({ error: "Internal server error" }); } }); diff --git a/src/routes/recover.ts b/src/routes/recover.ts index d861f46..6419f94 100644 --- a/src/routes/recover.ts +++ b/src/routes/recover.ts @@ -57,41 +57,47 @@ const recoverLimiter = rateLimit({ * description: Too many recovery attempts */ router.post("/", recoverLimiter, async (req: Request, res: Response) => { - const { email } = req.body || {}; + try { + const { email } = req.body || {}; - if (!email || typeof email !== "string" || !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) { - res.status(400).json({ error: "A valid email address is required." }); - return; - } - - const cleanEmail = email.trim().toLowerCase(); - const keys = getAllKeys(); - const userKey = keys.find(k => k.email === cleanEmail); - - if (!userKey) { - // DB fallback: cache may be stale in multi-replica setups - const dbResult = await queryWithRetry( - "SELECT key FROM api_keys WHERE email = $1 LIMIT 1", - [cleanEmail] - ); - if (dbResult.rows.length > 0) { - const pending = await createPendingVerification(cleanEmail); - sendVerificationEmail(cleanEmail, pending.code).catch(err => { - logger.error({ err, email: cleanEmail }, "Failed to send recovery email"); - }); - logger.info({ email: cleanEmail }, "recover: cache miss, sent recovery via DB fallback"); + if (!email || typeof email !== "string" || !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) { + res.status(400).json({ error: "A valid email address is required." }); + return; } + + const cleanEmail = email.trim().toLowerCase(); + const keys = getAllKeys(); + const userKey = keys.find(k => k.email === cleanEmail); + + if (!userKey) { + // DB fallback: cache may be stale in multi-replica setups + const dbResult = await queryWithRetry( + "SELECT key FROM api_keys WHERE email = $1 LIMIT 1", + [cleanEmail] + ); + if (dbResult.rows.length > 0) { + const pending = await createPendingVerification(cleanEmail); + sendVerificationEmail(cleanEmail, pending.code).catch(err => { + logger.error({ err, email: cleanEmail }, "Failed to send recovery email"); + }); + logger.info({ email: cleanEmail }, "recover: cache miss, sent recovery via DB fallback"); + } + res.json({ status: "recovery_sent", message: "If an account exists for this email, a verification code has been sent." }); + return; + } + + const pending = await createPendingVerification(cleanEmail); + + sendVerificationEmail(cleanEmail, pending.code).catch(err => { + logger.error({ err, email: cleanEmail }, "Failed to send recovery email"); + }); + res.json({ status: "recovery_sent", message: "If an account exists for this email, a verification code has been sent." }); - return; + } catch (err: unknown) { + const reqId = (req as any).requestId || "unknown"; + logger.error({ err, requestId: reqId }, "Unhandled error in POST /recover"); + res.status(500).json({ error: "Internal server error" }); } - - const pending = await createPendingVerification(cleanEmail); - - sendVerificationEmail(cleanEmail, pending.code).catch(err => { - logger.error({ err, email: cleanEmail }, "Failed to send recovery email"); - }); - - res.json({ status: "recovery_sent", message: "If an account exists for this email, a verification code has been sent." }); }); /** @@ -141,66 +147,72 @@ router.post("/", recoverLimiter, async (req: Request, res: Response) => { * description: Too many failed attempts */ router.post("/verify", recoverLimiter, async (req: Request, res: Response) => { - const { email, code } = req.body || {}; + try { + const { email, code } = req.body || {}; - if (!email || !code) { - res.status(400).json({ error: "Email and code are required." }); - return; - } - - const cleanEmail = email.trim().toLowerCase(); - const cleanCode = String(code).trim(); - - const result = await verifyCode(cleanEmail, cleanCode); - - switch (result.status) { - case "ok": { - const keys = getAllKeys(); - let userKey = keys.find(k => k.email === cleanEmail); - - // DB fallback: cache may be stale in multi-replica setups - if (!userKey) { - logger.info({ email: cleanEmail }, "recover verify: cache miss, falling back to DB"); - const dbResult = await queryWithRetry( - "SELECT key, tier, email, created_at, stripe_customer_id FROM api_keys WHERE email = $1 LIMIT 1", - [cleanEmail] - ); - if (dbResult.rows.length > 0) { - const row = dbResult.rows[0]; - userKey = { - key: row.key, - tier: row.tier as "free" | "pro", - email: row.email, - createdAt: row.created_at instanceof Date ? row.created_at.toISOString() : row.created_at, - stripeCustomerId: row.stripe_customer_id || undefined, - }; - } - } - - if (userKey) { - res.json({ - status: "recovered", - apiKey: userKey.key, - tier: userKey.tier, - message: "Your API key has been recovered. Save it securely — it is shown only once.", - }); - } else { - res.json({ - status: "recovered", - message: "No API key found for this email.", - }); - } - break; + if (!email || !code) { + res.status(400).json({ error: "Email and code are required." }); + return; } - case "expired": - res.status(410).json({ error: "Verification code has expired. Please request a new one." }); - break; - case "max_attempts": - res.status(429).json({ error: "Too many failed attempts. Please request a new code." }); - break; - case "invalid": - res.status(400).json({ error: "Invalid verification code." }); - break; + + const cleanEmail = email.trim().toLowerCase(); + const cleanCode = String(code).trim(); + + const result = await verifyCode(cleanEmail, cleanCode); + + switch (result.status) { + case "ok": { + const keys = getAllKeys(); + let userKey = keys.find(k => k.email === cleanEmail); + + // DB fallback: cache may be stale in multi-replica setups + if (!userKey) { + logger.info({ email: cleanEmail }, "recover verify: cache miss, falling back to DB"); + const dbResult = await queryWithRetry( + "SELECT key, tier, email, created_at, stripe_customer_id FROM api_keys WHERE email = $1 LIMIT 1", + [cleanEmail] + ); + if (dbResult.rows.length > 0) { + const row = dbResult.rows[0]; + userKey = { + key: row.key, + tier: row.tier as "free" | "pro", + email: row.email, + createdAt: row.created_at instanceof Date ? row.created_at.toISOString() : row.created_at, + stripeCustomerId: row.stripe_customer_id || undefined, + }; + } + } + + if (userKey) { + res.json({ + status: "recovered", + apiKey: userKey.key, + tier: userKey.tier, + message: "Your API key has been recovered. Save it securely — it is shown only once.", + }); + } else { + res.json({ + status: "recovered", + message: "No API key found for this email.", + }); + } + break; + } + case "expired": + res.status(410).json({ error: "Verification code has expired. Please request a new one." }); + break; + case "max_attempts": + res.status(429).json({ error: "Too many failed attempts. Please request a new code." }); + break; + case "invalid": + res.status(400).json({ error: "Invalid verification code." }); + break; + } + } catch (err: unknown) { + const reqId = (req as any).requestId || "unknown"; + logger.error({ err, requestId: reqId }, "Unhandled error in POST /recover/verify"); + res.status(500).json({ error: "Internal server error" }); } });