feat(logs): error-log parser fix + dedicated /logs page (DC-051) [UNJUDGED]

dashcaddy-api/__tests__/errorlogs-parser.test.js

@@ -0,0 +1,135 @@
+// Unit tests for the fixed /api/v1/error-logs parser
+// (route /opt/dashcaddy/dashcaddy-api/routes/errorlogs.js)
+//
+// Background: the prior implementation split on '='.repeat(80) but the
+// unified logger writes \u2500 horizontal-rule separators. As a result
+// every modal-open returned ZERO entries — same class of silent bug as
+// DC-050 (audit log). These tests pin the new behavior so future refactors
+// can't reintroduce it.
+
+const { parseEntries, readTailBytes, MAX_TAIL } = require('../routes/errorlogs');
+const path = require('path');
+const fs = require('fs');
+const fsp = require('fs').promises;
+const os = require('os');
+
+const SEP = '\n' + '\u2500'.repeat(72) + '\n';
+
+function buildLog(entries) {
+  return entries.map((e, i) => {
+    const head = `[${e.timestamp}] [${e.level}] ${e.context}: ${e.message}`;
+    return head + (e.details ? '\n' + e.details : '') + SEP;
+  }).join('');
+}
+
+describe('errorlogs parser (DC-051)', () => {
+  test('parses single entry with U+2500 separator', () => {
+    const text = buildLog([{
+      timestamp: '2026-08-16T23:13:14.123Z',
+      level: 'ERR',
+      context: '/api/v1/templates',
+      message: 'Route GET /v1/templates not found',
+      details: 'NotFoundError: Route GET /v1/templates not found\n    at notFoundHandler (/app/src/utilities/error-handler.js:71:8)',
+    }]);
+    const out = parseEntries(text);
+    expect(out).toHaveLength(1);
+    expect(out[0]).toMatchObject({
+      timestamp: '2026-08-16T23:13:14.123Z',
+      level: 'ERR',
+      context: '/api/v1/templates',
+      message: 'Route GET /v1/templates not found',
+    });
+    expect(out[0].details).toContain('notFoundHandler');
+    expect(out[0].details).not.toContain('\u2500');
+  });
+
+  test('returns multiple entries in order, ignoring separator residue', () => {
+    const text = buildLog([
+      { timestamp: '2026-08-16T23:00:00.000Z', level: 'ERR', context: 'a', message: 'first' },
+      { timestamp: '2026-08-16T23:01:00.000Z', level: 'WRN', context: 'b', message: 'second' },
+      { timestamp: '2026-08-16T23:02:00.000Z', level: 'INF', context: 'c', message: 'third', details: 'extra' },
+    ]);
+    const out = parseEntries(text);
+    expect(out.map(e => e.context)).toEqual(['a', 'b', 'c']);
+    expect(out[1].level).toBe('WRN');
+    expect(out[2].details).toBe('extra');
+  });
+
+  test('skips malformed lines without throwing', () => {
+    const text = 'this is not a log entry\n' + SEP + '[2026-08-16T23:00:00.000Z] [ERR] x: y\n' + SEP;
+    const out = parseEntries(text);
+    expect(out).toHaveLength(1);
+    expect(out[0].message).toBe('y');
+  });
+
+  test('empty input returns empty array', () => {
+    expect(parseEntries('')).toEqual([]);
+    expect(parseEntries('   \n\n   ')).toEqual([]);
+  });
+
+  test('regression: would have returned 0 entries under the OLD splitter', () => {
+    // Old impl: text.split('='.repeat(80)).filter(...). That produced one
+    // big block, parser rejected all headers, returned ZERO entries. New
+    // impl must NOT regress to that behavior on a real-format log.
+    const text = buildLog([
+      { timestamp: '2026-08-16T23:00:00.000Z', level: 'ERR', context: 'a', message: 'm' },
+      { timestamp: '2026-08-16T23:01:00.000Z', level: 'ERR', context: 'b', message: 'm' },
+    ]);
+    // Sanity: the old split would produce 1 block (no '=' in the text).
+    expect(text.split('='.repeat(80))).toHaveLength(1);
+    // New parser must surface both entries.
+    expect(parseEntries(text)).toHaveLength(2);
+  });
+
+  test('MAX_TAIL is bounded (>=100, <=1000) — prevents unbounded read', () => {
+    expect(MAX_TAIL).toBeGreaterThanOrEqual(100);
+    expect(MAX_TAIL).toBeLessThanOrEqual(1000);
+  });
+});
+
+describe('errorlogs readTailBytes (DC-051)', () => {
+  let tmpFile;
+  beforeAll(async () => {
+    tmpFile = path.join(os.tmpdir(), `dc-051-errorlog-${process.pid}.log`);
+    const entries = [];
+    for (let i = 0; i < 50; i++) {
+      entries.push({
+        timestamp: `2026-08-16T23:${String(i % 60).padStart(2,'0')}:00.000Z`,
+        level: i % 2 === 0 ? 'ERR' : 'WRN',
+        context: `ctx-${i}`,
+        message: `message body ${i}`,
+        details: i % 3 === 0 ? `stack for ${i}` : null,
+      });
+    }
+    await fsp.writeFile(tmpFile, buildLog(entries));
+  });
+  afterAll(async () => {
+    try { await fsp.unlink(tmpFile); } catch {}
+  });
+
+  test('returns parsed entries within the byte budget', async () => {
+    const { text, totalSize, truncated } = await readTailBytes(tmpFile, 4 * 1024);
+    expect(typeof totalSize).toBe('number');
+    expect(typeof truncated).toBe('boolean');
+    const parsed = parseEntries(text);
+    expect(parsed.length).toBeGreaterThan(0);
+    // Should never include partial first line — every parsed entry has a real timestamp.
+    for (const e of parsed) {
+      expect(e.timestamp).toMatch(/^\d{4}-\d{2}-\d{2}T/);
+    }
+  });
+
+  test('truncates when file exceeds byte budget', async () => {
+    const stat = await fsp.stat(tmpFile);
+    const smallBudget = Math.floor(stat.size / 4);
+    const { truncated } = await readTailBytes(tmpFile, smallBudget);
+    expect(truncated).toBe(true);
+  });
+
+  test('does not truncate when file fits within byte budget', async () => {
+    const stat = await fsp.stat(tmpFile);
+    const bigBudget = stat.size * 2;
+    const { truncated } = await readTailBytes(tmpFile, bigBudget);
+    expect(truncated).toBe(false);
+  });
+});

dashcaddy-api/routes/errorlogs.js

@@ -1,9 +1,80 @@
 const express = require('express');
-const fs = require('fs');
 const fsp = require('fs').promises;
 const { exists } = require('../src/utilities/fs-helpers');
 const { paginate, parsePaginationParams } = require('../src/utilities/pagination');
 const { success } = require('../src/utils/responses');
+const { ValidationError } = require('../src/utilities/errors');
+
+// The unified error logger writes entries separated by a long horizontal-rule
+// line made of U+2500 BOX DRAWINGS LIGHT HORIZONTAL (verified 2026-08-18
+// against /opt/dashcaddy/dashcaddy-api/data/error.log on DNS2 — the previous
+// implementation split on '='.repeat(80), which returned ONE block and
+// produced ZERO entries for the modal). Anything else got dropped silently.
+const ENTRY_SEPARATOR_RE = /\n\u2500{20,}\n?/;
+const ENTRY_HEADER_RE = /^\[([^\]]+)\]\s+\[([A-Z]+)\]\s+(.*?):\s*(.*)$/;
+
+const MAX_TAIL = 500;
+const MAX_TAIL_BYTES = 2 * 1024 * 1024; // never read more than 2 MiB from disk
+
+/**
+ * Parse the unified error-log format into structured entries.
+ * Each entry:
+ *   [2026-08-16T23:13:14.123Z] [ERR] ctx: message
+ *   <stack trace lines, if any>
+ *   request: ...     (optional)
+ *   context: {...}   (optional)
+ *   ────────────── (separator)
+ * @param {string} text
+ * @returns {Array<{timestamp:string,level:string,context:string,message:string,details:string|null}>}
+ */
+function parseEntries(text) {
+  if (!text) return [];
+  const blocks = text.split(ENTRY_SEPARATOR_RE);
+  const entries = [];
+  for (const block of blocks) {
+    const trimmed = block.replace(/^\n+|\n+$/g, '');
+    if (!trimmed) continue;
+    const firstLineEnd = trimmed.indexOf('\n');
+    const firstLine = firstLineEnd === -1 ? trimmed : trimmed.slice(0, firstLineEnd);
+    const rest = firstLineEnd === -1 ? '' : trimmed.slice(firstLineEnd + 1);
+    const m = firstLine.match(ENTRY_HEADER_RE);
+    if (!m) continue;
+    entries.push({
+      timestamp: m[1],
+      level: m[2],
+      context: m[3],
+      message: m[4],
+      details: rest ? rest.replace(/\n+$/g, '') : null,
+    });
+  }
+  return entries;
+}
+
+/**
+ * Read the last N bytes of a UTF-8 file safely (so the 4 MiB log doesn't
+ * blow up memory or block the event loop). Splits on the first complete
+ * line boundary after the cut.
+ */
+async function readTailBytes(filePath, byteLimit) {
+  const fh = await fsp.open(filePath, 'r');
+  try {
+    const stat = await fh.stat();
+    const start = Math.max(0, stat.size - byteLimit);
+    const length = stat.size - start;
+    const buf = Buffer.alloc(length);
+    await fh.read(buf, 0, length, start);
+    let text = buf.toString('utf8');
+    // If we cut into the middle of a UTF-8 sequence, drop the partial char
+    const partialLead = text.match(/[\uD800-\uDBFF]$/);
+    if (partialLead) text = text.slice(0, -1);
+    // Drop a half first line so we never start mid-entry
+    const nl = text.indexOf('\n');
+    if (start > 0 && nl !== -1) text = text.slice(nl + 1);
+    return { text, totalSize: stat.size, truncated: start > 0 };
+  } finally {
+    await fh.close();
+  }
+}
 
 /**
  * Error logs routes factory
@@ -17,38 +88,41 @@ module.exports = function({ ERROR_LOG_FILE, auditLogger, asyncHandler }) {
   const router = express.Router();
 
   // Get error logs
+  //   GET /api/v1/error-logs?tail=100&level=ERR
+  //   - tail: cap on returned entries (default 100, max 500)
+  //   - level: filter by level (ERR/WARN/INFO/DBG) — case-insensitive
   router.get('/error-logs', asyncHandler(async (req, res) => {
     if (!await exists(ERROR_LOG_FILE)) {
-      return success(res, { logs: [] });
+      return success(res, { logs: [], totalSize: 0, truncated: false });
     }
 
-    const logContent = await fsp.readFile(ERROR_LOG_FILE, 'utf8');
-      const logEntries = logContent.split('='.repeat(80)).filter(entry => entry.trim());
+    let tailRaw = parseInt(req.query.tail, 10);
+    if (!Number.isFinite(tailRaw) || tailRaw <= 0) tailRaw = 100;
+    const tail = Math.min(tailRaw, MAX_TAIL);
 
-      const logs = logEntries.map(entry => {
-        const lines = entry.trim().split('\n');
-        const firstLine = lines[0] || '';
-        const match = firstLine.match(/\[(.*?)\] (.*?): (.*)/);
+    const levelFilter = req.query.level ? String(req.query.level).toUpperCase() : null;
 
-        if (match) {
-          return {
-            timestamp: match[1],
-            context: match[2],
-            error: match[3]
-          };
-        }
-        return null;
-      }).filter(Boolean);
+    const { text, totalSize, truncated } = await readTailBytes(ERROR_LOG_FILE, MAX_TAIL_BYTES);
+    let logs = parseEntries(text);
 
-    success(res, { logs: logs.slice(-50).reverse() });
+    if (levelFilter) {
+      logs = logs.filter(e => e.level === levelFilter);
+    }
+
+    // Newest first; bounded by `tail`
+    logs = logs.slice(-tail).reverse();
+
+    success(res, { logs, totalSize, truncated, returned: logs.length });
   }, 'error-logs-get'));
 
   // Clear error logs
   router.delete('/error-logs', asyncHandler(async (req, res) => {
-    if (await exists(ERROR_LOG_FILE)) {
-      await fsp.writeFile(ERROR_LOG_FILE, '');
+    if (!await exists(ERROR_LOG_FILE)) {
+      return success(res, { message: 'Error logs cleared', cleared: 0 });
     }
-    success(res, { message: 'Error logs cleared' });
+    const before = await fsp.stat(ERROR_LOG_FILE).then(s => s.size).catch(() => 0);
+    await fsp.writeFile(ERROR_LOG_FILE, '');
+    success(res, { message: 'Error logs cleared', clearedBytes: before });
   }, 'error-logs-clear'));
 
   // Audit log
@@ -56,7 +130,6 @@ module.exports = function({ ERROR_LOG_FILE, auditLogger, asyncHandler }) {
     const paginationParams = parsePaginationParams(req.query);
     const action = req.query.action || '';
     if (paginationParams) {
-      // When paginating, fetch all matching entries and let pagination slice
       const entries = await auditLogger.query({ limit: Number.MAX_SAFE_INTEGER, offset: 0, action });
       const result = paginate(entries, paginationParams);
       success(res, { entries: result.data, pagination: result.pagination });
@@ -75,3 +148,9 @@ module.exports = function({ ERROR_LOG_FILE, auditLogger, asyncHandler }) {
 
   return router;
 };
+
+// Exported for unit testing
+module.exports.parseEntries = parseEntries;
+module.exports.readTailBytes = readTailBytes;
+module.exports.MAX_TAIL = MAX_TAIL;
+module.exports.MAX_TAIL_BYTES = MAX_TAIL_BYTES;