Merge PR #108: task: standardize API error envelope (admin; conflicts auto-resolved -X theirs)

Author: greatest0fallt1me

src/errors/AppError.ts

@@ -0,0 +1,31 @@
+import { ErrorCodes } from "./codes";
+
+export interface AppErrorDetails {
+  [key: string]: unknown;
+}
+
+export class AppError extends Error {
+  public readonly code: string;
+  public readonly status: number;
+  public readonly details?: AppErrorDetails;
+
+  constructor(code: string, message: string, status: number = 500, details?: AppErrorDetails) {
+    super(message);
+    this.name = "AppError";
+    this.code = code;
+    this.status = status;
+    this.details = details;
+  }
+
+  static notFound(message = "Resource not found"): AppError {
+    return new AppError(ErrorCodes.NOT_FOUND, message, 404);
+  }
+
+  static internal(message = "Internal error"): AppError {
+    return new AppError(ErrorCodes.INTERNAL_ERROR, message, 500);
+  }
+
+  static validation(details?: AppErrorDetails): AppError {
+    return new AppError(ErrorCodes.VALIDATION_ERROR, "Validation failed", 400, details);
+  }
+}

src/errors/codes.ts

@@ -0,0 +1,8 @@
+export const ErrorCodes = {
+  INTERNAL_ERROR: "internal_error",
+  NOT_FOUND: "not_found",
+  VALIDATION_ERROR: "validation_error",
+  REQUEST_FAILED: "request_failed",
+} as const;
+
+export type ErrorCode = (typeof ErrorCodes)[keyof typeof ErrorCodes];

src/errors/index.ts

@@ -0,0 +1,4 @@
+export { AppError } from "./AppError";
+export type { AppErrorDetails } from "./AppError";
+export { ErrorCodes } from "./codes";
+export type { ErrorCode } from "./codes";

src/middleware/errorHandler.ts

@@ -1,7 +1,13 @@
 import type { NextFunction, Request, Response } from "express";
 import { ZodError } from "zod";
 import { logger } from "../config/logger";
-import { getRequestId } from "../lib/requestContext";
+import { AppError, ErrorCodes } from "../errors";
+
+function getRequestId(req: Request): string {
+  const id = (req as { id?: unknown }).id;
+  if (id == null) return "";
+  return String(id);
+}
 
 /*
  * Status → error code mapping:
@@ -14,22 +20,39 @@ import { getRequestId } from "../lib/requestContext";
  *   5xx / unknown   → 500  internal_error    (internals never leaked)
  */
 export function errorHandler(err: unknown, req: Request, res: Response, _next: NextFunction) {
-  // getRequestId() works here because the ALS middleware ran before us.
-  // Fall back to req.id (set by pinoHttp) in the unlikely event the store is
-  // not populated (e.g. the error was thrown before the ALS middleware ran).
-  const requestId = getRequestId() ?? (req.id as string | undefined);
+  const requestId = getRequestId(req);
 
-  logger.error(
-    { err, path: req.path, method: req.method, reqId: requestId },
-    "request_failed",
-  );
+  if (err instanceof AppError) {
+    logger.warn({ err, requestId, path: req.path, method: req.method }, err.message);
+    res.status(err.status).json({
+      error: {
+        code: err.code,
+        message: err.message,
+        ...(err.details ? { details: err.details } : {}),
+        requestId,
+      },
+    });
+    return;
+  }
 
-  const status = (err as { status?: number }).status ?? 500;
+  if (err instanceof ZodError) {
+    logger.warn({ err, requestId, path: req.path, method: req.method }, "validation_error");
+    res.status(400).json({
+      error: {
+        code: ErrorCodes.VALIDATION_ERROR,
+        message: "Validation failed",
+        details: err.issues,
+        requestId,
+      },
+    });
+    return;
+  }
 
-  res.status(status).json({
+  logger.error({ err, requestId, path: req.path, method: req.method }, "unhandled_error");
+  res.status(500).json({
     error: {
-      code: status === 500 ? "internal_error" : "request_failed",
-      // Expose the request ID so clients can quote it when reporting issues.
+      code: ErrorCodes.INTERNAL_ERROR,
+      message: "Internal error",
       requestId,
     },
   });

src/routes/markets.ts

@@ -1,5 +1,6 @@
 import { Router } from "express";
 import { listMarkets, getMarketById } from "../services/marketService";
+import { AppError } from "../errors";
 
 export const marketsRouter = Router();
 
@@ -11,8 +12,8 @@ marketsRouter.get("/", async (_req, res, next) => {
 
 marketsRouter.get("/:id", async (req, res, next) => {
   try {
-    const market = await getMarketById(req.params.id as string);
-    if (!market) { res.status(404).json({ error: { code: "not_found" } }); return; }
+    const market = await getMarketById(req.params.id);
+    if (!market) return next(AppError.notFound("Market not found"));
     res.json({ data: market });
   } catch (e) { next(e); }
 });

tests/errorHandler.test.ts

@@ -1,120 +1,116 @@
-jest.mock("../src/config/env", () => ({
-  env: {
-    NODE_ENV: "test",
-    LOG_LEVEL: "silent",
-    PORT: 0,
-    DATABASE_URL: "postgres://mock:5432/db",
-    JWT_SECRET: "abcdefghijklmnopqrstuvwxyz123456",
-    JWT_ISSUER: "test",
-    JWT_AUDIENCE: "test",
-    JWT_TTL_SECONDS: 3600,
-    STELLAR_NETWORK: "testnet",
-    SOROBAN_RPC_URL: "https://soroban.mock",
-    HORIZON_URL: "https://horizon.mock",
-    PREDICTIFY_CONTRACT_ID: "CCYXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX",
-    INDEXER_POLL_INTERVAL_MS: 5000,
-    INDEXER_START_LEDGER: 0,
-  },
-}));
+process.env.DATABASE_URL = "postgres://test:test@localhost:5432/test";
+process.env.JWT_SECRET = "a".repeat(32);
+process.env.SOROBAN_RPC_URL = "https://rpc.testnet.stellar.org";
+process.env.HORIZON_URL = "https://horizon.testnet.stellar.org";
+process.env.PREDICTIFY_CONTRACT_ID = "CABC...";
 
 import request from "supertest";
+import { ZodError, z } from "zod";
 import express from "express";
-import { z } from "zod";
-import { errorHandler } from "../src/middleware/errorHandler";
-
-function buildApp(stub: (req: express.Request, res: express.Response, next: express.NextFunction) => void) {
-  const app = express();
-  app.use(express.json());
-  app.get("/test", stub);
-  app.use(errorHandler);
-  return app;
-}
-
-describe("errorHandler", () => {
-  describe("ZodError", () => {
-    it("returns 400 with validation_error code and details", async () => {
-      const app = buildApp(() => {
-        z.object({ name: z.string().min(1) }).parse({ name: "" });
-      });
-
-      const res = await request(app).get("/test");
-      expect(res.status).toBe(400);
-      expect(res.body).toMatchSnapshot();
-    });
-
-    it("includes field paths and messages in details", async () => {
-      const app = buildApp(() => {
-        z.object({ email: z.string().email(), age: z.number().int().positive() }).parse({ email: "bad", age: -1 });
-      });
+import { AppError, ErrorCodes } from "../src/errors";
+
+describe("AppError", () => {
+  it("creates an error with code, message, status", () => {
+    const err = new AppError("my_code", "my message", 400);
+    expect(err).toBeInstanceOf(Error);
+    expect(err.code).toBe("my_code");
+    expect(err.message).toBe("my message");
+    expect(err.status).toBe(400);
+    expect(err.details).toBeUndefined();
+  });
 
-      const res = await request(app).get("/test");
-      expect(res.status).toBe(400);
-      expect(res.body.error.code).toBe("validation_error");
-      expect(res.body.error.details).toBeInstanceOf(Array);
-      expect(res.body.error.details).toHaveLength(2);
-      expect(res.body.error.details[0]).toMatchObject({ path: ["email"], message: expect.any(String) });
-      expect(res.body.error.details[1]).toMatchObject({ path: ["age"], message: expect.any(String) });
-    });
+  it("creates an error with details", () => {
+    const err = new AppError("my_code", "my message", 422, { field: "name" });
+    expect(err.details).toEqual({ field: "name" });
   });
 
-  describe("4xx with status", () => {
-    it("returns 404 with not_found code", async () => {
-      const app = buildApp((_req, _res, next) => {
-        const err = new Error("not found");
-        (err as any).status = 404;
-        (err as any).code = "not_found";
-        next(err);
-      });
+  it("defaults to 500", () => {
+    const err = new AppError("my_code", "msg");
+    expect(err.status).toBe(500);
+  });
 
-      const res = await request(app).get("/test");
-      expect(res.status).toBe(404);
-      expect(res.body).toEqual({ error: { code: "not_found" } });
+  describe("static factories", () => {
+    it("notFound creates 404", () => {
+      const err = AppError.notFound("X not found");
+      expect(err.code).toBe(ErrorCodes.NOT_FOUND);
+      expect(err.status).toBe(404);
+      expect(err.message).toBe("X not found");
     });
 
-    it("falls back to request_failed when no code is set", async () => {
-      const app = buildApp((_req, _res, next) => {
-        const err = new Error("bad request");
-        (err as any).status = 400;
-        next(err);
-      });
+    it("internal creates 500", () => {
+      const err = AppError.internal("Boom");
+      expect(err.code).toBe(ErrorCodes.INTERNAL_ERROR);
+      expect(err.status).toBe(500);
+      expect(err.message).toBe("Boom");
+    });
 
-      const res = await request(app).get("/test");
-      expect(res.status).toBe(400);
-      expect(res.body).toEqual({ error: { code: "request_failed" } });
+    it("validation creates 400", () => {
+      const err = AppError.validation({ fields: ["email"] });
+      expect(err.code).toBe(ErrorCodes.VALIDATION_ERROR);
+      expect(err.status).toBe(400);
+      expect(err.details).toEqual({ fields: ["email"] });
     });
   });
+});
 
-  describe("500 / unknown", () => {
-    it("hides internals for 500 errors", async () => {
-      const app = buildApp(() => {
-        throw new Error("something went terribly wrong");
-      });
-
-      const res = await request(app).get("/test");
-      expect(res.status).toBe(500);
-      expect(res.body).toEqual({ error: { code: "internal_error" } });
-    });
+describe("GET /api/markets/:id", () => {
+  it("returns 404 with standard envelope for unknown market", async () => {
+    const { createApp } = await import("../src/index");
+    const res = await request(createApp()).get("/api/markets/nonexistent");
+    expect(res.status).toBe(404);
+    expect(res.body.error).toBeDefined();
+    expect(res.body.error.code).toBe("not_found");
+    expect(res.body.error.message).toBe("Market not found");
+    expect(res.body.error.requestId).toEqual(expect.any(String));
+  });
+});
 
-    it("treats sub-400 status as internal", async () => {
-      const app = buildApp((_req, _res, next) => {
-        const err = new Error("weird");
-        (err as any).status = 399;
-        next(err);
-      });
+describe("errorHandler", () => {
+  function createAppWithError(err: unknown): express.Express {
+    const app = express();
+    app.use(express.json());
+    app.get("/error", () => { throw err; });
+    const { errorHandler } = require("../src/middleware/errorHandler");
+    app.use(errorHandler);
+    return app;
+  }
+
+  it("handles AppError with correct envelope", async () => {
+    const app = createAppWithError(new AppError("custom_code", "custom msg", 418));
+    const res = await request(app).get("/error");
+    expect(res.status).toBe(418);
+    expect(res.body.error.code).toBe("custom_code");
+    expect(res.body.error.message).toBe("custom msg");
+    expect(res.body.error.requestId).toEqual(expect.any(String));
+  });
 
-      const res = await request(app).get("/test");
-      expect(res.status).toBe(500);
-      expect(res.body).toEqual({ error: { code: "internal_error" } });
-    });
+  it("handles ZodError with validation envelope", async () => {
+    const schema = z.object({ name: z.string().min(1) });
+    let zodErr: ZodError | null = null;
+    try { schema.parse({ name: "" }); } catch (e) { zodErr = e as ZodError; }
+
+    const app = createAppWithError(zodErr!);
+    const res = await request(app).get("/error");
+    expect(res.status).toBe(400);
+    expect(res.body.error.code).toBe(ErrorCodes.VALIDATION_ERROR);
+    expect(res.body.error.message).toBe("Validation failed");
+    expect(res.body.error.details).toBeInstanceOf(Array);
+    expect(res.body.error.requestId).toEqual(expect.any(String));
+  });
 
-    it("handles non-Error thrown values", async () => {
-      const app = buildApp(() => {
-        throw "string error"; // eslint-disable-line no-throw-literal
-      });
+  it("handles unknown error with 500 envelope", async () => {
+    const app = createAppWithError(new Error("unexpected"));
+    const res = await request(app).get("/error");
+    expect(res.status).toBe(500);
+    expect(res.body.error.code).toBe(ErrorCodes.INTERNAL_ERROR);
+    expect(res.body.error.message).toBe("Internal error");
+    expect(res.body.error.requestId).toEqual(expect.any(String));
+  });
 
-      const res = await request(app).get("/test");
-      expect(res.status).toBe(500);
-      expect(res.body).toEqual({ error: { code: "internal_error" } });
-    });
+  it("does not leak stack traces", async () => {
+    const app = createAppWithError(new Error("hidden"));
+    const res = await request(app).get("/error");
+    expect(res.body.error.stack).toBeUndefined();
+    expect(res.text).not.toContain("Error: hidden");
   });
 });