lib: address review feedback for logger API

Author: mertcanaltin

doc/api/logger.md

@@ -1,11 +1,14 @@
 # Logger
 
-<!--introduced_in=v26.0.0-->
+<!--introduced_in=REPLACEME-->
 
-> Stability: 1.0 - Early Development
+> Stability: 1.1 - Active Development
 
 <!-- source_link=lib/logger.js -->
 
+The `node:logger` module is available when Node.js is started with the
+`--experimental-logger` flag.
+
 The `node:logger` module provides high-performance structured logging
 capabilities for Node.js applications. It uses `diagnostics_channel` internally
 to dispatch log events to consumers, allowing multiple consumers to receive
@@ -100,10 +103,15 @@ const logger = new Logger({
 added: REPLACEME
 -->
 
-* `msg` {string} Log message.
-* `obj` {Object} Object containing `msg` property and additional fields.
-* `error` {Error} Error object to log.
-* `fields` {Object} Additional fields to include in the log record.
+* `msg` {string} The log message. When passed as the first argument, any second
+  argument is treated as `fields`.
+* `obj` {Object} When passed as the first argument, `obj.msg` is used as the
+  log message and all other properties are merged into the log record as fields.
+* `error` {Error} When passed as the first argument, the error is serialized
+  into log fields including `type`, `message`, and `stack`. An optional `fields`
+  argument may follow.
+* `fields` {Object} Additional fields to merge into the log record. Only
+  applicable alongside `msg` or `error`, not with `obj`.
 
 Logs a message at the `trace` level.
 
@@ -123,10 +131,15 @@ logger.trace({ msg: 'Object format', requestId: 'abc123' });
 added: REPLACEME
 -->
 
-* `msg` {string} Log message.
-* `obj` {Object} Object containing `msg` property and additional fields.
-* `error` {Error} Error object to log.
-* `fields` {Object} Additional fields to include in the log record.
+* `msg` {string} The log message. When passed as the first argument, any second
+  argument is treated as `fields`.
+* `obj` {Object} When passed as the first argument, `obj.msg` is used as the
+  log message and all other properties are merged into the log record as fields.
+* `error` {Error} When passed as the first argument, the error is serialized
+  into log fields including `type`, `message`, and `stack`. An optional `fields`
+  argument may follow.
+* `fields` {Object} Additional fields to merge into the log record. Only
+  applicable alongside `msg` or `error`, not with `obj`.
 
 Logs a message at the `debug` level.
 
@@ -145,10 +158,15 @@ logger.debug('Processing request', { requestId: 'abc123' });
 added: REPLACEME
 -->
 
-* `msg` {string} Log message.
-* `obj` {Object} Object containing `msg` property and additional fields.
-* `error` {Error} Error object to log.
-* `fields` {Object} Additional fields to include in the log record.
+* `msg` {string} The log message. When passed as the first argument, any second
+  argument is treated as `fields`.
+* `obj` {Object} When passed as the first argument, `obj.msg` is used as the
+  log message and all other properties are merged into the log record as fields.
+* `error` {Error} When passed as the first argument, the error is serialized
+  into log fields including `type`, `message`, and `stack`. An optional `fields`
+  argument may follow.
+* `fields` {Object} Additional fields to merge into the log record. Only
+  applicable alongside `msg` or `error`, not with `obj`.
 
 Logs a message at the `info` level.
 
@@ -168,10 +186,15 @@ logger.info({ msg: 'User logged in', userId: 123 });
 added: REPLACEME
 -->
 
-* `msg` {string} Log message.
-* `obj` {Object} Object containing `msg` property and additional fields.
-* `error` {Error} Error object to log.
-* `fields` {Object} Additional fields to include in the log record.
+* `msg` {string} The log message. When passed as the first argument, any second
+  argument is treated as `fields`.
+* `obj` {Object} When passed as the first argument, `obj.msg` is used as the
+  log message and all other properties are merged into the log record as fields.
+* `error` {Error} When passed as the first argument, the error is serialized
+  into log fields including `type`, `message`, and `stack`. An optional `fields`
+  argument may follow.
+* `fields` {Object} Additional fields to merge into the log record. Only
+  applicable alongside `msg` or `error`, not with `obj`.
 
 Logs a message at the `warn` level.
 
@@ -190,10 +213,15 @@ logger.warn('High memory usage', { memoryUsage: process.memoryUsage() });
 added: REPLACEME
 -->
 
-* `msg` {string} Log message.
-* `obj` {Object} Object containing `msg` property and additional fields.
-* `error` {Error} Error object to log.
-* `fields` {Object} Additional fields to include in the log record.
+* `msg` {string} The log message. When passed as the first argument, any second
+  argument is treated as `fields`.
+* `obj` {Object} When passed as the first argument, `obj.msg` is used as the
+  log message and all other properties are merged into the log record as fields.
+* `error` {Error} When passed as the first argument, the error is serialized
+  into log fields including `type`, `message`, and `stack`. An optional `fields`
+  argument may follow.
+* `fields` {Object} Additional fields to merge into the log record. Only
+  applicable alongside `msg` or `error`, not with `obj`.
 
 Logs a message at the `error` level.
 
@@ -213,10 +241,15 @@ logger.error(new Error('Request failed'), { requestId: 'abc123' });
 added: REPLACEME
 -->
 
-* `msg` {string} Log message.
-* `obj` {Object} Object containing `msg` property and additional fields.
-* `error` {Error} Error object to log.
-* `fields` {Object} Additional fields to include in the log record.
+* `msg` {string} The log message. When passed as the first argument, any second
+  argument is treated as `fields`.
+* `obj` {Object} When passed as the first argument, `obj.msg` is used as the
+  log message and all other properties are merged into the log record as fields.
+* `error` {Error} When passed as the first argument, the error is serialized
+  into log fields including `type`, `message`, and `stack`. An optional `fields`
+  argument may follow.
+* `fields` {Object} Additional fields to merge into the log record. Only
+  applicable alongside `msg` or `error`, not with `obj`.
 
 Logs a message at the `fatal` level.
 
@@ -231,6 +264,8 @@ logger.fatal(new Error('Unrecoverable error'));
 added: REPLACEME
 -->
 
+> Stability: 1.1 - Active Development
+
 * `bindings` {Object} Additional context fields for the child logger.
 * `options` {Object}
   * `level` {string} Log level for the child logger.
@@ -613,6 +648,17 @@ added: REPLACEME
 An object containing `diagnostics_channel` instances for each log level.
 Advanced users can subscribe directly to these channels.
 
+The following channel names are used:
+
+| Channel | Log Level |
+| ----------- | --------- |
+| `log:trace` | `trace` |
+| `log:debug` | `debug` |
+| `log:info`  | `info` |
+| `log:warn`  | `warn` |
+| `log:error` | `error` |
+| `log:fatal` | `fatal` |
+
 ```js
 const { channels } = require('node:logger');
 

lib/internal/logger/serializers.js

@@ -1,20 +1,33 @@
 'use strict';
 const {
   ObjectKeys,
+  SafeSet,
 } = primordials;
 
 const { isNativeError } = require('internal/util/types');
 
 /**
  * Serializes an Error object
  * @param {Error} error
+ * @param {Set} [seen] Set of already-visited errors to detect circular references
  * @returns {{ type: string, message: string, stack: string, code?: any, cause?: any } | Error }
  */
-function serializeErr(error) {
+function serializeErr(error, seen) {
   if (!isNativeError(error)) {
+    // Pass through non-Error values unchanged to allow this serializer
+    // to be used even when the value may not always be an Error instance.
     return error;
   }
 
+  if (seen === undefined) {
+    seen = new SafeSet();
+  }
+
+  if (seen.has(error)) {
+    return '[Circular]';
+  }
+  seen.add(error);
+
   const obj = {
     __proto__: null,
     type: error.constructor.name,
@@ -34,7 +47,7 @@ function serializeErr(error) {
   // Handle error cause recursively
   if (error.cause !== undefined) {
     obj.cause = typeof error.cause === 'object' && error.cause !== null ?
-      serializeErr(error.cause) :
+      serializeErr(error.cause, seen) :
       error.cause;
   }
 

test/parallel/test-log-basic.js

@@ -474,3 +474,151 @@ describe('channels export', () => {
     assert.strictEqual(typeof channels.fatal, 'object');
   });
 });
+
+describe('Logger constructor validation', () => {
+  it('should throw when bindings is not an object', () => {
+    assert.throws(() => {
+      new Logger({ bindings: 'not-an-object' });
+    }, { code: 'ERR_INVALID_ARG_TYPE' });
+  });
+
+  it('should throw when a serializer value is not a function', () => {
+    assert.throws(() => {
+      new Logger({ serializers: { myField: 'not-a-function' } });
+    }, { code: 'ERR_INVALID_ARG_TYPE' });
+  });
+
+  it('should include constructor bindings in every log record', () => {
+    const stream = new TestStream();
+    const consumer = new JSONConsumer({ stream, level: 'info' });
+    consumer.attach();
+
+    const logger = new Logger({
+      level: 'info',
+      bindings: { service: 'api', version: '2.0' },
+    });
+
+    logger.info('startup');
+    consumer.flushSync();
+
+    const log = stream.logs[0];
+    assert.strictEqual(log.service, 'api');
+    assert.strictEqual(log.version, '2.0');
+  });
+});
+
+describe('Logger all log levels', () => {
+  it('should log at trace, debug, warn, and fatal levels', () => {
+    const stream = new TestStream();
+    const consumer = new JSONConsumer({ stream, level: 'trace' });
+    consumer.attach();
+
+    const logger = new Logger({ level: 'trace' });
+
+    logger.trace('trace msg');
+    logger.debug('debug msg');
+    logger.warn('warn msg');
+    logger.fatal('fatal msg');
+    consumer.flushSync();
+
+    assert.strictEqual(stream.logs.length, 4);
+    assert.strictEqual(stream.logs[0].level, 'trace');
+    assert.strictEqual(stream.logs[1].level, 'debug');
+    assert.strictEqual(stream.logs[2].level, 'warn');
+    assert.strictEqual(stream.logs[3].level, 'fatal');
+  });
+});
+
+describe('Error logging with additional fields', () => {
+  it('should merge extra fields alongside serialized error', () => {
+    const stream = new TestStream();
+    const consumer = new JSONConsumer({ stream, level: 'info' });
+    consumer.attach();
+
+    const logger = new Logger({ level: 'info' });
+    const err = new Error('request failed');
+    logger.error(err, { requestId: 'abc-123', retries: 3 });
+    consumer.flushSync();
+
+    const log = stream.logs[0];
+    assert.strictEqual(log.msg, 'request failed');
+    assert.ok(log.err.stack);
+    assert.strictEqual(log.requestId, 'abc-123');
+    assert.strictEqual(log.retries, 3);
+  });
+});
+
+describe('child() validation', () => {
+  it('should throw when options.serializers is not an object', () => {
+    const logger = new Logger();
+    assert.throws(() => {
+      logger.child({}, { serializers: 'not-an-object' });
+    }, { code: 'ERR_INVALID_ARG_TYPE' });
+  });
+
+  it('should throw when a child serializer value is not a function', () => {
+    const logger = new Logger();
+    assert.throws(() => {
+      logger.child({}, { serializers: { field: 99 } });
+    }, { code: 'ERR_INVALID_ARG_TYPE' });
+  });
+});
+
+describe('LogConsumer validation', () => {
+  it('should throw for invalid log level', () => {
+    assert.throws(() => {
+      new LogConsumer({ level: 'verbose' });
+    }, { code: 'ERR_INVALID_ARG_VALUE' });
+  });
+
+  it('should have correct enabled values for all six levels', () => {
+    const consumer = new LogConsumer({ level: 'warn' });
+    assert.strictEqual(consumer.trace.enabled, false);
+    assert.strictEqual(consumer.debug.enabled, false);
+    assert.strictEqual(consumer.info.enabled, false);
+    assert.strictEqual(consumer.warn.enabled, true);
+    assert.strictEqual(consumer.error.enabled, true);
+    assert.strictEqual(consumer.fatal.enabled, true);
+  });
+});
+
+describe('JSONConsumer stream options', () => {
+  it('should throw for an invalid stream type', () => {
+    assert.throws(() => {
+      new JSONConsumer({ stream: true });
+    }, { code: 'ERR_INVALID_ARG_TYPE' });
+  });
+
+  it('should accept a file descriptor number as stream', () => {
+    // fd 2 = stderr; use fatal-only to avoid noise in test output
+    // Should not throw
+    new JSONConsumer({ stream: 2, level: 'fatal' });
+  });
+
+  it('should throw when options.fields is not an object', () => {
+    assert.throws(() => {
+      new JSONConsumer({ fields: 'not-an-object' });
+    }, { code: 'ERR_INVALID_ARG_TYPE' });
+  });
+
+  it('should call the callback passed to flush()', () => {
+    const stream = new TestStream();
+    const consumer = new JSONConsumer({ stream, level: 'info' });
+    let called = false;
+    consumer.flush(() => { called = true; });
+    assert.strictEqual(called, true);
+  });
+
+  it('should close the underlying stream on end()', () => {
+    let ended = false;
+    const mockStream = {
+      write() {},
+      flush(cb) { if (cb) cb(); },
+      flushSync() {},
+      end() { ended = true; },
+    };
+    const consumer = new JSONConsumer({ stream: mockStream, level: 'info' });
+    consumer.end();
+    assert.strictEqual(ended, true);
+  });
+});