diff --git a/dashcaddy-api/__tests__/error-handler.test.js b/dashcaddy-api/__tests__/error-handler.test.js index 1179c3b..5a742fd 100644 --- a/dashcaddy-api/__tests__/error-handler.test.js +++ b/dashcaddy-api/__tests__/error-handler.test.js @@ -1,8 +1,18 @@ -jest.mock('../error-logger', () => ({ - logError: jest.fn(), +// Mock the unified logging module so we can verify logError is called +// without writing to the actual error.log file +jest.mock('../src/utils/logging', () => ({ + logError: jest.fn().mockResolvedValue(), + safeErrorMessage: jest.fn((err) => { + if (!err) return 'An internal error occurred'; + return err.message || String(err); + }), + createLogger: jest.fn(() => ({ + info: jest.fn(), warn: jest.fn(), error: jest.fn(), debug: jest.fn() + })), + LOG_LEVELS: { debug: 0, info: 1, warn: 2, error: 3 } })); -const { asyncHandler, errorMiddleware, notFoundHandler } = require('../error-handler'); +const { errorMiddleware, notFoundHandler } = require('../error-handler'); const { AppError, ValidationError, @@ -30,23 +40,6 @@ describe('Error Handler', () => { next = jest.fn(); }); - describe('asyncHandler', () => { - it('calls the wrapped function', async () => { - const fn = jest.fn().mockResolvedValue(); - const wrapped = asyncHandler(fn); - await wrapped(req, res, next); - expect(fn).toHaveBeenCalledWith(req, res, next); - }); - - it('calls next(err) on rejected promise', async () => { - const error = new Error('async fail'); - const fn = jest.fn().mockRejectedValue(error); - const wrapped = asyncHandler(fn); - await wrapped(req, res, next); - expect(next).toHaveBeenCalledWith(error); - }); - }); - describe('errorMiddleware', () => { it('returns 400 for ValidationError', () => { const err = new ValidationError('bad input', 'email'); diff --git a/dashcaddy-api/__tests__/routes/services.routes.test.js b/dashcaddy-api/__tests__/routes/services.routes.test.js index 5506a47..339bb90 100644 --- a/dashcaddy-api/__tests__/routes/services.routes.test.js +++ b/dashcaddy-api/__tests__/routes/services.routes.test.js @@ -34,7 +34,7 @@ jest.mock('../../pagination', () => ({ parsePaginationParams: jest.fn(() => null), })); -jest.mock('../../response-helpers', () => ({ +jest.mock('../../src/utils/responses', () => ({ success: jest.fn((res, data, statusCode = 200) => { return res.status(statusCode).json({ success: true, ...data }); }), diff --git a/dashcaddy-api/error-handler.js b/dashcaddy-api/error-handler.js index 2e311a2..811920a 100644 --- a/dashcaddy-api/error-handler.js +++ b/dashcaddy-api/error-handler.js @@ -1,66 +1,70 @@ /** * DashCaddy Error Handler Middleware * Centralizes error handling logic to eliminate duplicate catch blocks + * + * Logging: this middleware uses the unified logError from src/utils/logging.js + * (same one src/app.js uses), so all errors go to one log file. The legacy + * ./error-logger.js and its ./error.log file have been retired. */ +const path = require('path'); const { AppError } = require('./errors'); -const { logError } = require('./error-logger'); +const { LIMITS } = require('./constants'); +const { logError: unifiedLogError, safeErrorMessage } = require('./src/utils/logging'); -/** - * Async route handler wrapper - * Automatically catches errors and passes to error middleware - * Usage: app.get('/route', asyncHandler(async (req, res) => { ... })) - */ -function asyncHandler(fn) { - return (req, res, next) => { - Promise.resolve(fn(req, res, next)).catch(next); - }; -} +const ERROR_LOG_FILE = path.join(__dirname, 'error.log'); +const MAX_ERROR_LOG_SIZE = LIMITS.ERROR_LOG_SIZE; /** * Global error handling middleware * MUST be registered after all routes in server.js */ function errorMiddleware(err, req, res, next) { - // Log all errors with request context - logError(req.path, err, { - method: req.method, - ip: req.ip, - userId: req.user?.id, - body: req.body - }); + // Log all errors with request context (unified, same file the rest of the app uses) + unifiedLogError( + ERROR_LOG_FILE, + MAX_ERROR_LOG_SIZE, + req.path, + err, + { + method: req.method, + ip: req.ip, + userId: req.user?.id, + body: req.body + } + ).catch(e => console.error('Failed to write to error log:', e.message)); // Determine if this is an operational error (AppError) or programming error const isOperational = err.isOperational || err instanceof AppError; - + // Status code const statusCode = err.statusCode || 500; - + // Error code (DC-XXX format) const code = err.code || `DC-${statusCode}`; - + // Build response const response = { success: false, - error: isOperational ? err.message : 'Internal server error', + error: isOperational ? safeErrorMessage(err) : 'Internal server error', code }; - + // Add optional fields if present if (err.requiresTotp) response.requiresTotp = true; if (err.retryAfter) response.retryAfter = err.retryAfter; if (err.field) response.field = err.field; if (err.resource) response.resource = err.resource; if (err.details && Object.keys(err.details).length > 0) response.details = err.details; - + // Development mode: include stack trace if (process.env.NODE_ENV === 'development') { response.stack = err.stack; } - + // Send response res.status(statusCode).json(response); - + // For non-operational errors, log as fatal if (!isOperational) { console.error('FATAL: Non-operational error detected', { @@ -81,7 +85,6 @@ function notFoundHandler(req, res, next) { } module.exports = { - asyncHandler, errorMiddleware, notFoundHandler }; diff --git a/dashcaddy-api/error-logger.js b/dashcaddy-api/error-logger.js deleted file mode 100644 index e35d337..0000000 --- a/dashcaddy-api/error-logger.js +++ /dev/null @@ -1,135 +0,0 @@ -// Error Logger Utility -// Centralized error logging with rotation and request context tracking - -const fsp = require('fs').promises; -const path = require('path'); -const { LIMITS } = require('./constants'); - -const ERROR_LOG_FILE = path.join(__dirname, 'error.log'); -const MAX_ERROR_LOG_SIZE = LIMITS.ERROR_LOG_SIZE; - -/** - * Check if file exists - */ -async function exists(filepath) { - try { - await fsp.access(filepath); - return true; - } catch { - return false; - } -} - -/** - * Log error with context and rotation - * @param {string} context - Where the error occurred - * @param {Error|string} error - The error to log - * @param {Object} additionalInfo - Additional context (req, etc.) - */ -async function logError(context, error, additionalInfo = {}) { - const timestamp = new Date().toISOString(); - - // Extract request context if a request object is provided - const requestContext = extractRequestContext(additionalInfo.req); - if (additionalInfo.req) { - delete additionalInfo.req; // Remove req to avoid circular refs - } - - const logEntry = { - timestamp, - context, - ...requestContext, - error: { - message: error.message || error, - stack: error.stack, - code: error.code - }, - ...additionalInfo - }; - - // Format log line with request context - const contextInfo = Object.keys(requestContext).length > 0 - ? `\nRequest Context: ${JSON.stringify(requestContext, null, 2)}` - : ''; - const logLine = `[${timestamp}] ${context}: ${error.message || error}\n${error.stack || ''}${contextInfo}\nAdditional Info: ${JSON.stringify(additionalInfo, null, 2)}\n${'='.repeat(80)}\n`; - - try { - // Rotate log if it exceeds max size - await rotateLogIfNeeded(); - await fsp.appendFile(ERROR_LOG_FILE, logLine); - } catch (e) { - console.error('Failed to write to error log', e.message); - } -} - -/** - * Extract request context from Express request object - */ -function extractRequestContext(req) { - if (!req) return {}; - - const clientIP = req.ip || req.socket?.remoteAddress || ''; - - return { - requestId: req.id, - ip: clientIP, - userAgent: req.get('user-agent'), - method: req.method, - path: req.path - }; -} - -/** - * Rotate log file if it exceeds max size - */ -async function rotateLogIfNeeded() { - try { - const stats = await fsp.stat(ERROR_LOG_FILE); - if (stats.size > MAX_ERROR_LOG_SIZE) { - const rotated = ERROR_LOG_FILE + '.1'; - if (await exists(rotated)) { - await fsp.unlink(rotated); - } - await fsp.rename(ERROR_LOG_FILE, rotated); - } - } catch (_) { - // File may not exist yet, that's fine - } -} - -/** - * Return a safe error message to the client without leaking internals - */ -function safeErrorMessage(error) { - const msg = error.message || String(error); - - // Detect port conflict errors from Docker - const portMatch = msg.match(/exposing port TCP [^:]*:(\d+)/); - if (portMatch || msg.includes('port is already allocated') || msg.includes('ports are not available')) { - const port = portMatch ? portMatch[1] : 'requested'; - return `Port ${port} is already in use. Please choose a different port or stop the conflicting service.`; - } - - // Detect container not found errors - if (msg.includes('No such container')) { - return 'Container not found'; - } - - // Detect network errors - if (msg.includes('ECONNREFUSED') || msg.includes('ETIMEDOUT')) { - return 'Service unavailable'; - } - - // Generic safe message for unknown errors - if (process.env.NODE_ENV === 'production') { - return 'An error occurred. Please try again or contact support.'; - } - - // In development, show the actual error - return msg; -} - -module.exports = { - logError, - safeErrorMessage -}; diff --git a/dashcaddy-api/package.json b/dashcaddy-api/package.json index b60fc6c..1db5bf8 100644 --- a/dashcaddy-api/package.json +++ b/dashcaddy-api/package.json @@ -1,6 +1,6 @@ { "name": "dashcaddy-api", - "version": "1.13.1", + "version": "1.13.2", "description": "DashCaddy API server - Dashboard backend for Docker, Caddy & DNS management", "main": "server.js", "scripts": { diff --git a/dashcaddy-api/response-helpers.js b/dashcaddy-api/response-helpers.js deleted file mode 100644 index 5f2e276..0000000 --- a/dashcaddy-api/response-helpers.js +++ /dev/null @@ -1,114 +0,0 @@ -// Response Helpers -// Standardize API response format across all routes - -const { HTTP_STATUS } = require('./constants'); - -/** - * Success response with data - */ -function success(res, data, statusCode = HTTP_STATUS.OK) { - return res.status(statusCode).json({ - success: true, - ...data - }); -} - -/** - * Success response with message - */ -function successMessage(res, message, statusCode = HTTP_STATUS.OK) { - return res.status(statusCode).json({ - success: true, - message - }); -} - -/** - * Created response (201) - */ -function created(res, data) { - return res.status(HTTP_STATUS.CREATED).json({ - success: true, - ...data - }); -} - -/** - * No content response (204) - */ -function noContent(res) { - return res.status(HTTP_STATUS.NO_CONTENT).send(); -} - -/** - * Error response - */ -function error(res, message, statusCode = HTTP_STATUS.INTERNAL_ERROR) { - return res.status(statusCode).json({ - success: false, - error: message - }); -} - -/** - * Validation error response (400) - */ -function validationError(res, message) { - return res.status(HTTP_STATUS.BAD_REQUEST).json({ - success: false, - error: message - }); -} - -/** - * Unauthorized response (401) - */ -function unauthorized(res, message = 'Unauthorized') { - return res.status(HTTP_STATUS.UNAUTHORIZED).json({ - success: false, - error: message - }); -} - -/** - * Forbidden response (403) - */ -function forbidden(res, message = 'Forbidden') { - return res.status(HTTP_STATUS.FORBIDDEN).json({ - success: false, - error: message - }); -} - -/** - * Not found response (404) - */ -function notFound(res, message = 'Not found') { - return res.status(HTTP_STATUS.NOT_FOUND).json({ - success: false, - error: message - }); -} - -/** - * Conflict response (409) - */ -function conflict(res, message) { - return res.status(HTTP_STATUS.CONFLICT).json({ - success: false, - error: message - }); -} - -module.exports = { - success, - successMessage, - created, - noContent, - error, - validationError, - unauthorized, - forbidden, - notFound, - conflict -}; diff --git a/dashcaddy-api/routes/auto-restart.js b/dashcaddy-api/routes/auto-restart.js index 0357548..e3b246b 100644 --- a/dashcaddy-api/routes/auto-restart.js +++ b/dashcaddy-api/routes/auto-restart.js @@ -8,7 +8,7 @@ */ const express = require('express'); -const { success } = require('../response-helpers'); +const { success } = require('../src/utils/responses'); const { ValidationError, NotFoundError } = require('../errors'); /** diff --git a/dashcaddy-api/routes/backups.js b/dashcaddy-api/routes/backups.js index a2e7a00..b1ab149 100644 --- a/dashcaddy-api/routes/backups.js +++ b/dashcaddy-api/routes/backups.js @@ -1,5 +1,5 @@ const express = require('express'); -const { success } = require('../response-helpers'); +const { success } = require('../src/utils/responses'); const fs = require('fs'); const path = require('path'); diff --git a/dashcaddy-api/routes/config-drift.js b/dashcaddy-api/routes/config-drift.js index e779004..52e6c29 100644 --- a/dashcaddy-api/routes/config-drift.js +++ b/dashcaddy-api/routes/config-drift.js @@ -8,7 +8,7 @@ */ const express = require('express'); -const { success } = require('../response-helpers'); +const { success } = require('../src/utils/responses'); const { ValidationError, NotFoundError } = require('../errors'); /** diff --git a/dashcaddy-api/routes/containers.js b/dashcaddy-api/routes/containers.js index 1b1a700..e4cef15 100644 --- a/dashcaddy-api/routes/containers.js +++ b/dashcaddy-api/routes/containers.js @@ -2,7 +2,7 @@ const express = require('express'); const { DOCKER } = require('../constants'); const { paginate, parsePaginationParams } = require('../pagination'); const { NotFoundError } = require('../errors'); -const { success } = require('../response-helpers'); +const { success } = require('../src/utils/responses'); /** * Containers route factory diff --git a/dashcaddy-api/routes/credentials.js b/dashcaddy-api/routes/credentials.js index f042c11..0baff54 100644 --- a/dashcaddy-api/routes/credentials.js +++ b/dashcaddy-api/routes/credentials.js @@ -1,5 +1,5 @@ const express = require('express'); -const { success, error: errorResponse } = require('../response-helpers'); +const { success, error: errorResponse } = require('../src/utils/responses'); /** * Credentials routes factory diff --git a/dashcaddy-api/routes/dependencies.js b/dashcaddy-api/routes/dependencies.js index 11629dd..b5c12a7 100644 --- a/dashcaddy-api/routes/dependencies.js +++ b/dashcaddy-api/routes/dependencies.js @@ -15,7 +15,7 @@ */ const express = require('express'); -const { success, error: errorResponse } = require('../response-helpers'); +const { success, error: errorResponse } = require('../src/utils/responses'); const { NotFoundError, ValidationError } = require('../errors'); /** diff --git a/dashcaddy-api/routes/dns.js b/dashcaddy-api/routes/dns.js index 2a8ef7b..fe4512b 100644 --- a/dashcaddy-api/routes/dns.js +++ b/dashcaddy-api/routes/dns.js @@ -4,7 +4,7 @@ const fsp = require('fs').promises; const validatorLib = require('validator'); const { APP, TIMEOUTS, CADDY, DNS_RECORD_TYPES, REGEX, SESSION_TTL } = require('../constants'); const { exists } = require('../fs-helpers'); -const { success, error: errorResponse } = require('../response-helpers'); +const { success, error: errorResponse } = require('../src/utils/responses'); const { ValidationError, AuthenticationError, NotFoundError } = require('../errors'); /** diff --git a/dashcaddy-api/routes/docker-resources.js b/dashcaddy-api/routes/docker-resources.js index 8abe317..aa68cd7 100644 --- a/dashcaddy-api/routes/docker-resources.js +++ b/dashcaddy-api/routes/docker-resources.js @@ -1,5 +1,5 @@ const express = require('express'); -const { success } = require('../response-helpers'); +const { success } = require('../src/utils/responses'); const { ValidationError } = require('../errors'); /** diff --git a/dashcaddy-api/routes/errorlogs.js b/dashcaddy-api/routes/errorlogs.js index d9454ab..7d3f016 100644 --- a/dashcaddy-api/routes/errorlogs.js +++ b/dashcaddy-api/routes/errorlogs.js @@ -3,7 +3,7 @@ const fs = require('fs'); const fsp = require('fs').promises; const { exists } = require('../fs-helpers'); const { paginate, parsePaginationParams } = require('../pagination'); -const { success } = require('../response-helpers'); +const { success } = require('../src/utils/responses'); /** * Error logs routes factory diff --git a/dashcaddy-api/routes/health.js b/dashcaddy-api/routes/health.js index badcbd3..c59ac48 100644 --- a/dashcaddy-api/routes/health.js +++ b/dashcaddy-api/routes/health.js @@ -7,7 +7,7 @@ const { exists } = require('../fs-helpers'); const { paginate, parsePaginationParams } = require('../pagination'); const platformPaths = require('../platform-paths'); const { resolveServiceUrl } = require('../url-resolver'); -const { success, error: errorResponse } = require('../response-helpers'); +const { success, error: errorResponse } = require('../src/utils/responses'); const { ValidationError } = require('../errors'); /** diff --git a/dashcaddy-api/routes/license.js b/dashcaddy-api/routes/license.js index 18b716a..45b91a1 100644 --- a/dashcaddy-api/routes/license.js +++ b/dashcaddy-api/routes/license.js @@ -1,5 +1,5 @@ const express = require('express'); -const { success, error: errorResponse } = require('../response-helpers'); +const { success, error: errorResponse } = require('../src/utils/responses'); const { ValidationError } = require('../errors'); /** diff --git a/dashcaddy-api/routes/monitoring.js b/dashcaddy-api/routes/monitoring.js index 46e7498..4a512e6 100644 --- a/dashcaddy-api/routes/monitoring.js +++ b/dashcaddy-api/routes/monitoring.js @@ -1,5 +1,5 @@ const express = require('express'); -const { success } = require('../response-helpers'); +const { success } = require('../src/utils/responses'); /** * Monitoring routes factory diff --git a/dashcaddy-api/routes/services.js b/dashcaddy-api/routes/services.js index f9ffa9d..a87a3f7 100644 --- a/dashcaddy-api/routes/services.js +++ b/dashcaddy-api/routes/services.js @@ -10,7 +10,7 @@ const { exists } = require('../fs-helpers'); const { paginate, parsePaginationParams } = require('../pagination'); const { ValidationError, NotFoundError, ConflictError } = require('../errors'); const { resolveServiceUrl } = require('../url-resolver'); -const { success, error: errorResponse } = require('../response-helpers'); +const { success, error: errorResponse } = require('../src/utils/responses'); const platformPaths = require('../platform-paths'); /** diff --git a/dashcaddy-api/routes/ssl-monitor.js b/dashcaddy-api/routes/ssl-monitor.js index ffe53fa..3157ced 100644 --- a/dashcaddy-api/routes/ssl-monitor.js +++ b/dashcaddy-api/routes/ssl-monitor.js @@ -6,7 +6,7 @@ */ const express = require('express'); -const { success, error: errorResponse, notFound } = require('../response-helpers'); +const { success, error: errorResponse, notFound } = require('../src/utils/responses'); /** * SSL Monitor route factory diff --git a/dashcaddy-api/routes/themes.js b/dashcaddy-api/routes/themes.js index 3c30858..404ea34 100644 --- a/dashcaddy-api/routes/themes.js +++ b/dashcaddy-api/routes/themes.js @@ -1,7 +1,7 @@ const express = require('express'); const fs = require('fs'); const path = require('path'); -const { success } = require('../response-helpers'); +const { success } = require('../src/utils/responses'); const { ValidationError, NotFoundError } = require('../errors'); const platformPaths = require('../platform-paths'); diff --git a/dashcaddy-api/src/utils/responses.js b/dashcaddy-api/src/utils/responses.js index f454549..eb59da0 100644 --- a/dashcaddy-api/src/utils/responses.js +++ b/dashcaddy-api/src/utils/responses.js @@ -1,22 +1,124 @@ /** * Response helpers - Standard API response formats + * + * Single source of truth for HTTP response shapes across DashCaddy. + * Standard envelope: { success: true, ...data } or { success: false, error: "..." }. + * + * All routes should import from this module — do not call res.json/res.status + * directly with the response shape, use these helpers instead. */ +const { HTTP_STATUS } = require('../../constants'); + +// ── Success helpers ──────────────────────────────────────────── /** - * Standard error response + * Standard success response. Use this in route handlers. + * Wraps the data object with a `success: true` envelope. + * @param {object} res Express response + * @param {object} [data={}] fields to include in the response body + * @param {number} [statusCode=200] HTTP status code + */ +function ok(res, data = {}, statusCode = HTTP_STATUS.OK) { + return res.status(statusCode).json({ success: true, ...data }); +} + +/** + * Alias for `ok` — prefer `ok` in new code, but kept for code that imports as `success`. + */ +function success(res, data, statusCode) { + return ok(res, data, statusCode); +} + +/** + * Success response with a human-readable message field. + * Use when there's no data to return, just confirmation. + */ +function successMessage(res, message, statusCode = HTTP_STATUS.OK) { + return res.status(statusCode).json({ success: true, message }); +} + +/** + * 201 Created response. + */ +function created(res, data = {}) { + return res.status(HTTP_STATUS.CREATED).json({ success: true, ...data }); +} + +/** + * 204 No Content response. + */ +function noContent(res) { + return res.status(HTTP_STATUS.NO_CONTENT).send(); +} + +// ── Error helpers ────────────────────────────────────────────── + +/** + * Standard error response. Use this in route handlers. + * @param {object} res Express response + * @param {number} statusCode HTTP status code + * @param {string} message Human-readable error message + * @param {object} [extras={}] additional fields to merge into the response */ function errorResponse(res, statusCode, message, extras = {}) { return res.status(statusCode).json({ success: false, error: message, ...extras }); } /** - * Standard success response + * Alias for `errorResponse` — kept for code that imports as `error`. */ -function ok(res, data = {}) { - return res.json({ success: true, ...data }); +function error(res, message, statusCode = HTTP_STATUS.INTERNAL_ERROR) { + return res.status(statusCode).json({ success: false, error: message }); +} + +/** + * 400 Bad Request — invalid input from the user. + */ +function validationError(res, message) { + return res.status(HTTP_STATUS.BAD_REQUEST).json({ success: false, error: message }); +} + +/** + * 401 Unauthorized — no valid credentials. + */ +function unauthorized(res, message = 'Unauthorized') { + return res.status(HTTP_STATUS.UNAUTHORIZED).json({ success: false, error: message }); +} + +/** + * 403 Forbidden — credentials valid but permission denied. + */ +function forbidden(res, message = 'Forbidden') { + return res.status(HTTP_STATUS.FORBIDDEN).json({ success: false, error: message }); +} + +/** + * 404 Not Found — resource doesn't exist. + */ +function notFound(res, message = 'Not found') { + return res.status(HTTP_STATUS.NOT_FOUND).json({ success: false, error: message }); +} + +/** + * 409 Conflict — request conflicts with current state (e.g. duplicate). + */ +function conflict(res, message) { + return res.status(HTTP_STATUS.CONFLICT).json({ success: false, error: message }); } module.exports = { - errorResponse, + // Success helpers ok, + success, + successMessage, + created, + noContent, + // Error helpers + errorResponse, + error, + validationError, + unauthorized, + forbidden, + notFound, + conflict, };