From 7e7486f94687887fcb7ade0cbbdfd15d8ca4dbb4 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 21 Feb 2026 19:35:43 +0100 Subject: [PATCH 1/3] refactor(errors): harden error contract and sanitize unsafe fallbacks --- lib/helpers/AppError.js | 3 +- lib/helpers/errors.js | 9 ++-- lib/helpers/responses.js | 73 ++++++++++++++++++++++++--- modules/core/tests/core.unit.tests.js | 51 ++++++++++++++++++- 4 files changed, 122 insertions(+), 14 deletions(-) diff --git a/lib/helpers/AppError.js b/lib/helpers/AppError.js index 0550e7952..bf865f455 100644 --- a/lib/helpers/AppError.js +++ b/lib/helpers/AppError.js @@ -15,8 +15,7 @@ class AppError extends Error { constructor(message, { details, status, code } = {}) { super(message); // Set HTTP status code - // eslint-disable-next-line no-unused-vars - status = status || 500; + this.status = status || 500; // Set API error code this.code = code || AppErrorCodes.serverError; diff --git a/lib/helpers/errors.js b/lib/helpers/errors.js index 9282e2653..19e048304 100644 --- a/lib/helpers/errors.js +++ b/lib/helpers/errors.js @@ -72,10 +72,9 @@ const getMessageFromErrors = (err) => { }; const cleanMessage = (message) => { - let output = ''; - if (message[message.length - 1] !== '.') output = `${message}.`; - else output = message; - return output; + if (!message || typeof message !== 'string' || !message.trim()) return 'Something went wrong.'; + if (message[message.length - 1] !== '.') return `${message}.`; + return message; }; /** @@ -88,7 +87,7 @@ const getMessage = (err) => { if (err.code) output = getMessageFromCode(err); else if (err.errors) output = getMessageFromErrors(err); else if (err.message) output = err.message; - else output = `error while retrieving the error :o : ${JSON.stringify(err)}`; + else output = 'Something went wrong.'; return cleanMessage(output); }; diff --git a/lib/helpers/responses.js b/lib/helpers/responses.js index d7b0f9fb3..d8abdadc6 100644 --- a/lib/helpers/responses.js +++ b/lib/helpers/responses.js @@ -10,21 +10,82 @@ const success = (res, message) => (data) => { return result; }; +/** + * @desc Validate HTTP status code boundaries + * @param {number} status - Candidate HTTP status code + * @return {boolean} true when status is a valid HTTP code + */ +const isValidHttpStatus = (status) => Number.isInteger(status) && status >= 100 && status <= 599; + +/** + * @desc Resolve HTTP status code from explicit value or error object fallback + * @param {number} status - Explicit status code + * @param {Object} err - Error object potentially containing status values + * @return {number} normalized HTTP status code + */ +const getHttpStatus = (status, err) => { + if (isValidHttpStatus(status)) return status; + if (isValidHttpStatus(err?.status)) return err.status; + if (isValidHttpStatus(err?.statusCode)) return err.statusCode; + if (isValidHttpStatus(err?.code)) return err.code; + return 500; +}; + +/** + * @desc Resolve safe client description text for error payload + * @param {string} description - Explicit description override + * @param {Object} err - Error object + * @return {string} error description + */ +const getDescription = (description, err) => { + if (description) return description; + if (err?.description) return err.description; + if (typeof err?.details === 'string') return err.details; + if (err?.details?.message) return err.details.message; + return ''; +}; + +/** + * @desc Resolve stable domain error code from an error object + * @param {Object} err - Error object + * @return {string} domain error code + */ +const getErrorCode = (err) => { + if (typeof err?.code === 'string' && err.code) return err.code; + return 'SERVER_ERROR'; +}; + +/** + * @desc JSON stringify helper resilient to circular payloads + * @param {Object} value - Value to stringify + * @return {string} safe JSON string + */ +const safeStringify = (value) => { + try { + return JSON.stringify(value); + } catch (_err) { + return JSON.stringify({ message: 'Unserializable error object' }); + } +}; + /** * @desc Function res error * @param {Object} res - Express response object * @param {String} success message * @return {Object} type, message and error */ -const error = (res, code, message, description) => (error) => { +const error = (res, code, message, description) => (error = {}) => { + const status = getHttpStatus(code, error); const result = { type: 'error', - message: message || error.message, - code: code || error.code, - description: description || error.description || error.details || '', - error: JSON.stringify(error), + message: message || error.message || 'Something went wrong.', + code: status, + status, + errorCode: getErrorCode(error), + description: getDescription(description, error), }; - res.status(code || error.code).json(result); + if (process.env.NODE_ENV !== 'production') result.error = safeStringify(error); + res.status(status).json(result); return result; }; diff --git a/modules/core/tests/core.unit.tests.js b/modules/core/tests/core.unit.tests.js index 07f048e3c..291f76158 100644 --- a/modules/core/tests/core.unit.tests.js +++ b/modules/core/tests/core.unit.tests.js @@ -12,6 +12,8 @@ import logger from '../../../lib/services/logger.js'; import mongooseService from '../../../lib/services/mongoose.js'; import expressService from '../../../lib/services/express.js'; import errors from '../../../lib/helpers/errors.js'; +import responses from '../../../lib/helpers/responses.js'; +import AppError from '../../../lib/helpers/AppError.js'; import policy from '../../../lib/middlewares/policy.js'; /** @@ -216,12 +218,59 @@ describe('Core unit tests:', () => { expect(fromMessage).toBe('error1.'); const fromEmpty = errors.getMessage({}); - expect(fromEmpty).toBe('error while retrieving the error :o : {}.'); + expect(fromEmpty).toBe('Something went wrong.'); } catch (err) { console.log(err); expect(err).toBeFalsy(); } }); + + it('should sanitize unknown errors message', () => { + const fromUnknown = errors.getMessage({ random: 'value' }); + expect(fromUnknown).toBe('Something went wrong.'); + }); + }); + + describe('Responses', () => { + it('should return explicit status and domain code in error response', () => { + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + const err = new AppError('Schema validation error', { + status: 422, + code: 'VALIDATION_ERROR', + details: { message: 'First name is required.' }, + }); + + const result = responses.error(mockRes)(err); + + expect(mockRes.status).toHaveBeenCalledWith(422); + expect(result.type).toBe('error'); + expect(result.status).toBe(422); + expect(result.code).toBe(422); + expect(result.errorCode).toBe('VALIDATION_ERROR'); + expect(result.description).toBe('First name is required.'); + }); + + it('should not expose raw error payload in production mode', () => { + const originalNodeEnv = process.env.NODE_ENV; + try { + process.env.NODE_ENV = 'production'; + + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + const result = responses.error(mockRes, 422, 'Schema validation error', 'Invalid payload')({ + details: { internal: 'secret' }, + }); + + expect(result.error).toBeUndefined(); + } finally { + process.env.NODE_ENV = originalNodeEnv; + } + }); }); describe('Config helpers', () => { From 75697825f46e2810c29ad5389e347ce6901168a2 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 21 Feb 2026 19:42:57 +0100 Subject: [PATCH 2/3] fix(errors): address review feedback on response helpers --- lib/helpers/responses.js | 21 +++++++-- modules/core/tests/core.unit.tests.js | 61 +++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/lib/helpers/responses.js b/lib/helpers/responses.js index d8abdadc6..f475494e5 100644 --- a/lib/helpers/responses.js +++ b/lib/helpers/responses.js @@ -41,6 +41,17 @@ const getDescription = (description, err) => { if (description) return description; if (err?.description) return err.description; if (typeof err?.details === 'string') return err.details; + if (Array.isArray(err?.details)) { + const messages = err.details + .map((item) => { + if (!item) return null; + if (typeof item === 'string') return item; + if (typeof item.message === 'string') return item.message; + return null; + }) + .filter(Boolean); + if (messages.length > 0) return messages.join(', '); + } if (err?.details?.message) return err.details.message; return ''; }; @@ -71,11 +82,13 @@ const safeStringify = (value) => { /** * @desc Function res error * @param {Object} res - Express response object - * @param {String} success message - * @return {Object} type, message and error + * @param {number} httpStatus - HTTP status code or candidate error status + * @param {string} message - Error message to send to the client + * @param {string} description - Optional detailed error description + * @return {Object} type, message, code, status, errorCode and description */ -const error = (res, code, message, description) => (error = {}) => { - const status = getHttpStatus(code, error); +const error = (res, httpStatus, message, description) => (error = {}) => { + const status = getHttpStatus(httpStatus, error); const result = { type: 'error', message: message || error.message || 'Something went wrong.', diff --git a/modules/core/tests/core.unit.tests.js b/modules/core/tests/core.unit.tests.js index 291f76158..f2cdfac5e 100644 --- a/modules/core/tests/core.unit.tests.js +++ b/modules/core/tests/core.unit.tests.js @@ -253,6 +253,67 @@ describe('Core unit tests:', () => { expect(result.description).toBe('First name is required.'); }); + it('should use AppError default details array as response description', () => { + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + const err = new AppError('From default details', { + status: 400, + code: 'VALIDATION_ERROR', + }); + + const result = responses.error(mockRes)(err); + + expect(result.description).toBe('From default details'); + }); + + it('should fallback to error statusCode and explicit description field', () => { + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + + const result = responses.error(mockRes, undefined, undefined, undefined)({ + statusCode: 409, + code: 'CONFLICT_ERROR', + description: 'Conflict', + }); + + expect(mockRes.status).toHaveBeenCalledWith(409); + expect(result.status).toBe(409); + expect(result.errorCode).toBe('CONFLICT_ERROR'); + expect(result.description).toBe('Conflict'); + }); + + it('should fallback to numeric error.code as http status when needed', () => { + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + + const result = responses.error(mockRes, undefined, undefined, undefined)({ + code: 401, + }); + + expect(mockRes.status).toHaveBeenCalledWith(401); + expect(result.status).toBe(401); + expect(result.errorCode).toBe('SERVER_ERROR'); + }); + + it('should safely serialize circular error objects in non production', () => { + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + const circular = {}; + circular.self = circular; + + const result = responses.error(mockRes, 500, 'Boom', '')(circular); + + expect(result.error).toContain('Unserializable error object'); + }); + it('should not expose raw error payload in production mode', () => { const originalNodeEnv = process.env.NODE_ENV; try { From 4d33ffa401d5d9b7e8fc12d3d5ab3fb44df4a164 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 21 Feb 2026 19:51:19 +0100 Subject: [PATCH 3/3] test(core): extend response helper coverage branches --- modules/core/tests/core.unit.tests.js | 45 +++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/modules/core/tests/core.unit.tests.js b/modules/core/tests/core.unit.tests.js index f2cdfac5e..0d54acedc 100644 --- a/modules/core/tests/core.unit.tests.js +++ b/modules/core/tests/core.unit.tests.js @@ -232,6 +232,23 @@ describe('Core unit tests:', () => { }); describe('Responses', () => { + it('should return success payload and send HTTP 200', () => { + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + const payload = { ok: true }; + + const result = responses.success(mockRes, 'Done')(payload); + + expect(mockRes.status).toHaveBeenCalledWith(200); + expect(result).toEqual({ + type: 'success', + message: 'Done', + data: payload, + }); + }); + it('should return explicit status and domain code in error response', () => { const mockRes = { status: jest.fn().mockReturnThis(), @@ -268,6 +285,20 @@ describe('Core unit tests:', () => { expect(result.description).toBe('From default details'); }); + it('should ignore invalid entries in details array and keep valid messages', () => { + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + + const result = responses.error(mockRes, 400, undefined, undefined)({ + code: 'VALIDATION_ERROR', + details: [null, { message: 'one' }, {}, 'two'], + }); + + expect(result.description).toBe('one, two'); + }); + it('should fallback to error statusCode and explicit description field', () => { const mockRes = { status: jest.fn().mockReturnThis(), @@ -301,6 +332,20 @@ describe('Core unit tests:', () => { expect(result.errorCode).toBe('SERVER_ERROR'); }); + it('should fallback to 500 and empty description for unknown error shape', () => { + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + + const result = responses.error(mockRes)({}); + + expect(mockRes.status).toHaveBeenCalledWith(500); + expect(result.status).toBe(500); + expect(result.message).toBe('Something went wrong.'); + expect(result.description).toBe(''); + }); + it('should safely serialize circular error objects in non production', () => { const mockRes = { status: jest.fn().mockReturnThis(),