Production Readiness: Handle invalid JSON and add db test coverage

api-gateway/src/middleware/errorHandler.ts

@@ -36,14 +36,23 @@ export function errorHandler(err: unknown, req: Request, res: Response, _next: N
     return;
   }
 
-  const e = err as Error & { status?: number; code?: string };
+  const e = err as Error & { status?: number; code?: string; type?: string; body?: string };
   if (e?.message === 'Origin not allowed by CORS') {
     res.status(403).json({
       error: { code: 'CORS_FORBIDDEN', message: 'Origin not allowed', requestId },
     });
     return;
   }
 
+  // Handle invalid JSON parsing from express.json()
+  if (e instanceof SyntaxError && e.status === 400 && 'body' in e) {
+    logger.warn('Invalid JSON in request body', { requestId, path: req.path });
+    res.status(400).json({
+      error: { code: 'BAD_REQUEST', message: 'Invalid JSON payload', requestId },
+    });
+    return;
+  }
+
   logger.error('Unhandled error', {
     requestId,
     name: e?.name,

api-gateway/tests/unit/migrate.test.ts

@@ -0,0 +1,116 @@
+import * as fs from 'fs';
+import * as poolModule from '../../src/db/pool';
+import { runMigrations } from '../../src/db/migrate';
+
+jest.mock('fs');
+jest.mock('../../src/db/pool', () => ({
+  getPool: jest.fn(),
+  verifyDatabaseConnection: jest.fn(),
+  closePool: jest.fn(),
+}));
+jest.mock('../../src/config/env', () => {
+  let hasDb = true;
+  return {
+    env: { LINODE_DB_HOST: 'localhost' },
+    get hasDatabase() { return hasDb; },
+    setHasDatabase(val: boolean) { hasDb = val; }
+  };
+});
+
+describe('migrate', () => {
+  let mClient: any;
+  let mPool: any;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mClient = {
+      query: jest.fn().mockResolvedValue({ rows: [] }),
+      release: jest.fn(),
+    };
+    mPool = {
+      connect: jest.fn().mockResolvedValue(mClient),
+      query: jest.fn().mockResolvedValue({ rows: [] }),
+    };
+    (poolModule.getPool as jest.Mock).mockReturnValue(mPool);
+    (poolModule.verifyDatabaseConnection as jest.Mock).mockResolvedValue(true);
+    require('../../src/config/env').setHasDatabase(true);
+    (fs.existsSync as jest.Mock).mockReturnValue(true);
+    (fs.readdirSync as jest.Mock).mockReturnValue([]);
+  });
+
+  it('skips migrations if no database is configured', async () => {
+    require('../../src/config/env').setHasDatabase(false);
+    const result = await runMigrations();
+    expect(result).toEqual({ applied: [], skipped: [] });
+    expect(poolModule.verifyDatabaseConnection).not.toHaveBeenCalled();
+  });
+
+  it('throws an error if database connection verification fails', async () => {
+    (poolModule.verifyDatabaseConnection as jest.Mock).mockResolvedValue(false);
+    await expect(runMigrations()).rejects.toThrow('Cannot connect to database');
+  });
+
+  it('applies new migrations successfully', async () => {
+    (fs.readdirSync as jest.Mock).mockReturnValue(['001_test.sql']);
+    (fs.readFileSync as jest.Mock).mockReturnValue('CREATE TABLE test (id int);');
+    mPool.query.mockImplementation((q: string) => {
+        if(q.includes("SELECT name FROM schema_migrations")) {
+            return Promise.resolve({ rows: [] });
+        }
+        return Promise.resolve({ rows: [] });
+    });
+
+    const result = await runMigrations();
+
+    expect(result.applied).toEqual(['001_test.sql']);
+    expect(result.skipped).toEqual([]);
+
+    expect(mClient.query).toHaveBeenCalledWith('BEGIN');
+    expect(mClient.query).toHaveBeenCalledWith('CREATE TABLE test (id int);');
+    expect(mClient.query).toHaveBeenCalledWith(
+      expect.stringContaining('INSERT INTO schema_migrations'),
+      expect.arrayContaining(['001_test.sql', expect.any(String)])
+    );
+    expect(mClient.query).toHaveBeenCalledWith('COMMIT');
+    expect(mClient.release).toHaveBeenCalled();
+  });
+
+  it('skips already applied migrations', async () => {
+    (fs.readdirSync as jest.Mock).mockReturnValue(['001_test.sql']);
+    mPool.query.mockImplementation((q: string) => {
+        if(q.includes("SELECT name FROM schema_migrations")) {
+            return Promise.resolve({ rows: [{ name: '001_test.sql' }] });
+        }
+        return Promise.resolve({ rows: [] });
+    });
+
+    const result = await runMigrations();
+
+    expect(result.applied).toEqual([]);
+    expect(result.skipped).toEqual(['001_test.sql']);
+    expect(mClient.query).not.toHaveBeenCalledWith('BEGIN');
+  });
+
+  it('rolls back on migration failure', async () => {
+    (fs.readdirSync as jest.Mock).mockReturnValue(['001_test.sql']);
+    (fs.readFileSync as jest.Mock).mockReturnValue('INVALID SQL;');
+    mPool.query.mockImplementation((q: string) => {
+        if(q.includes("SELECT name FROM schema_migrations")) {
+            return Promise.resolve({ rows: [] });
+        }
+        return Promise.resolve({ rows: [] });
+    });
+
+    // Fail the specific migration execution
+    mClient.query.mockImplementation((q: string) => {
+      if (q === 'INVALID SQL;') return Promise.reject(new Error('Syntax Error'));
+      return Promise.resolve({ rows: [] });
+    });
+
+    await expect(runMigrations()).rejects.toThrow('Failed migration 001_test.sql: Syntax Error');
+
+    expect(mClient.query).toHaveBeenCalledWith('BEGIN');
+    expect(mClient.query).toHaveBeenCalledWith('ROLLBACK');
+    expect(mClient.release).toHaveBeenCalled();
+  });
+});

api-gateway/tests/unit/pool.test.ts

@@ -0,0 +1,93 @@
+import { Pool } from 'pg';
+import * as poolModule from '../../src/db/pool';
+
+jest.mock('pg', () => {
+  const mPool = {
+    on: jest.fn(),
+    query: jest.fn(),
+    end: jest.fn(),
+  };
+  return { Pool: jest.fn(() => mPool) };
+});
+
+jest.mock('../../src/config/env', () => {
+  let hasDb = true;
+  return {
+    env: {
+      LINODE_DB_HOST: 'localhost',
+      LINODE_DB_PORT: 5432,
+      LINODE_DB_USER: 'test',
+      LINODE_DB_PASSWORD: 'password',
+      LINODE_DB_NAME: 'testdb',
+      LINODE_DB_SSL: false,
+    },
+    get hasDatabase() { return hasDb; },
+    setHasDatabase(val: boolean) { hasDb = val; }
+  };
+});
+
+describe('db pool', () => {
+  let mPool: any;
+
+  beforeEach(() => {
+    (Pool as unknown as jest.Mock).mockClear();
+    mPool = new Pool();
+    require('../../src/config/env').setHasDatabase(true);
+    // Because Pool module retains the singleton `pool` instance, we have to clear it before each test to test its instantiation
+    // However closePool sets it to null, so we just make sure to call it
+  });
+
+  afterEach(async () => {
+    await poolModule.closePool();
+  });
+
+  it('getPool returns null if hasDatabase is false', () => {
+    require('../../src/config/env').setHasDatabase(false);
+    const pool = poolModule.getPool();
+    expect(pool).toBeNull();
+  });
+
+  it('getPool creates and returns a Pool if hasDatabase is true', () => {
+    (Pool as unknown as jest.Mock).mockClear();
+    const pool = poolModule.getPool();
+    expect(pool).toBeDefined();
+    expect(Pool).toHaveBeenCalledTimes(1);
+
+    // returns cached pool
+    const pool2 = poolModule.getPool();
+    expect(pool2).toBe(pool);
+    expect(Pool).toHaveBeenCalledTimes(1);
+  });
+
+  it('verifyDatabaseConnection returns false if getPool returns null', async () => {
+    require('../../src/config/env').setHasDatabase(false);
+    const result = await poolModule.verifyDatabaseConnection();
+    expect(result).toBe(false);
+    expect(poolModule.isDatabaseHealthy()).toBe(false);
+  });
+
+  it('verifyDatabaseConnection returns true and sets connectionVerified if query succeeds', async () => {
+    mPool.query.mockResolvedValueOnce({ rows: [] });
+    const result = await poolModule.verifyDatabaseConnection();
+    expect(result).toBe(true);
+    expect(poolModule.isDatabaseHealthy()).toBe(true);
+    expect(mPool.query).toHaveBeenCalledWith('SELECT 1');
+  });
+
+  it('verifyDatabaseConnection returns false and unsets connectionVerified if query fails', async () => {
+    mPool.query.mockRejectedValueOnce(new Error('connection failed'));
+    const result = await poolModule.verifyDatabaseConnection();
+    expect(result).toBe(false);
+    expect(poolModule.isDatabaseHealthy()).toBe(false);
+  });
+
+  it('closePool ends the pool and unsets connectionVerified', async () => {
+    mPool.query.mockResolvedValueOnce({ rows: [] });
+    await poolModule.verifyDatabaseConnection();
+    expect(poolModule.isDatabaseHealthy()).toBe(true);
+
+    await poolModule.closePool();
+    expect(mPool.end).toHaveBeenCalled();
+    expect(poolModule.isDatabaseHealthy()).toBe(false);
+  });
+});