From 3fdd0c19def2b912d4a614f555a49cbb74a77f9d Mon Sep 17 00:00:00 2001 From: Netty-kun Date: Mon, 20 Jul 2026 11:13:12 +0100 Subject: [PATCH] fix(backend): consolidate error handling with custom error classes - Add AppError base class with statusCode, code, isOperational, details - Add ValidationError (400), NotFoundError (404), UnauthorizedError (401), RateLimitError (429) - Update errorHandler to handle all AppError subclasses - Hide stack traces in production (NODE_ENV=production) - Refactor auth middleware to throw UnauthorizedError - Refactor all routes to use custom error classes via next(err) - Update tests for new consistent error shape - All errors now return { error: { code, message, details? } } Closes #109 --- backend/src/api/app.ts | 197 ++++++++++----------- backend/src/api/middleware/auth.ts | 4 +- backend/src/api/middleware/errorHandler.ts | 45 ++++- backend/src/api/routes/agents.ts | 58 +++--- backend/src/api/routes/stats.ts | 8 +- backend/src/api/routes/tasks.ts | 25 ++- backend/src/errors/AppError.ts | 16 ++ backend/src/errors/NotFoundError.ts | 9 + backend/src/errors/RateLimitError.ts | 8 + backend/src/errors/UnauthorizedError.ts | 8 + backend/src/errors/ValidationError.ts | 8 + backend/src/errors/index.ts | 5 + backend/tests/agents.test.ts | 22 ++- backend/tests/middleware.test.ts | 17 +- 14 files changed, 265 insertions(+), 165 deletions(-) create mode 100644 backend/src/errors/AppError.ts create mode 100644 backend/src/errors/NotFoundError.ts create mode 100644 backend/src/errors/RateLimitError.ts create mode 100644 backend/src/errors/UnauthorizedError.ts create mode 100644 backend/src/errors/ValidationError.ts create mode 100644 backend/src/errors/index.ts diff --git a/backend/src/api/app.ts b/backend/src/api/app.ts index b8e4f73..86494b8 100644 --- a/backend/src/api/app.ts +++ b/backend/src/api/app.ts @@ -25,6 +25,7 @@ import { requestLogger } from "./middleware/requestLogger"; import { errorHandler } from "./middleware/errorHandler"; import { createLogger } from "../utils/logger"; import { createTaskDb, getTaskDb } from "../db/tasks"; +import { ValidationError, UnauthorizedError, NotFoundError, AppError } from "../errors"; export interface AppOptions { /** Called to execute a single DAG node; defaults to HTTP dispatch */ @@ -37,12 +38,6 @@ export interface AppOptions { stream?: TaskStreamOptions; } -/** - * Attempt to load smart-contracts releasePayment at runtime via dynamic require. - * Returns undefined when the module is unavailable (e.g. backend CI without - * smart-contracts compiled). Using require() instead of a static import keeps - * TypeScript's rootDir constraint intact. - */ function tryLoadStellarRelease(): StellarReleasePaymentFn | undefined { try { // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -59,7 +54,6 @@ export function createApp(opts: AppOptions = {}): { } { const app = express(); app.use(express.json()); - // ── Global middleware ──────────────────────────────────────────────────────── app.use(requestId); app.use(requestLogger); @@ -67,124 +61,126 @@ export function createApp(opts: AppOptions = {}): { const releasePayment: PaymentReleaseFn = opts.releasePayment ?? createPaymentReleaseFn(tryLoadStellarRelease()); - // ── Health routes ─────────────────────────────────────────────────────────── app.use("/health", healthRouter); - - // ── Agent routes ─────────────────────────────────────────────────────────── app.use("/api/agents", agentsRouter); - // ── POST /api/tasks ──────────────────────────────────────────────────────── app.post( "/api/tasks", authMiddleware, rateLimitMiddleware, - (req: Request, res: Response) => { - const { prompt, walletPublicKey, maxBudgetXLM } = req.body as { - prompt?: string; - walletPublicKey?: string; - maxBudgetXLM?: number; - }; - - if (!prompt || typeof prompt !== "string" || prompt.trim() === "") { - return res.status(400).json({ error: "prompt is required" }); - } - - if (maxBudgetXLM !== undefined && maxBudgetXLM < 0.1) { - return res.status(400).json({ error: "maxBudgetXLM must be >= 0.1" }); - } - - const taskId = `task_${randomUUID().replace(/-/g, "").slice(0, 12)}`; - const dag = decompose(taskId, prompt); - const now = new Date().toISOString(); - const correlationId = res.locals.requestId; - - createTask({ - taskId, - prompt, - walletPublicKey: - walletPublicKey ?? - (req.headers["walletpublickey"] as string | undefined) ?? - "anonymous", - status: "queued", - dag, - createdAt: now, - updatedAt: now, - requestId: correlationId, - }); + (req: Request, res: Response, next: NextFunction) => { + try { + const { prompt, walletPublicKey, maxBudgetXLM } = req.body as { + prompt?: string; + walletPublicKey?: string; + maxBudgetXLM?: number; + }; + + if (!prompt || typeof prompt !== "string" || prompt.trim() === "") { + throw new ValidationError("prompt is required"); + } + + if (maxBudgetXLM !== undefined && maxBudgetXLM < 0.1) { + throw new ValidationError("maxBudgetXLM must be >= 0.1"); + } + + const taskId = `task_${randomUUID().replace(/-/g, "").slice(0, 12)}`; + const dag = decompose(taskId, prompt); + const now = new Date().toISOString(); + const correlationId = res.locals.requestId; + + createTask({ + taskId, + prompt, + walletPublicKey: + walletPublicKey ?? + (req.headers["walletpublickey"] as string | undefined) ?? + "anonymous", + status: "queued", + dag, + createdAt: now, + updatedAt: now, + requestId: correlationId, + }); - const log = createLogger({ requestId: correlationId, taskId }); + const log = createLogger({ requestId: correlationId, taskId }); - // Run the DAG asynchronously — do not await - setImmediate(() => { - executeDAG(getTask(taskId)!, dispatch, releasePayment).catch((err) => { - log.error({ err }, "DAG execution error"); + setImmediate(() => { + executeDAG(getTask(taskId)!, dispatch, releasePayment).catch((err) => { + log.error({ err }, "DAG execution error"); + }); }); - }); - log.info({ dagNodeCount: dag.length }, "task created"); + log.info({ dagNodeCount: dag.length }, "task created"); - return res - .status(201) - .json({ taskId, dagPreview: dag, status: "queued" }); + return res + .status(201) + .json({ taskId, dagPreview: dag, status: "queued" }); + } catch (err) { + next(err); + } }, ); - // ── GET /api/tasks ───────────────────────────────────────────────────────── - app.get("/api/tasks", authMiddleware, (req: Request, res: Response) => { - const walletPublicKey = req.headers["walletpublickey"] as - string | undefined; - if (!walletPublicKey) - return res.status(401).json({ error: "walletpublickey header required" }); - const page = Math.max(1, parseInt((req.query.page as string) ?? "1", 10)); - const pageSize = Math.min( - 100, - Math.max(1, parseInt((req.query.pageSize as string) ?? "20", 10)), - ); - const taskDb = createTaskDb(getTaskDb()); - const status = req.query.status as string | undefined; - const q = req.query.q as string | undefined; - const sort = req.query.sort as - "createdAt:asc" | "createdAt:desc" | undefined; - const { tasks, total } = taskDb.list(walletPublicKey, page, pageSize, { - status, - q, - sort, - }); - return res.json({ tasks, total, page, pageSize }); + app.get("/api/tasks", authMiddleware, (req: Request, res: Response, next: NextFunction) => { + try { + const walletPublicKey = req.headers["walletpublickey"] as string | undefined; + if (!walletPublicKey) { + throw new UnauthorizedError("walletpublickey header required"); + } + const page = Math.max(1, parseInt((req.query.page as string) ?? "1", 10)); + const pageSize = Math.min( + 100, + Math.max(1, parseInt((req.query.pageSize as string) ?? "20", 10)), + ); + const taskDb = createTaskDb(getTaskDb()); + const status = req.query.status as string | undefined; + const q = req.query.q as string | undefined; + const sort = req.query.sort as + "createdAt:asc" | "createdAt:desc" | undefined; + const { tasks, total } = taskDb.list(walletPublicKey, page, pageSize, { + status, + q, + sort, + }); + return res.json({ tasks, total, page, pageSize }); + } catch (err) { + next(err); + } }); - // ── GET /api/tasks/:id ───────────────────────────────────────────────────── - app.get("/api/tasks/:id", (req: Request, res: Response) => { - const task = getTask(req.params.id!); - if (!task) return res.status(404).json({ error: "Task not found" }); - return res.json({ ...task, id: task.taskId, dag: task.dag }); + app.get("/api/tasks/:id", (req: Request, res: Response, next: NextFunction) => { + try { + const task = getTask(req.params.id!); + if (!task) throw new NotFoundError("Task"); + return res.json({ ...task, id: task.taskId, dag: task.dag }); + } catch (err) { + next(err); + } }); - // ── DELETE /api/tasks/:id ────────────────────────────────────────────────── - app.delete("/api/tasks/:id", (req: Request, res: Response) => { - const task = getTask(req.params.id!); - if (!task) return res.status(404).json({ error: "Task not found" }); - if (task.status === "running") { - return res.status(409).json({ error: "Cannot cancel a running task" }); + app.delete("/api/tasks/:id", (req: Request, res: Response, next: NextFunction) => { + try { + const task = getTask(req.params.id!); + if (!task) throw new NotFoundError("Task"); + if (task.status === "running") { + throw new AppError("Cannot cancel a running task", 409, "CONFLICT"); + } + const taskDb = createTaskDb(getTaskDb()); + taskDb.updateStatus(req.params.id!, "cancelled"); + return res.json({ ...task, id: task.taskId, status: "cancelled" }); + } catch (err) { + next(err); } - const taskDb = createTaskDb(getTaskDb()); - taskDb.updateStatus(req.params.id!, "cancelled"); - return res.json({ ...task, id: task.taskId, status: "cancelled" }); }); - // ── HTTP server ──────────────────────────────────────────────────────────── const httpServer = createServer(app); - // ── Event persistence ────────────────────────────────────────────────────── - // Record every Coordinator event (with its EventBus-assigned per-task seq) so - // a (re)connecting client can replay history before live streaming begins — - // either the full history, or only events past a `?lastEventId` cursor. const eventStore = opts.eventStore ?? createEventStore(); const stopRecording = eventBus.subscribeAll((event) => eventStore.append(event), ); - // ── WebSocket: /tasks/:id/stream ─────────────────────────────────────────── const detachStream = attachTaskStream({ httpServer, eventStore, @@ -193,7 +189,6 @@ export function createApp(opts: AppOptions = {}): { ...opts.stream, }); - // ── Error handler (must be last) ─────────────────────────────────────────── app.use(errorHandler); function close(): void { @@ -211,7 +206,9 @@ async function defaultDispatch( node: { nodeId: string; agentType: string; prompt: string }, context: string, ): Promise { - // In production this POSTs to the agent's HTTP endpoint. - // The e2e test replaces this via opts.dispatch. - throw new Error(`No agent registered for type: ${node.agentType}`); + throw new AppError( + `No agent registered for type: ${node.agentType}`, + 500, + "AGENT_NOT_FOUND", + ); } diff --git a/backend/src/api/middleware/auth.ts b/backend/src/api/middleware/auth.ts index abea02c..808b6e8 100644 --- a/backend/src/api/middleware/auth.ts +++ b/backend/src/api/middleware/auth.ts @@ -1,4 +1,5 @@ import type { Request, Response, NextFunction } from 'express'; +import { UnauthorizedError } from '../../errors'; function loadKeys(): Set | null { const raw = process.env.API_KEYS; @@ -10,7 +11,6 @@ function loadKeys(): Set | null { export function authMiddleware(req: Request, res: Response, next: NextFunction): void { const keys = loadKeys(); if (!keys) { - // API_KEYS unset — no-op, backward compatible next(); return; } @@ -18,7 +18,7 @@ export function authMiddleware(req: Request, res: Response, next: NextFunction): const auth = req.headers['authorization'] ?? ''; const token = auth.startsWith('Bearer ') ? auth.slice(7) : ''; if (!token || !keys.has(token)) { - res.status(401).json({ error: 'Unauthorized' }); + next(new UnauthorizedError()); return; } diff --git a/backend/src/api/middleware/errorHandler.ts b/backend/src/api/middleware/errorHandler.ts index 9e7c6f8..6c2a4bc 100644 --- a/backend/src/api/middleware/errorHandler.ts +++ b/backend/src/api/middleware/errorHandler.ts @@ -1,9 +1,52 @@ import type { Request, Response, NextFunction } from "express"; import { createLogger } from "../../utils/logger"; +import { AppError } from "../../errors"; const log = createLogger(); +const isProduction = process.env.NODE_ENV === "production"; export function errorHandler(err: Error, req: Request, res: Response, _next: NextFunction): void { + if (err instanceof AppError) { + if (!err.isOperational) { + log.error( + { + error: err.message, + code: err.code, + stack: err.stack, + method: req.method, + path: req.path, + requestId: res.locals.requestId, + }, + "non-operational error", + ); + } else { + log.warn( + { + error: err.message, + code: err.code, + method: req.method, + path: req.path, + requestId: res.locals.requestId, + }, + "operational error", + ); + } + + const body: { error: { code: string; message: string; details?: unknown } } = { + error: { + code: err.code, + message: err.message, + }, + }; + + if (err.details !== undefined) { + body.error.details = err.details; + } + + res.status(err.statusCode).json(body); + return; + } + log.error( { error: err.message, @@ -17,7 +60,7 @@ export function errorHandler(err: Error, req: Request, res: Response, _next: Nex res.status(500).json({ error: { - message: err.message || "Internal server error", + message: isProduction ? "Internal server error" : err.message || "Internal server error", code: "INTERNAL_ERROR", }, }); diff --git a/backend/src/api/routes/agents.ts b/backend/src/api/routes/agents.ts index c681a32..989b87a 100644 --- a/backend/src/api/routes/agents.ts +++ b/backend/src/api/routes/agents.ts @@ -1,7 +1,8 @@ -import { Router, Request, Response } from "express"; +import { Router, Request, Response, NextFunction } from "express"; import { z } from "zod"; import { Keypair, Server as HorizonServer } from "@stellar/stellar-sdk"; import { getAgentDb, createAgentDb, AgentDb } from "../../db/agents"; +import { NotFoundError, ValidationError, UnauthorizedError, AppError } from "../../errors"; export interface AgentsRouterOptions { healthTimeoutMs?: number; @@ -25,38 +26,35 @@ export function createAgentsRouter(options: AgentsRouterOptions = {}): Router { const getDb = () => options.db ?? createAgentDb(getAgentDb()); - // GET /api/agents - router.get("/", (req: Request, res: Response): void => { + router.get("/", (req: Request, res: Response, next: NextFunction): void => { const db = getDb(); const capability = req.query.capability as string | undefined; const minReputation = req.query.minReputation ? parseFloat(req.query.minReputation as string) : undefined; const maxPriceXLM = req.query.maxPriceXLM ? parseFloat(req.query.maxPriceXLM as string) : undefined; - + try { const agents = db.list({ capability, minReputation, maxPriceXLM }); res.json(agents); } catch (err) { - res.status(500).json({ error: "Internal Server Error" }); + next(new AppError("Internal Server Error", 500, "INTERNAL_ERROR")); } }); - // GET /api/agents/:id - router.get("/:id", (req: Request, res: Response): void => { + router.get("/:id", (req: Request, res: Response, next: NextFunction): void => { const db = getDb(); const agent = db.findById(req.params.id); if (!agent) { - res.status(404).json({ error: "Agent not found" }); + next(new NotFoundError("Agent")); return; } res.json(agent); }); - // GET /api/agents/:id/health - router.get("/:id/health", async (req: Request, res: Response): Promise => { + router.get("/:id/health", async (req: Request, res: Response, next: NextFunction): Promise => { const db = getDb(); const agent = db.findById(req.params.id); if (!agent) { - res.status(404).json({ error: "Agent not found" }); + next(new NotFoundError("Agent")); return; } @@ -84,28 +82,26 @@ export function createAgentsRouter(options: AgentsRouterOptions = {}): Router { } }); - // POST /api/agents/register - router.post("/register", async (req: Request, res: Response): Promise => { + router.post("/register", async (req: Request, res: Response, next: NextFunction): Promise => { const parse = RegisterAgentSchema.safeParse(req.body); if (!parse.success) { - res.status(400).json({ error: parse.error.flatten() }); + next(new ValidationError("Invalid request body", parse.error.flatten())); return; } - + const data = parse.data; - - // Verify Stellar account exists + try { await horizon.loadAccount(data.stellarPublicKey); } catch (err: any) { if (err?.response?.status === 404) { - res.status(400).json({ error: "StellarAccountNotFound" }); + next(new ValidationError("Stellar account not found")); return; } - res.status(400).json({ error: "Failed to verify Stellar account", details: err.message }); + next(new ValidationError("Failed to verify Stellar account", err.message)); return; } - + const db = getDb(); const agent = { id: data.agentId, @@ -116,41 +112,39 @@ export function createAgentsRouter(options: AgentsRouterOptions = {}): Router { reputationScore: 0, lastSeenAt: new Date().toISOString() }; - + db.upsert(agent); - res.status(201).json(agent); }); - // DELETE /api/agents/:id - router.delete("/:id", (req: Request, res: Response): void => { + router.delete("/:id", (req: Request, res: Response, next: NextFunction): void => { const db = getDb(); const agent = db.findById(req.params.id); if (!agent) { - res.status(404).json({ error: "Agent not found" }); + next(new NotFoundError("Agent")); return; } - + const signature = req.headers["x-signature"] as string; const challenge = req.headers["x-challenge"] as string; - + if (!signature || !challenge) { - res.status(401).json({ error: "Missing challenge or signature" }); + next(new UnauthorizedError("Missing challenge or signature")); return; } - + try { const keypair = Keypair.fromPublicKey(agent.stellarPublicKey); const isValid = keypair.verify(Buffer.from(challenge), Buffer.from(signature, "base64")); if (!isValid) { - res.status(401).json({ error: "Invalid signature" }); + next(new UnauthorizedError("Invalid signature")); return; } } catch (err) { - res.status(401).json({ error: "Invalid signature format" }); + next(new UnauthorizedError("Invalid signature format")); return; } - + db.delete(req.params.id); res.json({ message: "Agent deleted successfully" }); }); diff --git a/backend/src/api/routes/stats.ts b/backend/src/api/routes/stats.ts index 9be32f6..b877a1e 100644 --- a/backend/src/api/routes/stats.ts +++ b/backend/src/api/routes/stats.ts @@ -1,6 +1,7 @@ -import { Router } from 'express'; +import { Router, Request, Response, NextFunction } from 'express'; import { getStats, type DbClient } from '../../db/stats'; import { StatsCache } from '../../utils/statsCache'; +import { AppError } from '../../errors'; export function createStatsRouter(db: DbClient) { const router = Router(); @@ -9,13 +10,12 @@ export function createStatsRouter(db: DbClient) { computeStats: () => getStats(db) }); - router.get('/stats', async (req, res) => { + router.get('/stats', async (req: Request, res: Response, next: NextFunction) => { try { const stats = await cache.get(); return res.status(200).json(stats); } catch (error) { - console.error('Failed to load stats', error); - return res.status(500).json({ error: 'Unable to load stats' }); + next(new AppError('Unable to load stats', 500, 'STATS_LOAD_ERROR')); } }); diff --git a/backend/src/api/routes/tasks.ts b/backend/src/api/routes/tasks.ts index f3eddb7..6ed8be1 100644 --- a/backend/src/api/routes/tasks.ts +++ b/backend/src/api/routes/tasks.ts @@ -1,9 +1,10 @@ -import { Router, Request, Response } from "express"; +import { Router, Request, Response, NextFunction } from "express"; import { z } from "zod"; import { nanoid } from "nanoid"; import { getTaskDb, createTaskDb } from "../../db/tasks"; import { decompose } from "../../coordinator"; import type { Task } from "../../types/task"; +import { ValidationError, NotFoundError, AppError } from "../../errors"; export const tasksRouter = Router(); @@ -21,11 +22,10 @@ const TaskListSchema = z.object({ q: z.string().optional(), }); -// POST /api/tasks -tasksRouter.post("/", (req: Request, res: Response): void => { +tasksRouter.post("/", (req: Request, res: Response, next: NextFunction): void => { const parse = CreateTaskSchema.safeParse(req.body); if (!parse.success) { - res.status(400).json({ error: parse.error.flatten() }); + next(new ValidationError("Invalid request body", parse.error.flatten())); return; } @@ -50,12 +50,11 @@ tasksRouter.post("/", (req: Request, res: Response): void => { res.status(201).json({ taskId: task.id, dagPreview: dag, status: "queued" }); }); -// GET /api/tasks -tasksRouter.get("/", (req: Request, res: Response): void => { +tasksRouter.get("/", (req: Request, res: Response, next: NextFunction): void => { const walletPublicKey = (req.headers["walletpublickey"] as string) ?? ""; const parse = TaskListSchema.safeParse(req.query); if (!parse.success) { - res.status(400).json({ error: parse.error.flatten() }); + next(new ValidationError("Invalid query parameters", parse.error.flatten())); return; } @@ -70,27 +69,25 @@ tasksRouter.get("/", (req: Request, res: Response): void => { res.json({ tasks: tasks.map(t => ({ ...t, dag: JSON.parse(t.dagJson) })), total, page, pageSize }); }); -// GET /api/tasks/:id -tasksRouter.get("/:id", (req: Request, res: Response): void => { +tasksRouter.get("/:id", (req: Request, res: Response, next: NextFunction): void => { const db = createTaskDb(getTaskDb()); const task = db.findById(req.params.id); if (!task) { - res.status(404).json({ error: "Task not found" }); + next(new NotFoundError("Task")); return; } res.json({ ...task, dag: JSON.parse(task.dagJson) }); }); -// DELETE /api/tasks/:id -tasksRouter.delete("/:id", (req: Request, res: Response): void => { +tasksRouter.delete("/:id", (req: Request, res: Response, next: NextFunction): void => { const db = createTaskDb(getTaskDb()); const task = db.findById(req.params.id); if (!task) { - res.status(404).json({ error: "Task not found" }); + next(new NotFoundError("Task")); return; } if (task.status === "running") { - res.status(409).json({ error: "Cannot cancel a running task" }); + next(new AppError("Cannot cancel a running task", 409, "CONFLICT")); return; } db.updateStatus(req.params.id, "cancelled"); diff --git a/backend/src/errors/AppError.ts b/backend/src/errors/AppError.ts new file mode 100644 index 0000000..695532e --- /dev/null +++ b/backend/src/errors/AppError.ts @@ -0,0 +1,16 @@ +export class AppError extends Error { + public readonly statusCode: number; + public readonly code: string; + public readonly isOperational: boolean; + public readonly details?: unknown; + + constructor(message: string, statusCode: number, code: string, details?: unknown) { + super(message); + this.name = "AppError"; + this.statusCode = statusCode; + this.code = code; + this.isOperational = true; + this.details = details; + Object.setPrototypeOf(this, new.target.prototype); + } +} diff --git a/backend/src/errors/NotFoundError.ts b/backend/src/errors/NotFoundError.ts new file mode 100644 index 0000000..bd7ec69 --- /dev/null +++ b/backend/src/errors/NotFoundError.ts @@ -0,0 +1,9 @@ +import { AppError } from "./AppError"; + +export class NotFoundError extends AppError { + constructor(resource: string, id?: string) { + const message = id ? `${resource} not found: ${id}` : `${resource} not found`; + super(message, 404, "NOT_FOUND"); + this.name = "NotFoundError"; + } +} diff --git a/backend/src/errors/RateLimitError.ts b/backend/src/errors/RateLimitError.ts new file mode 100644 index 0000000..d427abe --- /dev/null +++ b/backend/src/errors/RateLimitError.ts @@ -0,0 +1,8 @@ +import { AppError } from "./AppError"; + +export class RateLimitError extends AppError { + constructor(message = "Too many requests") { + super(message, 429, "RATE_LIMITED"); + this.name = "RateLimitError"; + } +} diff --git a/backend/src/errors/UnauthorizedError.ts b/backend/src/errors/UnauthorizedError.ts new file mode 100644 index 0000000..398dd3a --- /dev/null +++ b/backend/src/errors/UnauthorizedError.ts @@ -0,0 +1,8 @@ +import { AppError } from "./AppError"; + +export class UnauthorizedError extends AppError { + constructor(message = "Unauthorized") { + super(message, 401, "UNAUTHORIZED"); + this.name = "UnauthorizedError"; + } +} diff --git a/backend/src/errors/ValidationError.ts b/backend/src/errors/ValidationError.ts new file mode 100644 index 0000000..3dd2758 --- /dev/null +++ b/backend/src/errors/ValidationError.ts @@ -0,0 +1,8 @@ +import { AppError } from "./AppError"; + +export class ValidationError extends AppError { + constructor(message: string, details?: unknown) { + super(message, 400, "VALIDATION_ERROR", details); + this.name = "ValidationError"; + } +} diff --git a/backend/src/errors/index.ts b/backend/src/errors/index.ts new file mode 100644 index 0000000..aacf936 --- /dev/null +++ b/backend/src/errors/index.ts @@ -0,0 +1,5 @@ +export { AppError } from "./AppError"; +export { ValidationError } from "./ValidationError"; +export { NotFoundError } from "./NotFoundError"; +export { UnauthorizedError } from "./UnauthorizedError"; +export { RateLimitError } from "./RateLimitError"; diff --git a/backend/tests/agents.test.ts b/backend/tests/agents.test.ts index 06bf52b..1476e27 100644 --- a/backend/tests/agents.test.ts +++ b/backend/tests/agents.test.ts @@ -1,9 +1,10 @@ -import express from "express"; +import express, { Request, Response, NextFunction } from "express"; import type { AddressInfo } from "net"; import request from "supertest"; import { createAgentsRouter } from "../src/api/routes/agents"; import { AgentRecord, createAgentDb } from "../src/db/agents"; import Database from "better-sqlite3"; +import { AppError } from "../src/errors"; const codingAgent: AgentRecord = { id: "coding-1", @@ -15,6 +16,20 @@ const codingAgent: AgentRecord = { lastSeenAt: new Date().toISOString() }; +function testErrorHandler(err: Error, _req: Request, res: Response, _next: NextFunction): void { + if (err instanceof AppError) { + const body: Record = { + error: { code: err.code, message: err.message }, + }; + if (err.details !== undefined) { + (body.error as Record).details = err.details; + } + res.status(err.statusCode).json(body); + return; + } + res.status(500).json({ error: { code: "INTERNAL_ERROR", message: err.message } }); +} + function createTestApp(initialAgents: AgentRecord[] = [], healthTimeoutMs = 500) { const rawDb = new Database(":memory:"); rawDb.exec(` @@ -35,6 +50,7 @@ function createTestApp(initialAgents: AgentRecord[] = [], healthTimeoutMs = 500) const app = express(); app.use(express.json()); app.use("/api/agents", createAgentsRouter({ db, healthTimeoutMs })); + app.use(testErrorHandler); return app; } @@ -74,7 +90,7 @@ describe("Agents API route", () => { const response = await request(createTestApp()).get("/api/agents/missing"); expect(response.status).toBe(404); - expect(response.body).toEqual({ error: "Agent not found" }); + expect(response.body).toEqual({ error: { code: "NOT_FOUND", message: "Agent not found" } }); }); it("returns healthy status and latency for a reachable agent endpoint", async () => { @@ -121,6 +137,6 @@ describe("Agents API route", () => { const response = await request(createTestApp()).get("/api/agents/missing/health"); expect(response.status).toBe(404); - expect(response.body).toEqual({ error: "Agent not found" }); + expect(response.body).toEqual({ error: { code: "NOT_FOUND", message: "Agent not found" } }); }); }); diff --git a/backend/tests/middleware.test.ts b/backend/tests/middleware.test.ts index ea279d1..a8241e7 100644 --- a/backend/tests/middleware.test.ts +++ b/backend/tests/middleware.test.ts @@ -2,7 +2,6 @@ import { Request, Response, NextFunction } from 'express'; // ── rateLimit ───────────────────────────────────────────────────────────────── -// Re-require between tests to get a fresh module with empty windows map function freshRateLimit() { jest.resetModules(); return require('../src/api/middleware/rateLimit').rateLimitMiddleware as typeof import('../src/api/middleware/rateLimit').rateLimitMiddleware; @@ -51,7 +50,6 @@ describe('rateLimitMiddleware', () => { for (let i = 0; i < 20; i++) { rateLimitMiddleware(makeReq('1.2.3.4'), makeRes() as unknown as Response, next as NextFunction); } - // different IP should still pass const next2 = jest.fn(); rateLimitMiddleware(makeReq('5.6.7.8'), makeRes() as unknown as Response, next2 as NextFunction); expect(next2).toHaveBeenCalled(); @@ -66,7 +64,6 @@ describe('rateLimitMiddleware', () => { rateLimitMiddleware(req, makeRes() as unknown as Response, jest.fn() as NextFunction); } - // Advance past the 60s window jest.advanceTimersByTime(61_000); const next = jest.fn(); @@ -116,15 +113,17 @@ describe('authMiddleware', () => { it('returns 401 for invalid API key', () => { const authMiddleware = freshAuth('key-abc'); - const res = makeRes(); - authMiddleware(makeAuthReq('Bearer wrong-key'), res as unknown as Response, jest.fn() as NextFunction); - expect(res.status).toHaveBeenCalledWith(401); + const next = jest.fn(); + authMiddleware(makeAuthReq('Bearer wrong-key'), makeRes() as unknown as Response, next as NextFunction); + expect(next).toHaveBeenCalledWith(expect.any(Error)); + expect((next.mock.calls[0][0] as any).statusCode).toBe(401); }); it('returns 401 when Authorization header is missing', () => { const authMiddleware = freshAuth('key-abc'); - const res = makeRes(); - authMiddleware(makeAuthReq(), res as unknown as Response, jest.fn() as NextFunction); - expect(res.status).toHaveBeenCalledWith(401); + const next = jest.fn(); + authMiddleware(makeAuthReq(), makeRes() as unknown as Response, next as NextFunction); + expect(next).toHaveBeenCalledWith(expect.any(Error)); + expect((next.mock.calls[0][0] as any).statusCode).toBe(401); }); });