feat: global error handler + JSON 404 fallthrough (#18)

Author: CryptoJones

app/middleware/error-handler.js

@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2026 Aaron K. Clark
+"use strict";
+
+const log = require('../config/logger.js');
+
+/**
+ * Global error handler. Mounted AFTER all routes in server.js.
+ *
+ * Two failure modes it cleans up:
+ *
+ * 1. `next(err)` calls from controllers / future middleware land
+ *    here; previously they fell through to Express's default
+ *    handler which renders an HTML error page. We always want JSON.
+ *
+ * 2. Uncaught exceptions inside an async route handler get
+ *    promoted to the error handler by Express ^4.21 automatically
+ *    — but only if the handler uses `async` and lets the promise
+ *    reject. Controllers in this codebase already do their own
+ *    try/catch + log + 500.json(), so this handler is the
+ *    safety net for anything that slips through.
+ *
+ * Body shape (matches the rest of the API):
+ *   { message: 'Error!', requestId: <pino req id, if available> }
+ *
+ * Stack traces are NEVER returned to the client. They land in the
+ * structured log instead, where they're searchable + filtered.
+ */
+function errorHandler(err, req, res, next) {
+    if (res.headersSent) {
+        // Express docs say: delegate to default handler if headers
+        // already sent (avoids "Cannot set headers" cascades).
+        return next(err);
+    }
+
+    // 4xx are client errors and should not log as `error` — they're
+    // expected. Default to 500 if no status is set.
+    const status = Number.isInteger(err && err.status) && err.status >= 400 && err.status < 600
+        ? err.status
+        : 500;
+
+    const logBody = {
+        err: err,
+        status,
+        method: req.method,
+        url: req.originalUrl,
+        requestId: req.id || (req.log && req.log.bindings && req.log.bindings().reqId),
+    };
+    if (status >= 500) {
+        log.error(logBody, 'unhandled error');
+    } else {
+        log.warn(logBody, 'request error');
+    }
+
+    const body = {
+        message: status >= 500 ? 'Error!' : (err.message || 'Bad Request'),
+    };
+    if (logBody.requestId) {
+        body.requestId = logBody.requestId;
+    }
+    return res.status(status).json(body);
+}
+
+/**
+ * 404 fallthrough. Mounted right before the error handler so any
+ * unmatched route emits a structured 404 instead of Express's HTML.
+ */
+function notFound(req, res) {
+    return res.status(404).json({
+        message: 'Not found.',
+        method: req.method,
+        path: req.originalUrl,
+    });
+}
+
+module.exports = { errorHandler, notFound };

server.js

@@ -12,6 +12,7 @@ const pinoHttp = require('pino-http');
 const db = require('./app/config/db.config.js');
 const log = require('./app/config/logger.js');
 const router = require('./app/routers/router.js');
+const { errorHandler, notFound } = require('./app/middleware/error-handler.js');
 
 const app = express();
 
@@ -105,6 +106,11 @@ if (rateLimitMax !== 0) {
 
 app.use('/', router);
 
+// 404 fallthrough + global error handler. Order matters — these
+// must be last so they catch what the router didn't.
+app.use(notFound);
+app.use(errorHandler);
+
 // Listen port — env-configurable. Defaults to 3000 so the API can be
 // started by a non-root user. Bind to 0.0.0.0 for container friendliness.
 const port = parseInt(process.env.PORT, 10) || 3000;

tests/api/error-handler.test.js

@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2026 Aaron K. Clark
+//
+// Tests for the global error handler + 404 fallthrough.
+
+import { describe, test, expect, vi, beforeAll } from 'vitest';
+import request from 'supertest';
+import express from 'express';
+import { errorHandler, notFound } from '../../app/middleware/error-handler.js';
+
+let app;
+
+beforeAll(() => {
+    app = express();
+    app.use(express.json());
+
+    // Routes that intentionally throw / next(err) so we can exercise
+    // the error handler from inside a test, without depending on the
+    // whole router.
+    app.get('/explode/500', (req, res, next) => {
+        next(new Error('boom'));
+    });
+    app.get('/explode/with-status', (req, res, next) => {
+        const err = new Error('I am a teapot');
+        err.status = 418;
+        next(err);
+    });
+    app.get('/explode/leaky', (req, res, next) => {
+        // Simulates a Sequelize-style error whose .message would leak
+        // a hostname or stack frame if we passed it straight through.
+        const err = new Error('SequelizeConnectionRefusedError: connect ECONNREFUSED 10.0.0.42:5432');
+        err.status = 500;
+        next(err);
+    });
+    app.use(notFound);
+    app.use(errorHandler);
+});
+
+describe('global error handler', () => {
+    test('500 errors return JSON {message: "Error!"} not HTML', async () => {
+        const res = await request(app).get('/explode/500');
+        expect(res.status).toBe(500);
+        expect(res.headers['content-type']).toMatch(/application\/json/);
+        expect(res.body.message).toBe('Error!');
+    });
+
+    test('500 errors never leak the original message (no stack info)', async () => {
+        const res = await request(app).get('/explode/leaky');
+        const text = JSON.stringify(res.body);
+        expect(text).not.toMatch(/ECONNREFUSED/);
+        expect(text).not.toMatch(/10\.0\.0\.42/);
+        expect(text).not.toMatch(/Sequelize/);
+    });
+
+    test('honors a numeric err.status in 4xx range', async () => {
+        const res = await request(app).get('/explode/with-status');
+        expect(res.status).toBe(418);
+        // For 4xx the message goes through (it's a client error, not a server one)
+        expect(res.body.message).toBe('I am a teapot');
+    });
+});
+
+describe('404 fallthrough', () => {
+    test('unmatched route returns JSON 404 (not HTML)', async () => {
+        const res = await request(app).get('/no/such/path');
+        expect(res.status).toBe(404);
+        expect(res.headers['content-type']).toMatch(/application\/json/);
+        expect(res.body.message).toMatch(/not found/i);
+        expect(res.body.path).toBe('/no/such/path');
+        expect(res.body.method).toBe('GET');
+    });
+
+    test('unmatched method on a known path returns 404 (Express convention)', async () => {
+        // We don't have an OPTIONS handler so this should fall to notFound.
+        const res = await request(app).post('/explode/500');
+        expect(res.status).toBe(404);
+    });
+});