lib: implement flexible API and error serialization for logger

Author: mertcanaltin

lib/logger.js

@@ -2,9 +2,11 @@
 
 const {
   DateNow,
+  Error,
   JSONStringify,
   ObjectAssign,
   ObjectKeys,
+  ObjectPrototypeToString,
 } = primordials;
 
 const {
@@ -241,83 +243,151 @@ class Logger {
     });
   }
 
+  /**
+   * Check if value is an Error object
+   * @param {*} value
+   * @returns {boolean}
+   * @private
+   */
+  _isError(value) {
+    return value !== null &&
+           typeof value === 'object' &&
+           (ObjectPrototypeToString(value) === '[object Error]' ||
+            value instanceof Error);
+  }
+
   /**
    * Internal log method
    * @param {string} level
    * @param {number} levelValue
-   * @param {object} obj
+   * @param {string|object} msgOrObj - Message string or object with msg property
+   * @param {object} [fields] - Optional fields to merge
    */
-  _log(level, levelValue, obj) {
-    // Note: Level check is now done at method assignment time (noop)
-    // So this function is only called for enabled levels
-
-    // Validate required msg field
-    validateObject(obj, 'obj');
-    if (typeof obj.msg !== 'string') {
-      throw new ERR_INVALID_ARG_TYPE('obj.msg', 'string', obj.msg);
-    }
+  _log(level, levelValue, msgOrObj, fields) {
+    let msg;
+    let logFields;
+
+    // Support two signatures:
+    // 1. logger.info('message', { fields })
+    // 2. logger.info({ msg: 'message', other: 'fields' })
+    // 3. logger.info(new Error('boom'))
+
+    if (this._isError(msgOrObj)) {
+      // Handle Error as first argument
+      msg = msgOrObj.message;
+      logFields = {
+        err: this._serializeError(msgOrObj),
+        ...fields,
+      };
+    } else if (typeof msgOrObj === 'string') {
+      // Support String message
+      msg = msgOrObj;
+      logFields = fields || {};
+      validateObject(logFields, 'fields');
+    } else {
+      // Support object with msg property
+      validateObject(msgOrObj, 'obj');
+      if (typeof msgOrObj.msg !== 'string') {
+        throw new ERR_INVALID_ARG_TYPE('obj.msg', 'string', msgOrObj.msg);
+      }
+      const { msg: extractedMsg, ...restFields } = msgOrObj;
+      msg = extractedMsg;
+      logFields = restFields;
 
-    // Extract msg and remaining fields
-    const { msg, ...fields } = obj;
+      // Serialize Error objects in fields
+      if (logFields.err && this._isError(logFields.err)) {
+        logFields.err = this._serializeError(logFields.err);
+      }
+    }
 
     // Build log record
     const record = {
       level,
       msg,
       time: DateNow(),
       bindings: this._bindings,
-      fields,
+      fields: logFields,
     };
 
     this._handler.handle(record);
   }
 
+  /**
+   * Serialize Error object for logging
+   * @param {object} err - Error object to serialize
+   * @returns {object} Serialized error object
+   * @private
+   */
+  _serializeError(err) {
+    const serialized = {
+      message: err.message,
+      name: err.name,
+      stack: err.stack,
+    };
+
+    // Add code if it exists
+    serialized.code ||= err.code;
+
+    // Add any enumerable custom properties
+    for (const key in err) {
+      serialized[key] ||= err[key];
+    }
+
+    return serialized;
+  }
+
   /**
    * Log at trace level
-   * @param {object} obj - Object with required msg property
+   * @param {string|object} msgOrObj - Message string or object with msg property
+   * @param {object} [fields] - Optional fields to merge
    */
-  trace(obj) {
-    this._log('trace', 10, obj);
+  trace(msgOrObj, fields) {
+    this._log('trace', 10, msgOrObj, fields);
   }
 
   /**
    * Log at debug level
-   * @param {object} obj - Object with required msg property
+   * @param {string|object} msgOrObj - Message string or object with msg property
+   * @param {object} [fields] - Optional fields to merge
    */
-  debug(obj) {
-    this._log('debug', 20, obj);
+  debug(msgOrObj, fields) {
+    this._log('debug', 20, msgOrObj, fields);
   }
 
   /**
    * Log at info level
-   * @param {object} obj - Object with required msg property
+   * @param {string|object} msgOrObj - Message string or object with msg property
+   * @param {object} [fields] - Optional fields to merge
    */
-  info(obj) {
-    this._log('info', 30, obj);
+  info(msgOrObj, fields) {
+    this._log('info', 30, msgOrObj, fields);
   }
 
   /**
    * Log at warn level
-   * @param {object} obj - Object with required msg property
+   * @param {string|object} msgOrObj - Message string or object with msg property
+   * @param {object} [fields] - Optional fields to merge
    */
-  warn(obj) {
-    this._log('warn', 40, obj);
+  warn(msgOrObj, fields) {
+    this._log('warn', 40, msgOrObj, fields);
   }
 
   /**
    * Log at error level
-   * @param {object} obj - Object with required msg property
+   * @param {string|object} msgOrObj - Message string or object with msg property
+   * @param {object} [fields] - Optional fields to merge
    */
-  error(obj) {
-    this._log('error', 50, obj);
+  error(msgOrObj, fields) {
+    this._log('error', 50, msgOrObj, fields);
   }
 
   /**
    * Log at fatal level (does NOT exit the process)
-   * @param {object} obj - Object with required msg property
+   * @param {string|object} msgOrObj - Message string or object with msg property
+   * @param {object} [fields] - Optional fields to merge
    */
-  fatal(obj) {
-    this._log('fatal', 60, obj);
+  fatal(msgOrObj, fields) {
+    this._log('fatal', 60, msgOrObj, fields);
   }
 
   /**

test/parallel/test-log-basic.js

@@ -52,6 +52,8 @@ const path = require('path');
 // Test msg field is required
 {
   const logger = createLogger();
+
+  // Object without msg should throw
   assert.throws(() => {
     logger.info({ userId: 123 }); // Missing msg
   }, {
@@ -63,9 +65,10 @@ const path = require('path');
   }, {
     code: 'ERR_INVALID_ARG_TYPE',
   });
-}
 
-// Test child logger context inheritance
+  // String without second arg should work (no assertion needed, just shouldn't throw)
+  logger.info('just a message');
+}// Test child logger context inheritance
 {
   const logger = createLogger({ level: 'info' });
   const childLogger = logger.child({ requestId: 'abc-123' });
@@ -182,7 +185,8 @@ const path = require('path');
   assert.throws(() => {
     handler.handle({});
   }, {
-    message: /must be implemented/,
+    code: 'ERR_METHOD_NOT_IMPLEMENTED',
+    message: /Handler\.handle\(\) method is not implemented/,
   });
 }
 
@@ -208,3 +212,162 @@ const path = require('path');
   logger.error({ msg: 'processed' });
   assert.strictEqual(handleCalled, true);
 }
+
+// Test invalid fields argument
+{
+  const logger = createLogger();
+  assert.throws(() => {
+    logger.info('message', 'not an object'); // Second arg must be object
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+  });
+}
+
+// Test string message signature
+{
+  const tmpfile = path.join(os.tmpdir(), `test-log-${process.pid}-string.json`);
+  const handler = new JSONHandler({
+    stream: fs.openSync(tmpfile, 'w'),
+    level: 'info',
+  });
+  const logger = new Logger({ handler });
+
+  logger.info('simple message');
+
+  handler.flushSync();
+  handler.end();
+  const output = fs.readFileSync(tmpfile, 'utf8');
+  const parsed = JSON.parse(output.trim());
+
+  assert.strictEqual(parsed.msg, 'simple message');
+  assert.strictEqual(parsed.level, 'info');
+
+  fs.unlinkSync(tmpfile);
+}
+
+// Test string message with fields
+{
+  const tmpfile = path.join(os.tmpdir(), `test-log-${process.pid}-string-fields.json`);
+  const handler = new JSONHandler({
+    stream: fs.openSync(tmpfile, 'w'),
+    level: 'info',
+  });
+  const logger = new Logger({ handler });
+
+  logger.info('user login', { userId: 123, ip: '127.0.0.1' });
+
+  handler.flushSync();
+  handler.end();
+  const output = fs.readFileSync(tmpfile, 'utf8');
+  const parsed = JSON.parse(output.trim());
+
+  assert.strictEqual(parsed.msg, 'user login');
+  assert.strictEqual(parsed.userId, 123);
+  assert.strictEqual(parsed.ip, '127.0.0.1');
+
+  fs.unlinkSync(tmpfile);
+}
+
+// Test Error object serialization
+{
+  const tmpfile = path.join(os.tmpdir(), `test-log-${process.pid}-error.json`);
+  const handler = new JSONHandler({
+    stream: fs.openSync(tmpfile, 'w'),
+    level: 'info',
+  });
+  const logger = new Logger({ handler });
+
+  const err = new Error('test error');
+  err.code = 'TEST_ERROR';
+  logger.error({ msg: 'operation failed', err });
+
+  handler.flushSync();
+  handler.end();
+  const output = fs.readFileSync(tmpfile, 'utf8');
+  const parsed = JSON.parse(output.trim());
+
+  // Error should be serialized with stack trace
+  assert.strictEqual(parsed.msg, 'operation failed');
+  assert.strictEqual(typeof parsed.err, 'object');
+  assert.strictEqual(parsed.err.message, 'test error');
+  assert.strictEqual(parsed.err.code, 'TEST_ERROR');
+  assert(parsed.err.stack);
+
+  fs.unlinkSync(tmpfile);
+}
+
+// Test Error as first argument
+{
+  const tmpfile = path.join(os.tmpdir(), `test-log-${process.pid}-error-first.json`);
+  const handler = new JSONHandler({
+    stream: fs.openSync(tmpfile, 'w'),
+    level: 'info',
+  });
+  const logger = new Logger({ handler });
+
+  const err = new Error('boom');
+  logger.error(err); // Error as first arg
+
+  handler.flushSync();
+  handler.end();
+  const output = fs.readFileSync(tmpfile, 'utf8');
+  const parsed = JSON.parse(output.trim());
+
+  assert.strictEqual(parsed.msg, 'boom'); // message from error
+  assert.strictEqual(typeof parsed.err, 'object');
+  assert(parsed.err.stack);
+
+  fs.unlinkSync(tmpfile);
+}
+
+// Test child logger with parent fields merge
+{
+  const tmpfile = path.join(os.tmpdir(), `test-log-${process.pid}-child-merge.json`);
+  const handler = new JSONHandler({
+    stream: fs.openSync(tmpfile, 'w'),
+    level: 'info',
+    fields: { service: 'api' }, // handler fields
+  });
+  const logger = new Logger({ handler });
+  const childLogger = logger.child({ requestId: '123' }); // child bindings
+
+  childLogger.info('request processed', { duration: 150 }); // log fields
+
+  handler.flushSync();
+  handler.end();
+  const output = fs.readFileSync(tmpfile, 'utf8');
+  const parsed = JSON.parse(output.trim());
+
+  // Merge order: handler fields < bindings < log fields
+  assert.strictEqual(parsed.service, 'api');
+  assert.strictEqual(parsed.requestId, '123');
+  assert.strictEqual(parsed.duration, 150);
+  assert.strictEqual(parsed.msg, 'request processed');
+
+  fs.unlinkSync(tmpfile);
+}
+
+// Test field override priority
+{
+  const tmpfile = path.join(os.tmpdir(), `test-log-${process.pid}-override.json`);
+  const handler = new JSONHandler({
+    stream: fs.openSync(tmpfile, 'w'),
+    level: 'info',
+    fields: { env: 'dev', version: '1.0' },
+  });
+  const logger = new Logger({ handler });
+  const childLogger = logger.child({ env: 'staging' });
+
+  childLogger.info('test', { env: 'production' });
+
+  handler.flushSync();
+  handler.end();
+  const output = fs.readFileSync(tmpfile, 'utf8');
+  const parsed = JSON.parse(output.trim());
+
+  // Log fields should override everything
+  assert.strictEqual(parsed.env, 'production');
+  assert.strictEqual(parsed.version, '1.0');
+
+  fs.unlinkSync(tmpfile);
+}