From d9a21ca1cdb6511687a26237fe1a4d71f569aa38 Mon Sep 17 00:00:00 2001 From: Reinaldy Rafli Date: Fri, 19 Sep 2025 13:30:46 +0700 Subject: [PATCH] feat: explicitly define logger Dear future minio-js maintainers: DO NOT USE CONSOLE AT ALL. It's painful to debug why a certain log suddenly pop up without a sensible message explaining (1) where the log came from, and (2) what caused this log to show up. With exposing the logger as an interface to the users, they can create a prefix or identifier that this log is coming from the MinIO JS SDK. --- src/internal/client.ts | 13 +++++++++++-- src/internal/logger.ts | 8 ++++++++ src/internal/request.ts | 5 +++-- src/internal/xml-parser.ts | 7 ++++--- 4 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 src/internal/logger.ts diff --git a/src/internal/client.ts b/src/internal/client.ts index 3b4520d1..3be8d21c 100644 --- a/src/internal/client.ts +++ b/src/internal/client.ts @@ -128,6 +128,7 @@ import { uploadPartParser, } from './xml-parser.ts' import * as xmlParsers from './xml-parser.ts' +import { Logger } from './logger.ts' const xml = new xml2js.Builder({ renderOpts: { pretty: false }, headless: true }) @@ -163,6 +164,7 @@ export interface ClientOptions { port?: number region?: Region transport?: Transport + logger?: Logger sessionToken?: string partSize?: number pathStyle?: boolean @@ -198,6 +200,7 @@ type Part = { export class TypedClient { protected transport: Transport + protected logger: Logger protected host: string protected port: number protected protocol: string @@ -295,6 +298,12 @@ export class TypedClient { transportAgent = params.transportAgent } + if (params.logger) { + this.logger = params.logger; + } else { + this.logger = console; + } + // User Agent should always following the below style. // Please open an issue to discuss any new changes here. // @@ -724,7 +733,7 @@ export class TypedClient { reqOptions.headers.authorization = signV4(reqOptions, this.accessKey, this.secretKey, region, date, sha256sum) } - const response = await requestWithRetry(this.transport, reqOptions, body) + const response = await requestWithRetry(this.transport, reqOptions, body, undefined, this.logger) if (!response.statusCode) { throw new Error("BUG: response doesn't have a statusCode") } @@ -2357,7 +2366,7 @@ export class TypedClient { const res = await this.makeRequestAsync({ method, bucketName, objectName, query }, payload) const body = await readAsBuffer(res) - return parseSelectObjectContentResponse(body) + return parseSelectObjectContentResponse(body, this.logger) } private async applyBucketLifecycle(bucketName: string, policyConfig: LifeCycleConfigParam): Promise { diff --git a/src/internal/logger.ts b/src/internal/logger.ts new file mode 100644 index 00000000..9b25f4d8 --- /dev/null +++ b/src/internal/logger.ts @@ -0,0 +1,8 @@ +/** + * Logger provides a common interface for logging. + * This interface is compatible with the default `console` logger. + */ +export interface Logger { + warn(message: string, ...optionalParams: never[]): void; + log(message: string, ...optionalParams: never[]): void; +} \ No newline at end of file diff --git a/src/internal/request.ts b/src/internal/request.ts index 25b9af47..654b7826 100644 --- a/src/internal/request.ts +++ b/src/internal/request.ts @@ -5,6 +5,7 @@ import { pipeline } from 'node:stream' import { promisify } from 'node:util' import type { Transport } from './type.ts' +import { Logger } from './logger.ts' const pipelineAsync = promisify(pipeline) @@ -63,6 +64,7 @@ export async function requestWithRetry( opt: https.RequestOptions, body: Buffer | string | stream.Readable | null = null, maxRetries: number = MAX_RETRIES, + logger: Logger, ): Promise { let attempt = 0 let isRetryable = false @@ -85,8 +87,7 @@ export async function requestWithRetry( throw new Error(`Request failed after ${maxRetries} retries: ${err}`) } const delay = getExpBackOffDelay(attempt) - // eslint-disable-next-line no-console - console.warn( + logger.warn( `${new Date().toLocaleString()} Retrying request (attempt ${attempt}/${maxRetries}) after ${delay}ms due to: ${err}`, ) await sleep(delay) diff --git a/src/internal/xml-parser.ts b/src/internal/xml-parser.ts index 84a520ef..707898ab 100644 --- a/src/internal/xml-parser.ts +++ b/src/internal/xml-parser.ts @@ -22,6 +22,7 @@ import type { Tags, } from './type.ts' import { RETENTION_VALIDITY_UNITS } from './type.ts' +import { Logger } from './logger.ts' // parse XML response for bucket region export function parseBucketRegion(xml: string): string { @@ -459,7 +460,7 @@ function extractHeaderValue(stream: stream.Readable) { return Buffer.from(stream.read(bodyLen)).toString() } -export function parseSelectObjectContentResponse(res: Buffer) { +export function parseSelectObjectContentResponse(res: Buffer, logger: Logger) { const selectResults = new SelectResults({}) // will be returned const responseStream = readableStream(res) // convert byte array to a readable responseStream @@ -577,9 +578,9 @@ export function parseSelectObjectContentResponse(res: Buffer) { default: { // Continuation message: Not sure if it is supported. did not find a reference or any message in response. // It does not have a payload. - const warningMessage = `Un implemented event detected ${messageType}.` + const warningMessage = `Unimplemented event detected ${messageType}.` // eslint-disable-next-line no-console - console.warn(warningMessage) + logger.warn(warningMessage) } } }