ci: Bump version to 0.1.13 [skip ci]

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "flyer-crawler",
-  "version": "0.1.10",
+  "version": "0.1.13",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "flyer-crawler",
-      "version": "0.1.10",
+      "version": "0.1.13",
       "dependencies": {
         "@bull-board/api": "^6.14.2",
         "@bull-board/express": "^6.14.2",

package.json

@@ -1,7 +1,7 @@
 {
   "name": "flyer-crawler",
   "private": true,
-  "version": "0.1.10",
+  "version": "0.1.13",
   "type": "module",
   "scripts": {
     "dev": "concurrently \"npm:start:dev\" \"vite\"",

src/middleware/errorHandler.test.ts

@@ -109,7 +109,10 @@ describe('errorHandler Middleware', () => {
   it('should return a generic 500 error for a standard Error object', async () => {
     const response = await supertest(app).get('/generic-error');
     expect(response.status).toBe(500);
-    expect(response.body).toEqual({ message: 'A generic server error occurred.' });
+    // In test/dev, we now expect a stack trace for 5xx errors.
+    expect(response.body.message).toBe('A generic server error occurred.');
+    expect(response.body.stack).toBeDefined();
+    expect(response.body.errorId).toEqual(expect.any(String));
     expect(mockLogger.error).toHaveBeenCalledWith(
       expect.objectContaining({
         err: expect.any(Error),
@@ -133,15 +136,11 @@ describe('errorHandler Middleware', () => {
     expect(mockLogger.warn).toHaveBeenCalledWith(
       {
         err: expect.any(Error),
-        validationErrors: undefined,
         statusCode: 404,
       },
       'Client Error on GET /http-error-404: Resource not found',
     );
-    expect(consoleErrorSpy).toHaveBeenCalledWith(
-      expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
-      expect.any(Error),
-    );
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
   });
 
   it('should handle a NotFoundError with a 404 status', async () => {
@@ -153,15 +152,11 @@ describe('errorHandler Middleware', () => {
     expect(mockLogger.warn).toHaveBeenCalledWith(
       {
         err: expect.any(NotFoundError),
-        validationErrors: undefined,
         statusCode: 404,
       },
       'Client Error on GET /not-found-error: Specific resource missing',
     );
-    expect(consoleErrorSpy).toHaveBeenCalledWith(
-      expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
-      expect.any(NotFoundError),
-    );
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
   });
 
   it('should handle a ForeignKeyConstraintError with a 400 status and the specific error message', async () => {
@@ -173,15 +168,11 @@ describe('errorHandler Middleware', () => {
     expect(mockLogger.warn).toHaveBeenCalledWith(
       {
         err: expect.any(ForeignKeyConstraintError),
-        validationErrors: undefined,
         statusCode: 400,
       },
       'Client Error on GET /fk-error: The referenced item does not exist.',
     );
-    expect(consoleErrorSpy).toHaveBeenCalledWith(
-      expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
-      expect.any(ForeignKeyConstraintError),
-    );
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
   });
 
   it('should handle a UniqueConstraintError with a 409 status and the specific error message', async () => {
@@ -193,15 +184,11 @@ describe('errorHandler Middleware', () => {
     expect(mockLogger.warn).toHaveBeenCalledWith(
       {
         err: expect.any(UniqueConstraintError),
-        validationErrors: undefined,
         statusCode: 409,
       },
       'Client Error on GET /unique-error: This item already exists.',
     );
-    expect(consoleErrorSpy).toHaveBeenCalledWith(
-      expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
-      expect.any(UniqueConstraintError),
-    );
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
   });
 
   it('should handle a ValidationError with a 400 status and include the validation errors array', async () => {
@@ -222,17 +209,17 @@ describe('errorHandler Middleware', () => {
       },
       'Client Error on GET /validation-error: Input validation failed',
     );
-    expect(consoleErrorSpy).toHaveBeenCalledWith(
-      expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
-      expect.any(ValidationError),
-    );
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
   });
 
   it('should handle a DatabaseError with a 500 status and a generic message', async () => {
     const response = await supertest(app).get('/db-error-500');
 
     expect(response.status).toBe(500);
-    expect(response.body).toEqual({ message: 'A database connection issue occurred.' });
+    // In test/dev, we now expect a stack trace for 5xx errors.
+    expect(response.body.message).toBe('A database connection issue occurred.');
+    expect(response.body.stack).toBeDefined();
+    expect(response.body.errorId).toEqual(expect.any(String));
     expect(mockLogger.error).toHaveBeenCalledWith(
       expect.objectContaining({
         err: expect.any(DatabaseError),
@@ -252,8 +239,14 @@ describe('errorHandler Middleware', () => {
 
     expect(response.status).toBe(401);
     expect(response.body).toEqual({ message: 'Invalid Token' });
-    // 4xx errors log as warn
-    expect(mockLogger.warn).toHaveBeenCalled();
+    expect(mockLogger.warn).toHaveBeenCalledWith(
+      {
+        err: expect.any(Error),
+        statusCode: 401,
+      },
+      'Client Error on GET /unauthorized-error-no-status: Invalid Token',
+    );
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
   });
 
   it('should handle an UnauthorizedError with explicit status', async () => {
@@ -261,6 +254,14 @@ describe('errorHandler Middleware', () => {
 
     expect(response.status).toBe(401);
     expect(response.body).toEqual({ message: 'Invalid Token' });
+    expect(mockLogger.warn).toHaveBeenCalledWith(
+      {
+        err: expect.any(Error),
+        statusCode: 401,
+      },
+      'Client Error on GET /unauthorized-error-with-status: Invalid Token',
+    );
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
   });
 
   it('should call next(err) if headers have already been sent', () => {
@@ -305,6 +306,7 @@ describe('errorHandler Middleware', () => {
       expect(response.body.message).toMatch(
         /An unexpected server error occurred. Please reference error ID: \w+/,
       );
+      expect(response.body.stack).toBeUndefined();
     });
 
     it('should return the actual error message for client errors (4xx) in production', async () => {

src/middleware/errorHandler.ts

@@ -1,5 +1,6 @@
 // src/middleware/errorHandler.ts
 import { Request, Response, NextFunction } from 'express';
+import crypto from 'crypto';
 import { ZodError } from 'zod';
 import {
   ForeignKeyConstraintError,
@@ -24,45 +25,73 @@ export const errorHandler = (err: Error, req: Request, res: Response, next: Next
   // Use the request-scoped logger if available, otherwise fall back to the global logger.
   const log = req.log || logger;
 
-  // --- Handle Zod Validation Errors ---
+  // --- Handle Zod Validation Errors (from validateRequest middleware) ---
   if (err instanceof ZodError) {
-    log.warn({ err: err.flatten() }, 'Request validation failed');
-    return res.status(400).json({
-      message: 'The request data is invalid.',
-      errors: err.issues.map((e) => ({ path: e.path, message: e.message })),
-    });
+    const statusCode = 400;
+    const message = 'The request data is invalid.';
+    const errors = err.issues.map((e) => ({ path: e.path, message: e.message }));
+    log.warn({ err, validationErrors: errors, statusCode }, `Client Error on ${req.method} ${req.path}: ${message}`);
+    return res.status(statusCode).json({ message, errors });
   }
 
   // --- Handle Custom Operational Errors ---
   if (err instanceof NotFoundError) {
-    log.info({ err }, 'Resource not found');
-    return res.status(404).json({ message: err.message });
+    const statusCode = 404;
+    log.warn({ err, statusCode }, `Client Error on ${req.method} ${req.path}: ${err.message}`);
+    return res.status(statusCode).json({ message: err.message });
   }
 
   if (err instanceof ValidationError) {
-    log.warn({ err }, 'Validation error occurred');
-    return res.status(400).json({ message: err.message, errors: err.validationErrors });
+    const statusCode = 400;
+    log.warn(
+      { err, validationErrors: err.validationErrors, statusCode },
+      `Client Error on ${req.method} ${req.path}: ${err.message}`,
+    );
+    return res.status(statusCode).json({ message: err.message, errors: err.validationErrors });
   }
 
   if (err instanceof UniqueConstraintError) {
-    log.warn({ err }, 'Constraint error occurred');
-    return res.status(409).json({ message: err.message }); // Use 409 Conflict for unique constraints
+    const statusCode = 409;
+    log.warn({ err, statusCode }, `Client Error on ${req.method} ${req.path}: ${err.message}`);
+    return res.status(statusCode).json({ message: err.message }); // Use 409 Conflict for unique constraints
   }
 
   if (err instanceof ForeignKeyConstraintError) {
-    log.warn({ err }, 'Foreign key constraint violation');
-    return res.status(400).json({ message: err.message });
+    const statusCode = 400;
+    log.warn({ err, statusCode }, `Client Error on ${req.method} ${req.path}: ${err.message}`);
+    return res.status(statusCode).json({ message: err.message });
   }
 
-  // --- Handle Generic Errors ---
-  // Log the full error object for debugging. The pino logger will handle redaction.
-  log.error({ err }, 'An unhandled error occurred in an Express route');
+  // --- Handle Generic Client Errors (e.g., from express-jwt, or manual status setting) ---
+  const status = (err as any).status || (err as any).statusCode;
+  if (status && status >= 400 && status < 500) {
+    log.warn({ err, statusCode: status }, `Client Error on ${req.method} ${req.path}: ${err.message}`);
+    return res.status(status).json({ message: err.message });
+  }
+
+  // --- Handle All Other (500-level) Errors ---
+  const errorId = crypto.randomBytes(4).toString('hex');
+  log.error(
+    {
+      err,
+      errorId,
+      req: { method: req.method, url: req.url, headers: req.headers, body: req.body },
+    },
+    `Unhandled API Error (ID: ${errorId})`,
+  );
+
+  // Also log to console in test environment for visibility in test runners
+  if (process.env.NODE_ENV === 'test') {
+    console.error(`--- [TEST] UNHANDLED ERROR (ID: ${errorId}) ---`, err);
+  }
 
   // In production, send a generic message to avoid leaking implementation details.
   if (process.env.NODE_ENV === 'production') {
-    return res.status(500).json({ message: 'An internal server error occurred.' });
+    return res.status(500).json({
+      message: `An unexpected server error occurred. Please reference error ID: ${errorId}`,
+    });
   }
 
-  // In development, send more details for easier debugging.
-  return res.status(500).json({ message: err.message, stack: err.stack });
+  // In non-production environments (dev, test, etc.), send more details for easier debugging.
+  return res.status(500).json({ message: err.message, stack: err.stack, errorId });
 };

src/routes/deals.routes.test.ts

@@ -16,6 +16,7 @@ vi.mock('../services/db/deals.db', () => ({
 // Import the router and mocked repo AFTER all mocks are defined.
 import dealsRouter from './deals.routes';
 import { dealsRepo } from '../services/db/deals.db';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // Mock the logger to keep test output clean
 vi.mock('../services/logger.server', async () => ({

src/routes/flyer.routes.test.ts

@@ -20,6 +20,7 @@ vi.mock('../services/db/index.db', () => ({
 // Import the router and mocked DB AFTER all mocks are defined.
 import flyerRouter from './flyer.routes';
 import * as db from '../services/db/index.db';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // Mock the logger to keep test output clean
 vi.mock('../services/logger.server', async () => ({

src/routes/passport.routes.test.ts

@@ -73,7 +73,7 @@ vi.mock('../services/db/index.db', () => ({
 
 const mockedDb = db as Mocked<typeof db>;
 
-vi.mock('../services/logger.server', () => ({
+vi.mock('../services/logger.server', async () => ({
   // Use async import to avoid hoisting issues with mockLogger
   // Note: We need to await the import inside the factory
   logger: (await import('../tests/utils/mockLogger')).mockLogger,

src/routes/personalization.routes.test.ts

@@ -20,6 +20,7 @@ vi.mock('../services/db/index.db', () => ({
 // Import the router and mocked DB AFTER all mocks are defined.
 import personalizationRouter from './personalization.routes';
 import * as db from '../services/db/index.db';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // Mock the logger to keep test output clean
 vi.mock('../services/logger.server', async () => ({

src/routes/recipe.routes.test.ts

@@ -19,6 +19,7 @@ vi.mock('../services/db/index.db', () => ({
 // Import the router and mocked DB AFTER all mocks are defined.
 import recipeRouter from './recipe.routes';
 import * as db from '../services/db/index.db';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // Mock the logger to keep test output clean
 vi.mock('../services/logger.server', async () => ({

src/routes/stats.routes.test.ts

@@ -13,6 +13,7 @@ vi.mock('../services/db/index.db', () => ({
 // Import the router and mocked DB AFTER all mocks are defined.
 import statsRouter from './stats.routes';
 import * as db from '../services/db/index.db';
+import { mockLogger } from '../tests/utils/mockLogger';
 
 // Mock the logger to keep test output clean
 vi.mock('../services/logger.server', async () => ({