Fix default logger issue - remove console as default, add logger param support

baelter · baelter · commit 5bdb39ca2690 · 2025-09-11T11:48:45.000+02:00
diff --git a/src/amqp-base-client.ts b/src/amqp-base-client.ts
@@ -26,7 +26,7 @@ export abstract class AMQPBaseClient {
   frameMax: number
   heartbeat: number
   onerror: (error: AMQPError) => void
-  logger: Logger | null | undefined = console
+  logger: Logger | undefined
   /** Used for string -> arraybuffer when publishing */
   readonly textEncoder: InstanceType<typeof TextEncoder> = new TextEncoder()
   // Buffer pool for publishes, let multiple microtasks publish at the same time but save on allocations
@@ -35,8 +35,9 @@ export abstract class AMQPBaseClient {
   /**
    * @param name - name of the connection, set in client properties
    * @param platform - used in client properties
+   * @param logger - optional logger instance, defaults to undefined (no logging)
    */
-  constructor(vhost: string, username: string, password: string, name?: string, platform?: string, frameMax = 8192, heartbeat = 0, channelMax = 0) {
+  constructor(vhost: string, username: string, password: string, name?: string, platform?: string, frameMax = 8192, heartbeat = 0, channelMax = 0, logger?: Logger | null) {
     this.vhost = vhost
     this.username = username
     this.password = ""
@@ -46,6 +47,7 @@ export abstract class AMQPBaseClient {
     })
     if (name) this.name = name // connection name
     if (platform) this.platform = platform
+    this.logger = logger || undefined
     this.channels = [new AMQPChannel(this, 0)]
     this.onerror = (error: AMQPError) => this.logger?.error("amqp-client connection closed", error.message)
     if (frameMax < 8192) throw new Error("frameMax must be 8192 or larger")
@@ -318,7 +320,7 @@ export abstract class AMQPBaseClient {
                   break
                 }
                 case 71: { // update-secret-ok
-                  console.info("AMQP connection update secret ok")
+                  this.logger?.info("AMQP connection update secret ok")
                   this.onUpdateSecretOk?.()
                   delete this.onUpdateSecretOk
                   break
diff --git a/src/amqp-socket-client.ts b/src/amqp-socket-client.ts
@@ -1,6 +1,7 @@
 import { AMQPBaseClient } from './amqp-base-client.js'
 import { AMQPError } from './amqp-error.js'
 import type { AMQPTlsOptions } from './amqp-tls-options.js'
+import type { Logger } from './types.js'
 import { AMQPView } from './amqp-view.js'
 import { Buffer } from 'buffer'
 import * as net from 'net'
@@ -22,8 +23,10 @@ export class AMQPClient extends AMQPBaseClient {
 
   /**
    * @param url - uri to the server, example: amqp://user:passwd@localhost:5672/vhost
+   * @param tlsOptions - optional TLS options
+   * @param logger - optional logger instance, defaults to null (no logging)
    */
-  constructor(url: string, tlsOptions?: AMQPTlsOptions) {
+  constructor(url: string, tlsOptions?: AMQPTlsOptions, logger?: Logger | null) {
     const u = new URL(url)
     const vhost = decodeURIComponent(u.pathname.slice(1)) || "/"
     const username = decodeURIComponent(u.username) || "guest"
@@ -33,7 +36,7 @@ export class AMQPClient extends AMQPBaseClient {
     const heartbeat = parseInt(u.searchParams.get("heartbeat") || "0")
     const channelMax = parseInt(u.searchParams.get("channelMax") || "0")
     const platform = `${process.release.name} ${process.version} ${process.platform} ${process.arch}`
-    super(vhost, username, password, name, platform, frameMax, heartbeat, channelMax)
+    super(vhost, username, password, name, platform, frameMax, heartbeat, channelMax, logger)
     this.tls = u.protocol === "amqps:"
     this.tlsOptions = tlsOptions
     this.host = u.hostname || "localhost"
diff --git a/src/amqp-websocket-client.ts b/src/amqp-websocket-client.ts
@@ -5,6 +5,7 @@ import { AMQPChannel } from './amqp-channel.js'
 import { AMQPConsumer } from './amqp-consumer.js'
 import { AMQPMessage } from './amqp-message.js'
 import { AMQPQueue } from './amqp-queue.js'
+import type { Logger } from './types.js'
 
 interface AMQPWebSocketInit {
   url: string
@@ -14,6 +15,7 @@ interface AMQPWebSocketInit {
   name?: string
   frameMax?: number
   heartbeat?: number
+  logger?: Logger | null
 }
 
 /**
@@ -28,20 +30,22 @@ export class AMQPWebSocketClient extends AMQPBaseClient {
 
   /**
    * @param url to the websocket endpoint, example: wss://server/ws/amqp
+   * @param logger - optional logger instance, defaults to null (no logging)
    */
-  constructor(url: string, vhost?: string, username?: string, password?: string, name?: string, frameMax?: number, heartbeat?: number);
+  constructor(url: string, vhost?: string, username?: string, password?: string, name?: string, frameMax?: number, heartbeat?: number, logger?: Logger | null);
   constructor(init: AMQPWebSocketInit);
-  constructor(url: string | AMQPWebSocketInit, vhost = "/", username = "guest", password = "guest", name?: string, frameMax = 8192, heartbeat = 0) {
+  constructor(url: string | AMQPWebSocketInit, vhost = "/", username = "guest", password = "guest", name?: string, frameMax = 8192, heartbeat = 0, logger?: Logger | null) {
     if (typeof url === 'object') {
       vhost = url.vhost ?? vhost
       username = url.username ?? username
       password = url.password ?? password
       name = url.name ?? name
       frameMax = url.frameMax ?? frameMax
       heartbeat = url.heartbeat ?? heartbeat
+      logger = url.logger ?? logger
       url = url.url
     }
-    super(vhost, username, password, name, AMQPWebSocketClient.platform(), frameMax, heartbeat)
+    super(vhost, username, password, name, AMQPWebSocketClient.platform(), frameMax, heartbeat, 0, logger)
     this.url = url
     this.frameBuffer = new Uint8Array(frameMax)
   }
diff --git a/test/test.ts b/test/test.ts
@@ -1,5 +1,6 @@
 import { assert, expect, test, beforeEach, vi } from "vitest";
 import { AMQPClient } from '../src/amqp-socket-client.js';
+import { AMQPWebSocketClient } from '../src/amqp-websocket-client.js';
 import { AMQPMessage } from '../src/amqp-message.js';
 import type { AMQPError } from "../src/amqp-error.js";
 
@@ -750,5 +751,84 @@ test('can bind queues in parallel', async () => {
     q.bind('test-exchange', 'quux:*'),
   ])
 
-  await expect(Promise.resolve('success')).resolves.toBe('success')
+  // Verify the test completed successfully - parallel binds worked without hanging
+  expect(q.name).toBe('test-queue')
+})
+
+test('should have no logger by default', () => {
+  const amqp = getNewClient()
+  expect(amqp.logger).toBeUndefined()
+})
+
+test('should accept logger in AMQPClient constructor', () => {
+  const mockLogger = {
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn()
+  }
+
+  const amqp = new AMQPClient("amqp://127.0.0.1", undefined, mockLogger)
+  expect(amqp.logger).toBe(mockLogger)
+})
+
+test('should accept logger in AMQPWebSocketClient constructor', () => {
+  const mockLogger = {
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn()
+  }
+
+  const client = new AMQPWebSocketClient("wss://example.com/ws", undefined, undefined, undefined, undefined, undefined, undefined, mockLogger)
+  expect(client.logger).toBe(mockLogger)
+})
+
+test('should accept logger via AMQPWebSocketClient init object', () => {
+  const mockLogger = {
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn()
+  }
+
+  const client = new AMQPWebSocketClient({
+    url: "wss://example.com/ws",
+    logger: mockLogger
+  })
+  expect(client.logger).toBe(mockLogger)
+})
+
+test('should not log when logger is null', () => {
+  const amqp = getNewClient()
+
+  // This should not throw even with null logger
+  expect(() => {
+    amqp.logger?.error("test message")
+    amqp.logger?.warn("test message")
+    amqp.logger?.info("test message")
+    amqp.logger?.debug("test message")
+  }).not.toThrow()
+})
+
+test('should use provided logger when available', () => {
+  const mockLogger = {
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn()
+  }
+
+  const amqp = new AMQPClient("amqp://127.0.0.1", undefined, mockLogger)
+
+  // Call logger methods directly (simulating how they're used in the code)
+  amqp.logger?.error("error message")
+  amqp.logger?.warn("warn message")
+  amqp.logger?.info("info message")
+  amqp.logger?.debug("debug message")
+
+  expect(mockLogger.error).toHaveBeenCalledWith("error message")
+  expect(mockLogger.warn).toHaveBeenCalledWith("warn message")
+  expect(mockLogger.info).toHaveBeenCalledWith("info message")
+  expect(mockLogger.debug).toHaveBeenCalledWith("debug message")
 })
