From 94da521b1912e338948b5cd0897df8fa343e5da6 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Tue, 19 May 2026 01:24:24 -0500 Subject: [PATCH] fix(logger): actually verify pino-pretty is installed before configuring it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing `try` block around the `transport = { target: 'pino-pretty', … }` assignment was a no-op. Object-literal construction doesn't throw — pino lazy-resolves the `target` module the first time the logger emits, which means a deployment that sets `LOG_PRETTY=1` without `pino-pretty` on disk would crash on first log line (often during DB-init logging, seconds into startup) with a confusing pino-internal error. The "fall through to JSON output" path the comment promised never ran. Insert an explicit `require.resolve('pino-pretty')` inside the try block. `require.resolve` synchronously throws MODULE_NOT_FOUND when the module is missing, which lets the catch fire and the transport stay undefined — so pino falls through to default JSON output, which is what the operator should see when they ask for pretty output but haven't installed the (optional) dev dependency. No test added because exercising the missing-pino-pretty branch requires either a subprocess spawn (heavyweight) or filesystem mocking that interacts with Node's module loader (brittle); the existing logger tests verify the loaded-module case and continue to pass. The change is a behavior-correctness fix that brings the code in line with what the comment already promised. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/config/logger.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/app/config/logger.js b/app/config/logger.js index 10de276..1bcdcfd 100644 --- a/app/config/logger.js +++ b/app/config/logger.js @@ -18,13 +18,26 @@ const level = process.env.LOG_LEVEL || 'info'; let transport; if (process.env.LOG_PRETTY === '1') { + // Check `pino-pretty` is actually installed BEFORE handing the + // transport config to pino. Assigning the object literal alone + // never throws — pino lazy-resolves the `target` module later + // and emits a cryptic error on startup if it's missing. The + // `require.resolve` synchronously throws MODULE_NOT_FOUND when + // the module isn't on disk, so the catch below fires cleanly + // and we fall through to default JSON output, matching the + // "pino-pretty is NOT a required dependency" header comment. try { + require.resolve('pino-pretty'); transport = { target: 'pino-pretty', options: { colorize: true, translateTime: 'SYS:standard' }, }; - } catch (_) { - // pino-pretty not installed — fall through to JSON output. + } catch (_err) { + // pino-pretty not installed — silently fall through to JSON + // output. We don't warn here because the only consumer of + // LOG_PRETTY=1 is interactive dev, and a noisy stderr line + // would clutter the very output the operator was trying to + // make readable. } }