From bd1691be945284f5870d71b4ba3e257c07a09af5 Mon Sep 17 00:00:00 2001 From: John Lwin Date: Tue, 21 Jul 2026 01:08:20 -0700 Subject: [PATCH 01/11] feat(node-http-handler): pass client logger to request handlers --- .../pass-client-logger-to-request-handler.md | 17 ++++ .../httpExtensionConfiguration.spec.ts | 82 +++++++++++++++++++ .../extensions/httpExtensionConfiguration.ts | 21 +++++ .../src/node-http-handler.spec.ts | 53 ++++++++++++ .../src/node-http-handler.ts | 3 + .../src/undici-http-handler.spec.ts | 13 ++- .../src/undici-http-handler.ts | 6 +- 7 files changed, 191 insertions(+), 4 deletions(-) create mode 100644 .changeset/pass-client-logger-to-request-handler.md create mode 100644 packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts diff --git a/.changeset/pass-client-logger-to-request-handler.md b/.changeset/pass-client-logger-to-request-handler.md new file mode 100644 index 00000000000..48316e49673 --- /dev/null +++ b/.changeset/pass-client-logger-to-request-handler.md @@ -0,0 +1,17 @@ +--- +"@smithy/core": minor +"@smithy/node-http-handler": minor +"@smithy/undici-http-handler": minor +--- + +feat: pass client logger to request handlers + +Injects the client-level logger into the request handler during +`getHttpHandlerExtensionConfiguration`, so handler-emitted diagnostics +(e.g. socket exhaustion warnings) route through the customer's logger +instead of `console.warn`. + +Skips injection when the logger is `NoOpLogger` (the default), +preserving the existing console fallback for customers who never set a logger. +Handler-level loggers take precedence via nullish assignment in +`updateHttpClientConfig`. diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts new file mode 100644 index 00000000000..0eee09fcde9 --- /dev/null +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts @@ -0,0 +1,82 @@ +import { describe, expect, it, vi } from "vitest"; + +import { getHttpHandlerExtensionConfiguration } from "./httpExtensionConfiguration"; + +describe("getHttpHandlerExtensionConfiguration", () => { + const createMockHandler = () => ({ + metadata: { handlerProtocol: "http/1.1" }, + handle: vi.fn(), + updateHttpClientConfig: vi.fn(), + httpHandlerConfigs: vi.fn().mockReturnValue({}), + }); + + const createMockLogger = () => ({ + trace: vi.fn(), + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }); + + describe("client logger injection", () => { + it("injects logger into requestHandler when logger is explicitly set", () => { + const handler = createMockHandler(); + const logger = createMockLogger(); + + getHttpHandlerExtensionConfiguration({ requestHandler: handler, logger } as any); + + expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("logger", logger); + }); + + it("injects logger into httpHandler when requestHandler is absent", () => { + const handler = createMockHandler(); + const logger = createMockLogger(); + + getHttpHandlerExtensionConfiguration({ httpHandler: handler, logger } as any); + + expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("logger", logger); + }); + + it("prefers httpHandler over requestHandler", () => { + const httpHandler = createMockHandler(); + const requestHandler = createMockHandler(); + const logger = createMockLogger(); + + getHttpHandlerExtensionConfiguration({ httpHandler, requestHandler, logger } as any); + + expect(httpHandler.updateHttpClientConfig).toHaveBeenCalledWith("logger", logger); + expect(requestHandler.updateHttpClientConfig).not.toHaveBeenCalled(); + }); + + it("does not inject NoOpLogger", () => { + const handler = createMockHandler(); + + class NoOpLogger { + trace() {} + debug() {} + info() {} + warn() {} + error() {} + } + + getHttpHandlerExtensionConfiguration({ requestHandler: handler, logger: new NoOpLogger() } as any); + + expect(handler.updateHttpClientConfig).not.toHaveBeenCalled(); + }); + + it("does not inject when no logger is provided", () => { + const handler = createMockHandler(); + + getHttpHandlerExtensionConfiguration({ requestHandler: handler } as any); + + expect(handler.updateHttpClientConfig).not.toHaveBeenCalled(); + }); + + it("does not inject when no handler is present", () => { + const logger = createMockLogger(); + + // should not throw + getHttpHandlerExtensionConfiguration({ logger } as any); + }); + }); +}); diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts index d5bb23f2a2e..382e374ebf9 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts @@ -1,3 +1,5 @@ +import type { Logger } from "@smithy/types"; + import type { HttpHandler } from "../httpHandler"; /** @@ -17,6 +19,16 @@ export type HttpHandlerExtensionConfigType = httpHandler: HttpHandler; }>; +/** + * @internal + * + * Returns true if the logger is a no-op (all methods are empty), + * indicating the customer never explicitly configured one. + */ +const isNoOpLogger = (logger: Logger): boolean => { + return logger.constructor?.name === "NoOpLogger"; +}; + /** * Helper function to resolve default extension configuration from runtime config * @@ -25,6 +37,15 @@ export type HttpHandlerExtensionConfigType = export const getHttpHandlerExtensionConfiguration = ( runtimeConfig: HttpHandlerExtensionConfigType ) => { + const rc = runtimeConfig as HttpHandlerExtensionConfigType & { + requestHandler?: HttpHandler; + logger?: Logger; + }; + const handler = rc.httpHandler ?? rc.requestHandler; + if (handler && rc.logger && !isNoOpLogger(rc.logger)) { + handler.updateHttpClientConfig("logger" as keyof HandlerConfig, rc.logger as HandlerConfig[keyof HandlerConfig]); + } + return { setHttpHandler(handler: HttpHandler): void { runtimeConfig.httpHandler = handler; diff --git a/packages/node-http-handler/src/node-http-handler.spec.ts b/packages/node-http-handler/src/node-http-handler.spec.ts index 1867ee4b002..65cf6999c75 100644 --- a/packages/node-http-handler/src/node-http-handler.spec.ts +++ b/packages/node-http-handler/src/node-http-handler.spec.ts @@ -485,6 +485,59 @@ describe("NodeHttpHandler", () => { }); }); + describe("updateHttpClientConfig", () => { + it("uses nullish assignment for the logger key", async () => { + const handlerLogger = { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const handler = new NodeHttpHandler({ logger: handlerLogger }); + + const clientLogger = { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + handler.updateHttpClientConfig("logger", clientLogger); + + // trigger config resolution + const request = new HttpRequest({ hostname: "localhost", method: "GET", protocol: "https:", path: "/" }); + try { + await handler.handle(request); + } catch { + // ignore request errors, we just need config to resolve + } + + const configs = handler.httpHandlerConfigs(); + expect(configs.logger).toBe(handlerLogger); + }); + + it("sets the logger when none was provided in handler options", async () => { + const handler = new NodeHttpHandler(); + + const clientLogger = { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + handler.updateHttpClientConfig("logger", clientLogger); + + const request = new HttpRequest({ hostname: "localhost", method: "GET", protocol: "https:", path: "/" }); + try { + await handler.handle(request); + } catch { + // ignore request errors + } + + const configs = handler.httpHandlerConfigs(); + expect(configs.logger).toBe(clientLogger); + }); + + it("overwrites for non-logger keys", async () => { + const handler = new NodeHttpHandler({ requestTimeout: 1000 }); + handler.updateHttpClientConfig("requestTimeout", 5000); + + const request = new HttpRequest({ hostname: "localhost", method: "GET", protocol: "https:", path: "/" }); + try { + await handler.handle(request); + } catch { + // ignore request errors + } + + const configs = handler.httpHandlerConfigs(); + expect(configs.requestTimeout).toBe(5000); + }); + }); + describe("checkSocketUsage", () => { beforeEach(() => { vi.spyOn(console, "warn").mockImplementation(vi.fn() as any); diff --git a/packages/node-http-handler/src/node-http-handler.ts b/packages/node-http-handler/src/node-http-handler.ts index 7990cd3addd..6d24e317cdb 100644 --- a/packages/node-http-handler/src/node-http-handler.ts +++ b/packages/node-http-handler/src/node-http-handler.ts @@ -325,6 +325,9 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf public updateHttpClientConfig(key: keyof NodeHttpHandlerOptions, value: NodeHttpHandlerOptions[typeof key]): void { this.config = undefined; this.configProvider = this.configProvider.then((config) => { + if (key === "logger") { + return { ...config, logger: config.logger ?? value }; + } return { ...config, [key]: value, diff --git a/packages/undici-http-handler/src/undici-http-handler.spec.ts b/packages/undici-http-handler/src/undici-http-handler.spec.ts index a11cc742c66..94a06ed7cd1 100644 --- a/packages/undici-http-handler/src/undici-http-handler.spec.ts +++ b/packages/undici-http-handler/src/undici-http-handler.spec.ts @@ -561,15 +561,22 @@ describe("UndiciHttpHandler", () => { expect(configs.logger).toBe(logger); }); - it("updates config", async () => { + it("does not overwrite an existing logger via updateHttpClientConfig", async () => { const logger = createMockLogger(); const updatedLogger = createMockLogger(); handler = new UndiciHttpHandler({ logger }); await handler.handle(createMockRequest()); handler.updateHttpClientConfig("logger", updatedLogger); - // Config is reset, need another request to resolve await handler.handle(createMockRequest()); - expect(handler.httpHandlerConfigs().logger).toBe(updatedLogger); + expect(handler.httpHandlerConfigs().logger).toBe(logger); + }); + + it("sets logger via updateHttpClientConfig when none was provided", async () => { + handler = new UndiciHttpHandler(); + const clientLogger = createMockLogger(); + handler.updateHttpClientConfig("logger", clientLogger); + await handler.handle(createMockRequest()); + expect(handler.httpHandlerConfigs().logger).toBe(clientLogger); }); it("retains existing dispatcher if undefined is passed", () => { diff --git a/packages/undici-http-handler/src/undici-http-handler.ts b/packages/undici-http-handler/src/undici-http-handler.ts index 0ad62a80adb..98f7a362c67 100644 --- a/packages/undici-http-handler/src/undici-http-handler.ts +++ b/packages/undici-http-handler/src/undici-http-handler.ts @@ -214,7 +214,11 @@ export class UndiciHttpHandler implements HttpHandler value: UndiciHttpHandlerOptions[K] ): void { if (key !== "dispatcher") { - (this.config as any)[key] = value; + if (key === "logger") { + (this.config as any)[key] ??= value; + } else { + (this.config as any)[key] = value; + } return; } From 66718983cf81ebe071db36561f285bdb5794ebc3 Mon Sep 17 00:00:00 2001 From: John Lwin Date: Tue, 21 Jul 2026 01:37:29 -0700 Subject: [PATCH 02/11] fix: cast logger value type in updateHttpClientConfig TypeScript strict mode cannot narrow `value` from the full `NodeHttpHandlerOptions[typeof key]` union when branching on `key === "logger"`. Add an explicit cast to fix the build:types target in CI. --- packages/node-http-handler/src/node-http-handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node-http-handler/src/node-http-handler.ts b/packages/node-http-handler/src/node-http-handler.ts index 6d24e317cdb..8e13c0968cb 100644 --- a/packages/node-http-handler/src/node-http-handler.ts +++ b/packages/node-http-handler/src/node-http-handler.ts @@ -326,7 +326,7 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf this.config = undefined; this.configProvider = this.configProvider.then((config) => { if (key === "logger") { - return { ...config, logger: config.logger ?? value }; + return { ...config, logger: config.logger ?? (value as NodeHttpHandlerOptions["logger"]) }; } return { ...config, From 418fa747a19d76076cfe83c29dd43568779f6ec9 Mon Sep 17 00:00:00 2001 From: John Lwin Date: Thu, 23 Jul 2026 22:48:40 -0700 Subject: [PATCH 03/11] simplify logger injection per team decision --- .../pass-client-logger-to-request-handler.md | 10 ---- .../httpExtensionConfiguration.spec.ts | 50 +++---------------- .../extensions/httpExtensionConfiguration.ts | 21 +------- 3 files changed, 8 insertions(+), 73 deletions(-) diff --git a/.changeset/pass-client-logger-to-request-handler.md b/.changeset/pass-client-logger-to-request-handler.md index 48316e49673..9821a38dd5c 100644 --- a/.changeset/pass-client-logger-to-request-handler.md +++ b/.changeset/pass-client-logger-to-request-handler.md @@ -5,13 +5,3 @@ --- feat: pass client logger to request handlers - -Injects the client-level logger into the request handler during -`getHttpHandlerExtensionConfiguration`, so handler-emitted diagnostics -(e.g. socket exhaustion warnings) route through the customer's logger -instead of `console.warn`. - -Skips injection when the logger is `NoOpLogger` (the default), -preserving the existing console fallback for customers who never set a logger. -Handler-level loggers take precedence via nullish assignment in -`updateHttpClientConfig`. diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts index 0eee09fcde9..d77ee950624 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts @@ -19,16 +19,7 @@ describe("getHttpHandlerExtensionConfiguration", () => { }); describe("client logger injection", () => { - it("injects logger into requestHandler when logger is explicitly set", () => { - const handler = createMockHandler(); - const logger = createMockLogger(); - - getHttpHandlerExtensionConfiguration({ requestHandler: handler, logger } as any); - - expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("logger", logger); - }); - - it("injects logger into httpHandler when requestHandler is absent", () => { + it("passes logger to httpHandler via updateHttpClientConfig", () => { const handler = createMockHandler(); const logger = createMockLogger(); @@ -37,46 +28,19 @@ describe("getHttpHandlerExtensionConfiguration", () => { expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("logger", logger); }); - it("prefers httpHandler over requestHandler", () => { - const httpHandler = createMockHandler(); - const requestHandler = createMockHandler(); + it("does not throw when no handler is present", () => { const logger = createMockLogger(); - getHttpHandlerExtensionConfiguration({ httpHandler, requestHandler, logger } as any); - - expect(httpHandler.updateHttpClientConfig).toHaveBeenCalledWith("logger", logger); - expect(requestHandler.updateHttpClientConfig).not.toHaveBeenCalled(); - }); - - it("does not inject NoOpLogger", () => { - const handler = createMockHandler(); - - class NoOpLogger { - trace() {} - debug() {} - info() {} - warn() {} - error() {} - } - - getHttpHandlerExtensionConfiguration({ requestHandler: handler, logger: new NoOpLogger() } as any); - - expect(handler.updateHttpClientConfig).not.toHaveBeenCalled(); + // should not throw + getHttpHandlerExtensionConfiguration({ logger } as any); }); - it("does not inject when no logger is provided", () => { + it("does not throw when no logger is provided", () => { const handler = createMockHandler(); - getHttpHandlerExtensionConfiguration({ requestHandler: handler } as any); + getHttpHandlerExtensionConfiguration({ httpHandler: handler } as any); - expect(handler.updateHttpClientConfig).not.toHaveBeenCalled(); - }); - - it("does not inject when no handler is present", () => { - const logger = createMockLogger(); - - // should not throw - getHttpHandlerExtensionConfiguration({ logger } as any); + expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("logger", undefined); }); }); }); diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts index 382e374ebf9..2d3306fd7fd 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts @@ -1,5 +1,3 @@ -import type { Logger } from "@smithy/types"; - import type { HttpHandler } from "../httpHandler"; /** @@ -19,16 +17,6 @@ export type HttpHandlerExtensionConfigType = httpHandler: HttpHandler; }>; -/** - * @internal - * - * Returns true if the logger is a no-op (all methods are empty), - * indicating the customer never explicitly configured one. - */ -const isNoOpLogger = (logger: Logger): boolean => { - return logger.constructor?.name === "NoOpLogger"; -}; - /** * Helper function to resolve default extension configuration from runtime config * @@ -37,14 +25,7 @@ const isNoOpLogger = (logger: Logger): boolean => { export const getHttpHandlerExtensionConfiguration = ( runtimeConfig: HttpHandlerExtensionConfigType ) => { - const rc = runtimeConfig as HttpHandlerExtensionConfigType & { - requestHandler?: HttpHandler; - logger?: Logger; - }; - const handler = rc.httpHandler ?? rc.requestHandler; - if (handler && rc.logger && !isNoOpLogger(rc.logger)) { - handler.updateHttpClientConfig("logger" as keyof HandlerConfig, rc.logger as HandlerConfig[keyof HandlerConfig]); - } + runtimeConfig.httpHandler?.updateHttpClientConfig("logger" as keyof HandlerConfig, (runtimeConfig as any).logger); return { setHttpHandler(handler: HttpHandler): void { From a43865aa0a8137818e4640d529c2d18589937fa4 Mon Sep 17 00:00:00 2001 From: John Lwin Date: Fri, 24 Jul 2026 10:30:19 -0700 Subject: [PATCH 04/11] remove nullish assignments and add truthy check --- .../httpExtensionConfiguration.spec.ts | 16 ++++++------- .../extensions/httpExtensionConfiguration.ts | 8 +++++-- .../src/node-http-handler.spec.ts | 23 ++----------------- .../src/node-http-handler.ts | 3 --- .../src/undici-http-handler.spec.ts | 12 ++-------- .../src/undici-http-handler.ts | 6 +---- 6 files changed, 19 insertions(+), 49 deletions(-) diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts index d77ee950624..14096db5a93 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts @@ -28,19 +28,19 @@ describe("getHttpHandlerExtensionConfiguration", () => { expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("logger", logger); }); + it("does not call updateHttpClientConfig when logger is not set", () => { + const handler = createMockHandler(); + + getHttpHandlerExtensionConfiguration({ httpHandler: handler } as any); + + expect(handler.updateHttpClientConfig).not.toHaveBeenCalled(); + }); + it("does not throw when no handler is present", () => { const logger = createMockLogger(); // should not throw getHttpHandlerExtensionConfiguration({ logger } as any); }); - - it("does not throw when no logger is provided", () => { - const handler = createMockHandler(); - - getHttpHandlerExtensionConfiguration({ httpHandler: handler } as any); - - expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("logger", undefined); - }); }); }); diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts index 2d3306fd7fd..f12df9b3468 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts @@ -1,3 +1,5 @@ +import type { Logger } from "@smithy/types"; + import type { HttpHandler } from "../httpHandler"; /** @@ -22,10 +24,12 @@ export type HttpHandlerExtensionConfigType = * * @internal */ -export const getHttpHandlerExtensionConfiguration = ( +export const getHttpHandlerExtensionConfiguration = ( runtimeConfig: HttpHandlerExtensionConfigType ) => { - runtimeConfig.httpHandler?.updateHttpClientConfig("logger" as keyof HandlerConfig, (runtimeConfig as any).logger); + if ((runtimeConfig as any).logger) { + runtimeConfig.httpHandler?.updateHttpClientConfig("logger" as keyof HandlerConfig, (runtimeConfig as any).logger); + } return { setHttpHandler(handler: HttpHandler): void { diff --git a/packages/node-http-handler/src/node-http-handler.spec.ts b/packages/node-http-handler/src/node-http-handler.spec.ts index 65cf6999c75..a670114252c 100644 --- a/packages/node-http-handler/src/node-http-handler.spec.ts +++ b/packages/node-http-handler/src/node-http-handler.spec.ts @@ -486,26 +486,7 @@ describe("NodeHttpHandler", () => { }); describe("updateHttpClientConfig", () => { - it("uses nullish assignment for the logger key", async () => { - const handlerLogger = { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; - const handler = new NodeHttpHandler({ logger: handlerLogger }); - - const clientLogger = { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; - handler.updateHttpClientConfig("logger", clientLogger); - - // trigger config resolution - const request = new HttpRequest({ hostname: "localhost", method: "GET", protocol: "https:", path: "/" }); - try { - await handler.handle(request); - } catch { - // ignore request errors, we just need config to resolve - } - - const configs = handler.httpHandlerConfigs(); - expect(configs.logger).toBe(handlerLogger); - }); - - it("sets the logger when none was provided in handler options", async () => { + it("updates the logger", async () => { const handler = new NodeHttpHandler(); const clientLogger = { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; @@ -522,7 +503,7 @@ describe("NodeHttpHandler", () => { expect(configs.logger).toBe(clientLogger); }); - it("overwrites for non-logger keys", async () => { + it("updates non-logger keys", async () => { const handler = new NodeHttpHandler({ requestTimeout: 1000 }); handler.updateHttpClientConfig("requestTimeout", 5000); diff --git a/packages/node-http-handler/src/node-http-handler.ts b/packages/node-http-handler/src/node-http-handler.ts index 8e13c0968cb..7990cd3addd 100644 --- a/packages/node-http-handler/src/node-http-handler.ts +++ b/packages/node-http-handler/src/node-http-handler.ts @@ -325,9 +325,6 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf public updateHttpClientConfig(key: keyof NodeHttpHandlerOptions, value: NodeHttpHandlerOptions[typeof key]): void { this.config = undefined; this.configProvider = this.configProvider.then((config) => { - if (key === "logger") { - return { ...config, logger: config.logger ?? (value as NodeHttpHandlerOptions["logger"]) }; - } return { ...config, [key]: value, diff --git a/packages/undici-http-handler/src/undici-http-handler.spec.ts b/packages/undici-http-handler/src/undici-http-handler.spec.ts index 94a06ed7cd1..f2c5f612816 100644 --- a/packages/undici-http-handler/src/undici-http-handler.spec.ts +++ b/packages/undici-http-handler/src/undici-http-handler.spec.ts @@ -561,22 +561,14 @@ describe("UndiciHttpHandler", () => { expect(configs.logger).toBe(logger); }); - it("does not overwrite an existing logger via updateHttpClientConfig", async () => { + it("updates logger via updateHttpClientConfig", async () => { const logger = createMockLogger(); const updatedLogger = createMockLogger(); handler = new UndiciHttpHandler({ logger }); await handler.handle(createMockRequest()); handler.updateHttpClientConfig("logger", updatedLogger); await handler.handle(createMockRequest()); - expect(handler.httpHandlerConfigs().logger).toBe(logger); - }); - - it("sets logger via updateHttpClientConfig when none was provided", async () => { - handler = new UndiciHttpHandler(); - const clientLogger = createMockLogger(); - handler.updateHttpClientConfig("logger", clientLogger); - await handler.handle(createMockRequest()); - expect(handler.httpHandlerConfigs().logger).toBe(clientLogger); + expect(handler.httpHandlerConfigs().logger).toBe(updatedLogger); }); it("retains existing dispatcher if undefined is passed", () => { diff --git a/packages/undici-http-handler/src/undici-http-handler.ts b/packages/undici-http-handler/src/undici-http-handler.ts index 98f7a362c67..0ad62a80adb 100644 --- a/packages/undici-http-handler/src/undici-http-handler.ts +++ b/packages/undici-http-handler/src/undici-http-handler.ts @@ -214,11 +214,7 @@ export class UndiciHttpHandler implements HttpHandler value: UndiciHttpHandlerOptions[K] ): void { if (key !== "dispatcher") { - if (key === "logger") { - (this.config as any)[key] ??= value; - } else { - (this.config as any)[key] = value; - } + (this.config as any)[key] = value; return; } From 6f96ae302c4af80d3b9071652495797db8a0c1b8 Mon Sep 17 00:00:00 2001 From: John Lwin Date: Mon, 3 Aug 2026 10:21:17 -0700 Subject: [PATCH 05/11] feat(core): offer client logger to request handlers as a fallback --- .../pass-client-logger-to-request-handler.md | 3 +- .../core/src/submodules/protocols/index.ts | 1 + .../extensions/httpExtensionConfiguration.ts | 8 +++-- .../protocols/protocol-http/fallbackLogger.ts | 11 +++++++ .../protocols/protocol-http/httpHandler.ts | 12 ++++++- .../src/fetch-http-handler.ts | 27 +++++++++++++--- .../src/node-http-handler.ts | 32 ++++++++++++++----- .../src/undici-http-handler.ts | 23 ++++++++++++- 8 files changed, 99 insertions(+), 18 deletions(-) create mode 100644 packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts diff --git a/.changeset/pass-client-logger-to-request-handler.md b/.changeset/pass-client-logger-to-request-handler.md index 9821a38dd5c..7187ae5548b 100644 --- a/.changeset/pass-client-logger-to-request-handler.md +++ b/.changeset/pass-client-logger-to-request-handler.md @@ -1,7 +1,8 @@ --- "@smithy/core": minor +"@smithy/fetch-http-handler": minor "@smithy/node-http-handler": minor "@smithy/undici-http-handler": minor --- -feat: pass client logger to request handlers +feat: offer the client logger to request handlers as a fallback, without overwriting a handler's own logger diff --git a/packages/core/src/submodules/protocols/index.ts b/packages/core/src/submodules/protocols/index.ts index 4e8d64c64f5..f26a9073d9f 100644 --- a/packages/core/src/submodules/protocols/index.ts +++ b/packages/core/src/submodules/protocols/index.ts @@ -16,6 +16,7 @@ export { SerdeContext } from "./SerdeContext"; export { Field } from "./protocol-http/Field"; export { Fields, type FieldsOptions } from "./protocol-http/Fields"; export { type HttpHandler, type HttpHandlerUserInput } from "./protocol-http/httpHandler"; +export { FALLBACK_LOGGER } from "./protocol-http/fallbackLogger"; export { HttpRequest, type IHttpRequest } from "@smithy/core/transport"; export { HttpResponse } from "@smithy/core/transport"; export { isValidHostname } from "@smithy/core/transport"; diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts index f12df9b3468..bc2c15f170b 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts @@ -1,5 +1,6 @@ import type { Logger } from "@smithy/types"; +import { FALLBACK_LOGGER } from "../fallbackLogger"; import type { HttpHandler } from "../httpHandler"; /** @@ -25,10 +26,11 @@ export type HttpHandlerExtensionConfigType = * @internal */ export const getHttpHandlerExtensionConfiguration = ( - runtimeConfig: HttpHandlerExtensionConfigType + runtimeConfig: HttpHandlerExtensionConfigType & { logger?: Logger } ) => { - if ((runtimeConfig as any).logger) { - runtimeConfig.httpHandler?.updateHttpClientConfig("logger" as keyof HandlerConfig, (runtimeConfig as any).logger); + // Offered as a fallback only: the handler keeps its own logger if it has one. + if (runtimeConfig.logger) { + runtimeConfig.httpHandler?.updateHttpClientConfig?.(FALLBACK_LOGGER, runtimeConfig.logger); } return { diff --git a/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts b/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts new file mode 100644 index 00000000000..d400c919d1e --- /dev/null +++ b/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts @@ -0,0 +1,11 @@ +/** + * Key with which a client offers its logger to an HttpHandler via + * `updateHttpClientConfig`, to be used only if the handler has no logger + * of its own. + * + * A symbol keeps this off the handlers' public options types. `Symbol.for` + * makes the key shared between duplicate copies of this package. + * + * @internal + */ +export const FALLBACK_LOGGER: unique symbol = Symbol.for("smithy.httpHandler.fallbackLogger"); diff --git a/packages/core/src/submodules/protocols/protocol-http/httpHandler.ts b/packages/core/src/submodules/protocols/protocol-http/httpHandler.ts index d2230d14ead..8b3130eca66 100644 --- a/packages/core/src/submodules/protocols/protocol-http/httpHandler.ts +++ b/packages/core/src/submodules/protocols/protocol-http/httpHandler.ts @@ -2,10 +2,13 @@ import type { HttpRequest, HttpResponse } from "@smithy/core/transport"; import type { FetchHttpHandlerOptions, HttpHandlerOptions, + Logger, NodeHttpHandlerOptions, RequestHandler, } from "@smithy/types"; +import type { FALLBACK_LOGGER } from "./fallbackLogger"; + /** * @internal */ @@ -16,8 +19,15 @@ export type HttpHandler = RequestHandler< > & { /** * @internal + * + * The key may also be {@link FALLBACK_LOGGER}, with which a client offers its + * logger for use only when the handler has no logger of its own. Handlers + * that predate that key ignore it. */ - updateHttpClientConfig(key: keyof HttpHandlerConfig, value: HttpHandlerConfig[typeof key]): void; + updateHttpClientConfig( + key: keyof HttpHandlerConfig | typeof FALLBACK_LOGGER, + value: HttpHandlerConfig[keyof HttpHandlerConfig] | Logger + ): void; /** * @internal diff --git a/packages/fetch-http-handler/src/fetch-http-handler.ts b/packages/fetch-http-handler/src/fetch-http-handler.ts index 88c7546fa6c..cf8acbc416f 100644 --- a/packages/fetch-http-handler/src/fetch-http-handler.ts +++ b/packages/fetch-http-handler/src/fetch-http-handler.ts @@ -1,5 +1,11 @@ -import { HttpResponse, buildQueryString, type HttpHandler, type HttpRequest } from "@smithy/core/protocols"; -import type { FetchHttpHandlerOptions, HeaderBag, HttpHandlerOptions, Provider } from "@smithy/types"; +import { + FALLBACK_LOGGER, + HttpResponse, + buildQueryString, + type HttpHandler, + type HttpRequest, +} from "@smithy/core/protocols"; +import type { FetchHttpHandlerOptions, HeaderBag, HttpHandlerOptions, Logger, Provider } from "@smithy/types"; import { createRequest } from "./create-request"; import { requestTimeout as requestTimeoutFn } from "./request-timeout"; @@ -38,6 +44,10 @@ export type AdditionalRequestParameters = { export class FetchHttpHandler implements HttpHandler { private config?: FetchHttpHandlerOptions; private configProvider: Promise; + /** + * Client logger, used only when this handler has no logger of its own. + */ + private fallbackLogger?: Logger; /** * @returns the input if it is an HttpHandler of any class, @@ -200,10 +210,19 @@ export class FetchHttpHandler implements HttpHandler { return Promise.race(raceOfPromises).finally(removeSignalEventListener); } - updateHttpClientConfig(key: keyof FetchHttpHandlerOptions, value: FetchHttpHandlerOptions[typeof key]): void { + updateHttpClientConfig(key: typeof FALLBACK_LOGGER, value: Logger): void; + updateHttpClientConfig(key: keyof FetchHttpHandlerOptions, value: FetchHttpHandlerOptions[typeof key]): void; + updateHttpClientConfig( + key: keyof FetchHttpHandlerOptions | typeof FALLBACK_LOGGER, + value: FetchHttpHandlerOptions[keyof FetchHttpHandlerOptions] | Logger + ): void { + if (key === FALLBACK_LOGGER) { + this.fallbackLogger = value as Logger; + return; + } this.config = undefined; this.configProvider = this.configProvider.then((config) => { - (config as Record)[key] = value; + (config as Record)[key as string] = value; return config; }); } diff --git a/packages/node-http-handler/src/node-http-handler.ts b/packages/node-http-handler/src/node-http-handler.ts index 7990cd3addd..7eb0174659b 100644 --- a/packages/node-http-handler/src/node-http-handler.ts +++ b/packages/node-http-handler/src/node-http-handler.ts @@ -1,6 +1,12 @@ import type { Agent as hAgentType, request as hRequestType } from "node:http"; import type { RequestOptions, Agent as hsAgentType } from "node:https"; -import { HttpResponse, buildQueryString, type HttpHandler, type HttpRequest } from "@smithy/core/protocols"; +import { + FALLBACK_LOGGER, + HttpResponse, + buildQueryString, + type HttpHandler, + type HttpRequest, +} from "@smithy/core/protocols"; import type { HttpHandlerOptions, Logger, NodeHttpHandlerOptions, Provider } from "@smithy/types"; import { buildAbortError } from "./build-abort-error"; @@ -42,6 +48,10 @@ export class NodeHttpHandler implements HttpHandler { private configProvider: Promise; private socketWarningTimestamp = 0; private externalAgent = false; + /** + * Client logger, used only when this handler has no logger of its own. + */ + private fallbackLogger?: Logger; // Node http handler is hard-coded to http/1.1: https://github.com/nodejs/node/blob/ff5664b83b89c55e4ab5d5f60068fb457f1f5872/lib/_http_server.js#L286 public readonly metadata = { handlerProtocol: "http/1.1" }; @@ -143,6 +153,7 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf } const config = this.config!; + const logger = config.logger ?? this.fallbackLogger; // determine which http(s) client to use const isSSL = request.protocol === "https:"; @@ -208,11 +219,7 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf // This warning will be cancelled if the request resolves. socketWarningTimeoutId = timing.setTimeout( () => { - this.socketWarningTimestamp = NodeHttpHandler.checkSocketUsage( - agent!, - this.socketWarningTimestamp, - config.logger - ); + this.socketWarningTimestamp = NodeHttpHandler.checkSocketUsage(agent!, this.socketWarningTimestamp, logger); }, config.socketAcquisitionWarningTimeout ?? (config.requestTimeout ?? 2000) + (config.connectionTimeout ?? 1000) ); @@ -298,7 +305,7 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf reject, effectiveRequestTimeout, config.throwOnRequestTimeout, - config.logger ?? console + logger ?? console ); socketTimeoutId = setSocketTimeout(req, reject, config.socketTimeout); @@ -322,7 +329,16 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf }); } - public updateHttpClientConfig(key: keyof NodeHttpHandlerOptions, value: NodeHttpHandlerOptions[typeof key]): void { + public updateHttpClientConfig(key: typeof FALLBACK_LOGGER, value: Logger): void; + public updateHttpClientConfig(key: keyof NodeHttpHandlerOptions, value: NodeHttpHandlerOptions[typeof key]): void; + public updateHttpClientConfig( + key: keyof NodeHttpHandlerOptions | typeof FALLBACK_LOGGER, + value: NodeHttpHandlerOptions[keyof NodeHttpHandlerOptions] | Logger + ): void { + if (key === FALLBACK_LOGGER) { + this.fallbackLogger = value as Logger; + return; + } this.config = undefined; this.configProvider = this.configProvider.then((config) => { return { diff --git a/packages/undici-http-handler/src/undici-http-handler.ts b/packages/undici-http-handler/src/undici-http-handler.ts index 0ad62a80adb..34a0486af18 100644 --- a/packages/undici-http-handler/src/undici-http-handler.ts +++ b/packages/undici-http-handler/src/undici-http-handler.ts @@ -1,5 +1,11 @@ import type { Readable } from "node:stream"; -import { HttpResponse, buildQueryString, type HttpHandler, type HttpRequest } from "@smithy/core/protocols"; +import { + FALLBACK_LOGGER, + HttpResponse, + buildQueryString, + type HttpHandler, + type HttpRequest, +} from "@smithy/core/protocols"; import type { HttpHandlerOptions, Logger } from "@smithy/types"; import { Agent, Dispatcher, getGlobalDispatcher } from "undici"; @@ -67,6 +73,11 @@ export class UndiciHttpHandler implements HttpHandler */ private internalAgentOptions?: Agent.Options; + /** + * Client logger, used only when this handler has no logger of its own. + */ + private fallbackLogger?: Logger; + constructor(options?: UndiciHttpHandlerOptions) { if (options?.dispatcher && isDispatcher(options.dispatcher)) { this.config = { ...options, dispatcher: options.dispatcher }; @@ -209,10 +220,20 @@ export class UndiciHttpHandler implements HttpHandler } } + public updateHttpClientConfig(key: typeof FALLBACK_LOGGER, value: Logger): void; public updateHttpClientConfig( key: K, value: UndiciHttpHandlerOptions[K] + ): void; + public updateHttpClientConfig( + key: keyof UndiciHttpHandlerOptions | typeof FALLBACK_LOGGER, + value: UndiciHttpHandlerOptions[keyof UndiciHttpHandlerOptions] | Logger ): void { + if (key === FALLBACK_LOGGER) { + this.fallbackLogger = value as Logger; + return; + } + if (key !== "dispatcher") { (this.config as any)[key] = value; return; From 203c84ee3d675577657c91971f5e7b6e2f97f157 Mon Sep 17 00:00:00 2001 From: John Lwin Date: Tue, 4 Aug 2026 23:13:52 -0700 Subject: [PATCH 06/11] test: cover fallback logger in handler specs --- .../httpExtensionConfiguration.spec.ts | 23 +++++- .../src/fetch-http-handler.spec.ts | 23 +++++- .../src/node-http-handler.spec.ts | 77 ++++++++++++++++--- .../src/undici-http-handler.spec.ts | 19 ++++- 4 files changed, 127 insertions(+), 15 deletions(-) diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts index 14096db5a93..7bb8819f194 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from "vitest"; +import { FALLBACK_LOGGER } from "../fallbackLogger"; import { getHttpHandlerExtensionConfiguration } from "./httpExtensionConfiguration"; describe("getHttpHandlerExtensionConfiguration", () => { @@ -19,13 +20,23 @@ describe("getHttpHandlerExtensionConfiguration", () => { }); describe("client logger injection", () => { - it("passes logger to httpHandler via updateHttpClientConfig", () => { + it("offers the client logger to the handler under the fallback key", () => { const handler = createMockHandler(); const logger = createMockLogger(); getHttpHandlerExtensionConfiguration({ httpHandler: handler, logger } as any); - expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("logger", logger); + expect(handler.updateHttpClientConfig).toHaveBeenCalledWith(FALLBACK_LOGGER, logger); + }); + + it("does not assign the public logger key, so an explicit handler logger is never overwritten", () => { + const handler = createMockHandler(); + const logger = createMockLogger(); + + getHttpHandlerExtensionConfiguration({ httpHandler: handler, logger } as any); + + expect(handler.updateHttpClientConfig).not.toHaveBeenCalledWith("logger", expect.anything()); + expect(handler.updateHttpClientConfig).toHaveBeenCalledTimes(1); }); it("does not call updateHttpClientConfig when logger is not set", () => { @@ -42,5 +53,13 @@ describe("getHttpHandlerExtensionConfiguration", () => { // should not throw getHttpHandlerExtensionConfiguration({ logger } as any); }); + + it("does not throw for a handler that predates updateHttpClientConfig", () => { + const logger = createMockLogger(); + const legacyHandler = { metadata: {}, handle: vi.fn() }; + + // should not throw + getHttpHandlerExtensionConfiguration({ httpHandler: legacyHandler, logger } as any); + }); }); }); diff --git a/packages/fetch-http-handler/src/fetch-http-handler.spec.ts b/packages/fetch-http-handler/src/fetch-http-handler.spec.ts index 07353ea1097..7aaf443590e 100644 --- a/packages/fetch-http-handler/src/fetch-http-handler.spec.ts +++ b/packages/fetch-http-handler/src/fetch-http-handler.spec.ts @@ -1,5 +1,5 @@ import { AbortController } from "@smithy/abort-controller"; -import { HttpRequest } from "@smithy/core/protocols"; +import { FALLBACK_LOGGER, HttpRequest } from "@smithy/core/protocols"; import { afterAll, afterEach, beforeEach, describe, expect, test as it, vi } from "vitest"; import { FetchHttpHandler, keepAliveSupport } from "./fetch-http-handler"; @@ -98,6 +98,27 @@ const globalFetch = global.fetch; expect(fetchHttpHandler.httpHandlerConfigs()).toEqual({}); }); + it("accepts the fallback logger without adding it to the public config", async () => { + const mockResponse = { + headers: { + entries: vi.fn().mockReturnValue([]), + }, + blob: vi.fn().mockResolvedValue(new Blob(["FOO"])), + }; + (global as any).fetch = vi.fn().mockResolvedValue(mockResponse); + + const fetchHttpHandler = new FetchHttpHandler({ requestTimeout: 200 }); + const clientLogger = { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + fetchHttpHandler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); + + await fetchHttpHandler.handle({} as any, {}); + + const configs = fetchHttpHandler.httpHandlerConfigs(); + expect(configs).not.toHaveProperty("logger"); + // unrelated config is untouched by the fallback logger call. + expect(configs.requestTimeout).toBe(200); + }); + it("defaults to response.blob for response.body = null", async () => { const mockResponse = { body: null, diff --git a/packages/node-http-handler/src/node-http-handler.spec.ts b/packages/node-http-handler/src/node-http-handler.spec.ts index a670114252c..67f579108b9 100644 --- a/packages/node-http-handler/src/node-http-handler.spec.ts +++ b/packages/node-http-handler/src/node-http-handler.spec.ts @@ -1,6 +1,7 @@ import http from "node:http"; import https from "node:https"; -import { HttpRequest } from "@smithy/core/protocols"; +import { FALLBACK_LOGGER, HttpRequest } from "@smithy/core/protocols"; +import type { NodeHttpHandlerOptions } from "@smithy/types"; import { afterEach, beforeEach, describe, expect, test as it, vi } from "vitest"; import { NodeHttpHandler } from "./node-http-handler"; @@ -486,22 +487,23 @@ describe("NodeHttpHandler", () => { }); describe("updateHttpClientConfig", () => { - it("updates the logger", async () => { - const handler = new NodeHttpHandler(); - - const clientLogger = { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; - handler.updateHttpClientConfig("logger", clientLogger); - + const createLogger = () => ({ trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }); + + /** + * @returns the logger the handler resolved for the request, observed via + * setRequestTimeout, which receives it as its last argument. + */ + const getEffectiveLogger = async (handler: NodeHttpHandler) => { + const spy = vi.spyOn(setRequestTimeoutModule, "setRequestTimeout"); + spy.mockClear(); const request = new HttpRequest({ hostname: "localhost", method: "GET", protocol: "https:", path: "/" }); try { await handler.handle(request); } catch { // ignore request errors } - - const configs = handler.httpHandlerConfigs(); - expect(configs.logger).toBe(clientLogger); - }); + return spy.mock.calls[0][4]; + }; it("updates non-logger keys", async () => { const handler = new NodeHttpHandler({ requestTimeout: 1000 }); @@ -517,6 +519,59 @@ describe("NodeHttpHandler", () => { const configs = handler.httpHandlerConfigs(); expect(configs.requestTimeout).toBe(5000); }); + + it("uses the fallback logger when the handler has no logger of its own", async () => { + const handler = new NodeHttpHandler(); + const clientLogger = createLogger(); + + handler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); + + expect(await getEffectiveLogger(handler)).toBe(clientLogger); + }); + + it("keeps the handler's explicit logger instead of the fallback logger", async () => { + const handlerLogger = createLogger(); + const handler = new NodeHttpHandler({ logger: handlerLogger }); + const clientLogger = createLogger(); + + handler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); + + expect(await getEffectiveLogger(handler)).toBe(handlerLogger); + }); + + it("accepts the fallback logger synchronously while config resolves asynchronously", async () => { + let resolveOptions: (o: NodeHttpHandlerOptions) => void; + const handler = new NodeHttpHandler( + () => new Promise((resolve) => (resolveOptions = resolve)) + ); + const clientLogger = createLogger(); + + // Offered before the handler's own config has resolved. + handler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); + resolveOptions!({}); + + expect(await getEffectiveLogger(handler)).toBe(clientLogger); + }); + + it("keeps an explicit logger that resolves asynchronously, over the fallback logger", async () => { + const handlerLogger = createLogger(); + const handler = new NodeHttpHandler(async () => ({ logger: handlerLogger })); + const clientLogger = createLogger(); + + handler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); + + expect(await getEffectiveLogger(handler)).toBe(handlerLogger); + }); + + it("does not expose the fallback logger on the handler's public config", async () => { + const handler = new NodeHttpHandler(); + const clientLogger = createLogger(); + + handler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); + await getEffectiveLogger(handler); + + expect(handler.httpHandlerConfigs().logger).toBeUndefined(); + }); }); describe("checkSocketUsage", () => { diff --git a/packages/undici-http-handler/src/undici-http-handler.spec.ts b/packages/undici-http-handler/src/undici-http-handler.spec.ts index f2c5f612816..29f2994e680 100644 --- a/packages/undici-http-handler/src/undici-http-handler.spec.ts +++ b/packages/undici-http-handler/src/undici-http-handler.spec.ts @@ -1,6 +1,6 @@ import http, { type IncomingMessage, type Server, type ServerResponse } from "node:http"; import type { AddressInfo } from "node:net"; -import { HttpRequest } from "@smithy/core/protocols"; +import { FALLBACK_LOGGER, HttpRequest } from "@smithy/core/protocols"; import { Agent, getGlobalDispatcher, setGlobalDispatcher, type Dispatcher } from "undici"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; @@ -571,6 +571,23 @@ describe("UndiciHttpHandler", () => { expect(handler.httpHandlerConfigs().logger).toBe(updatedLogger); }); + it("does not overwrite an explicit logger with the fallback logger", async () => { + const logger = createMockLogger(); + const clientLogger = createMockLogger(); + handler = new UndiciHttpHandler({ logger }); + handler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); + await handler.handle(createMockRequest()); + expect(handler.httpHandlerConfigs().logger).toBe(logger); + }); + + it("does not expose the fallback logger on the public config", async () => { + handler = new UndiciHttpHandler(); + const clientLogger = createMockLogger(); + handler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); + await handler.handle(createMockRequest()); + expect(handler.httpHandlerConfigs().logger).toBeUndefined(); + }); + it("retains existing dispatcher if undefined is passed", () => { handler = new UndiciHttpHandler(); const configBefore = handler.httpHandlerConfigs(); From 25212adc5c308121ab8e02726d26627c0b41f42e Mon Sep 17 00:00:00 2001 From: John Lwin Date: Wed, 5 Aug 2026 12:31:47 -0700 Subject: [PATCH 07/11] chore: record fallbackLogger in API snapshot --- api-snapshot/api.json | 1 + 1 file changed, 1 insertion(+) diff --git a/api-snapshot/api.json b/api-snapshot/api.json index 6a17d57ddea..19744074783 100644 --- a/api-snapshot/api.json +++ b/api-snapshot/api.json @@ -346,6 +346,7 @@ "escapeUri": "function", "escapeUriPath": "function", "extendedEncodeURIComponent": "function", + "FALLBACK_LOGGER": "symbol", "Field": "function", "FieldOptions": "type(object)", "FieldPosition": "type(union)", From d84a21985c6fe27443c52a19493725ff722a1482 Mon Sep 17 00:00:00 2001 From: John Lwin Date: Sat, 8 Aug 2026 19:04:40 -0700 Subject: [PATCH 08/11] fix: read requestHandler in http handler extension config --- .../pass-client-logger-to-request-handler.md | 5 +- api-snapshot/api.json | 38 +++++++++- .../core/src/submodules/protocols/index.ts | 1 - .../httpExtensionConfiguration.spec.ts | 75 +++++++++++++++++-- .../extensions/httpExtensionConfiguration.ts | 21 ++++-- .../protocols/protocol-http/fallbackLogger.ts | 8 +- .../protocols/protocol-http/httpHandler.ts | 12 +-- .../src/fetch-http-handler.spec.ts | 23 +----- .../src/fetch-http-handler.ts | 29 ++----- .../src/node-http-handler.spec.ts | 10 ++- .../src/node-http-handler.ts | 40 +++++----- .../src/undici-http-handler.spec.ts | 12 ++- .../src/undici-http-handler.ts | 31 +++----- 13 files changed, 180 insertions(+), 125 deletions(-) diff --git a/.changeset/pass-client-logger-to-request-handler.md b/.changeset/pass-client-logger-to-request-handler.md index 7187ae5548b..64c906aae5e 100644 --- a/.changeset/pass-client-logger-to-request-handler.md +++ b/.changeset/pass-client-logger-to-request-handler.md @@ -1,8 +1,9 @@ --- "@smithy/core": minor -"@smithy/fetch-http-handler": minor "@smithy/node-http-handler": minor "@smithy/undici-http-handler": minor --- -feat: offer the client logger to request handlers as a fallback, without overwriting a handler's own logger +feat: offer the client logger to request handlers as a fallback, without overwriting a handler's own logger. A NoOpLogger is not offered, so handlers keep their own console-based defaults. + +fix: `getHttpHandlerExtensionConfiguration` and `resolveHttpHandlerRuntimeConfig` now read and write `requestHandler` instead of `httpHandler`, which is the field clients actually populate. This changes the shape of the internal `HttpHandlerExtensionConfigType` and of the object returned by `resolveHttpHandlerRuntimeConfig`. diff --git a/api-snapshot/api.json b/api-snapshot/api.json index 1754208ca9f..2fd5219f030 100644 --- a/api-snapshot/api.json +++ b/api-snapshot/api.json @@ -346,7 +346,6 @@ "escapeUri": "function", "escapeUriPath": "function", "extendedEncodeURIComponent": "function", - "FALLBACK_LOGGER": "symbol", "Field": "function", "FieldOptions": "type(object)", "FieldPosition": "type(union)", @@ -563,9 +562,11 @@ "ENV_CMDS_RELATIVE_URI": "string", "fromContainerMetadata": "function", "fromInstanceMetadata": "function", + "fromInstanceMetadataRegion": "function", "getInstanceMetadataEndpoint": "function", "httpRequest": "function", "InstanceMetadataCredentials": "type(interface)", + "InstanceMetadataRegionInit": "type(interface)", "providerConfigFromInit": "function", "RemoteProviderConfig": "type(interface)", "RemoteProviderInit": "type(interface)" @@ -776,6 +777,8 @@ }, "@smithy/node-config-provider": { "EnvOptions": "type(interface)", + "FromStaticConfig": "type(union)", + "GetterFromConfig": "type(object)", "GetterFromEnv": "type(object)", "loadConfig": "function", "LoadedConfigSelectors": "type(interface)", @@ -1002,20 +1005,28 @@ "_parseRfc3339DateTimeWithOffset": "function", "_parseRfc7231DateTime": "function", "AutomaticJsonStringConversion": "type(alias)", + "calculateBodyLength": "function", + "ChecksumStream": "function", + "ChecksumStreamInit": "type(interface)", "Client": "function", "collectBody": "function", "Command": "function", "CommandImpl": "type(interface)", + "concatBytes": "function", "ConditionalLazyValueInstruction": "type(object)", "ConditionalValueInstruction": "type(object)", "convertMap": "function", "copyDocumentWithTransform": "function", "createAggregatedClient": "function", + "createBufferedReadable": "function", + "createChecksumStream": "function", "dateToUtcString": "function", "decorateServiceException": "function", "DefaultExtensionRuntimeConfigType": "type(intersection)", "DefaultsMode": "type(union)", "DefaultsModeConfigs": "type(interface)", + "deserializerMiddleware": "function", + "deserializerMiddlewareOption": "object", "DocumentType": "type(union)", "emitWarningIfUnsupportedVersion": "function", "ExceptionOptionType": "type(object)", @@ -1034,12 +1045,24 @@ "extendedEncodeURIComponent": "function", "FilterStatus": "type(alias)", "FilterStatusSupplier": "type(object)", + "fromArrayBuffer": "function", + "fromBase64": "function", + "fromHex": "function", + "fromString": "function", + "fromUtf8": "function", "generateIdempotencyToken": "function", "getArrayIfSingleItem": "function", + "getAwsChunkedEncodingStream": "function", "getDefaultClientConfiguration": "function", "getDefaultExtensionConfiguration": "function", + "getSerdePlugin": "function", "getValueFromTextNode": "function", "handleFloat": "function", + "Hash": "function", + "headStream": "function", + "isArrayBuffer": "function", + "isBlob": "function", + "isReadableStream": "function", "isSerializableHeaderValue": "function", "LazyJsonString": "function", "LazyValueInstruction": "type(object)", @@ -1065,9 +1088,12 @@ "resolveDefaultRuntimeConfig": "function", "resolvedPath": "function", "SdkError": "type(intersection)", + "sdkStreamMixin": "function", "SENSITIVE_STRING": "string", "serializeDateTime": "function", "serializeFloat": "function", + "serializerMiddleware": "function", + "serializerMiddlewareOption": "object", "ServiceException": "function", "ServiceExceptionOptions": "type(interface)", "SimpleValueInstruction": "type(object)", @@ -1078,6 +1104,8 @@ "SourceMappingInstructions": "type(object)", "splitEvery": "function", "splitHeader": "function", + "splitStream": "function", + "streamCollector": "function", "strictParseByte": "function", "strictParseDouble": "function", "strictParseFloat": "function", @@ -1086,9 +1114,17 @@ "strictParseInt32": "function", "strictParseLong": "function", "strictParseShort": "function", + "StringEncoding": "type(union)", "take": "function", "throwDefaultError": "function", + "toBase64": "function", + "toHex": "function", + "toUint8Array": "function", + "toUtf8": "function", + "Uint8ArrayBlobAdapter": "function", "UnfilteredValue": "type(alias)", + "V1OrV2Endpoint": "type(object)", + "v4": "function", "Value": "type(alias)", "ValueFilteringFunction": "type(object)", "ValueMapper": "type(object)", diff --git a/packages/core/src/submodules/protocols/index.ts b/packages/core/src/submodules/protocols/index.ts index f26a9073d9f..4e8d64c64f5 100644 --- a/packages/core/src/submodules/protocols/index.ts +++ b/packages/core/src/submodules/protocols/index.ts @@ -16,7 +16,6 @@ export { SerdeContext } from "./SerdeContext"; export { Field } from "./protocol-http/Field"; export { Fields, type FieldsOptions } from "./protocol-http/Fields"; export { type HttpHandler, type HttpHandlerUserInput } from "./protocol-http/httpHandler"; -export { FALLBACK_LOGGER } from "./protocol-http/fallbackLogger"; export { HttpRequest, type IHttpRequest } from "@smithy/core/transport"; export { HttpResponse } from "@smithy/core/transport"; export { isValidHostname } from "@smithy/core/transport"; diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts index 7bb8819f194..d69fb9a847a 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import { FALLBACK_LOGGER } from "../fallbackLogger"; -import { getHttpHandlerExtensionConfiguration } from "./httpExtensionConfiguration"; +import { getHttpHandlerExtensionConfiguration, resolveHttpHandlerRuntimeConfig } from "./httpExtensionConfiguration"; describe("getHttpHandlerExtensionConfiguration", () => { const createMockHandler = () => ({ @@ -19,12 +19,24 @@ describe("getHttpHandlerExtensionConfiguration", () => { error: vi.fn(), }); + /** + * Stands in for the client's NoOpLogger, which is detected by constructor + * name to avoid a cross-submodule import. + */ + class NoOpLogger { + public trace = vi.fn(); + public debug = vi.fn(); + public info = vi.fn(); + public warn = vi.fn(); + public error = vi.fn(); + } + describe("client logger injection", () => { it("offers the client logger to the handler under the fallback key", () => { const handler = createMockHandler(); const logger = createMockLogger(); - getHttpHandlerExtensionConfiguration({ httpHandler: handler, logger } as any); + getHttpHandlerExtensionConfiguration({ requestHandler: handler, logger } as any); expect(handler.updateHttpClientConfig).toHaveBeenCalledWith(FALLBACK_LOGGER, logger); }); @@ -33,7 +45,7 @@ describe("getHttpHandlerExtensionConfiguration", () => { const handler = createMockHandler(); const logger = createMockLogger(); - getHttpHandlerExtensionConfiguration({ httpHandler: handler, logger } as any); + getHttpHandlerExtensionConfiguration({ requestHandler: handler, logger } as any); expect(handler.updateHttpClientConfig).not.toHaveBeenCalledWith("logger", expect.anything()); expect(handler.updateHttpClientConfig).toHaveBeenCalledTimes(1); @@ -42,7 +54,15 @@ describe("getHttpHandlerExtensionConfiguration", () => { it("does not call updateHttpClientConfig when logger is not set", () => { const handler = createMockHandler(); - getHttpHandlerExtensionConfiguration({ httpHandler: handler } as any); + getHttpHandlerExtensionConfiguration({ requestHandler: handler } as any); + + expect(handler.updateHttpClientConfig).not.toHaveBeenCalled(); + }); + + it("does not offer a NoOpLogger, so handlers keep their console defaults", () => { + const handler = createMockHandler(); + + getHttpHandlerExtensionConfiguration({ requestHandler: handler, logger: new NoOpLogger() } as any); expect(handler.updateHttpClientConfig).not.toHaveBeenCalled(); }); @@ -59,7 +79,52 @@ describe("getHttpHandlerExtensionConfiguration", () => { const legacyHandler = { metadata: {}, handle: vi.fn() }; // should not throw - getHttpHandlerExtensionConfiguration({ httpHandler: legacyHandler, logger } as any); + getHttpHandlerExtensionConfiguration({ requestHandler: legacyHandler, logger } as any); + }); + }); + + describe("handler accessors read and write requestHandler", () => { + it("returns the handler that clients populate as requestHandler", () => { + const handler = createMockHandler(); + + const extension = getHttpHandlerExtensionConfiguration({ requestHandler: handler } as any); + + expect(extension.httpHandler()).toBe(handler); + }); + + it("setHttpHandler replaces the handler seen by the runtime config", () => { + const handler = createMockHandler(); + const replacement = createMockHandler(); + const runtimeConfig = { requestHandler: handler } as any; + + const extension = getHttpHandlerExtensionConfiguration(runtimeConfig); + extension.setHttpHandler(replacement as any); + + expect(runtimeConfig.requestHandler).toBe(replacement); + expect(extension.httpHandler()).toBe(replacement); + }); + + it("forwards updateHttpClientConfig and httpHandlerConfigs to the handler", () => { + const handler = createMockHandler(); + + const extension = getHttpHandlerExtensionConfiguration({ requestHandler: handler } as any); + extension.updateHttpClientConfig("requestTimeout" as any, 1000 as any); + extension.httpHandlerConfigs(); + + expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("requestTimeout", 1000); + expect(handler.httpHandlerConfigs).toHaveBeenCalled(); }); }); }); + +describe("resolveHttpHandlerRuntimeConfig", () => { + it("emits the handler under requestHandler, matching what clients read", () => { + const handler = { metadata: {}, handle: vi.fn() } as any; + + const runtimeConfig = resolveHttpHandlerRuntimeConfig({ + httpHandler: () => handler, + } as any); + + expect(runtimeConfig).toEqual({ requestHandler: handler }); + }); +}); diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts index bc2c15f170b..a122c9e15db 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts @@ -17,7 +17,7 @@ export interface HttpHandlerExtensionConfiguration = Partial<{ - httpHandler: HttpHandler; + requestHandler: HttpHandler; }>; /** @@ -29,22 +29,27 @@ export const getHttpHandlerExtensionConfiguration = & { logger?: Logger } ) => { // Offered as a fallback only: the handler keeps its own logger if it has one. - if (runtimeConfig.logger) { - runtimeConfig.httpHandler?.updateHttpClientConfig?.(FALLBACK_LOGGER, runtimeConfig.logger); + // A NoOpLogger is not offered at all, so that handlers fall through to their + // own console-based defaults instead of being silenced. + if (runtimeConfig.logger && runtimeConfig.logger.constructor?.name !== "NoOpLogger") { + runtimeConfig.requestHandler?.updateHttpClientConfig?.( + FALLBACK_LOGGER as unknown as keyof HandlerConfig, + runtimeConfig.logger as HandlerConfig[keyof HandlerConfig] + ); } return { setHttpHandler(handler: HttpHandler): void { - runtimeConfig.httpHandler = handler; + runtimeConfig.requestHandler = handler; }, httpHandler(): HttpHandler { - return runtimeConfig.httpHandler!; + return runtimeConfig.requestHandler!; }, updateHttpClientConfig(key: keyof HandlerConfig, value: HandlerConfig[typeof key]): void { - runtimeConfig.httpHandler?.updateHttpClientConfig(key, value); + runtimeConfig.requestHandler?.updateHttpClientConfig(key, value); }, httpHandlerConfigs(): HandlerConfig { - return runtimeConfig.httpHandler!.httpHandlerConfigs(); + return runtimeConfig.requestHandler!.httpHandlerConfigs(); }, }; }; @@ -58,6 +63,6 @@ export const resolveHttpHandlerRuntimeConfig = ): HttpHandlerExtensionConfigType => { return { - httpHandler: httpHandlerExtensionConfiguration.httpHandler(), + requestHandler: httpHandlerExtensionConfiguration.httpHandler(), }; }; diff --git a/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts b/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts index d400c919d1e..6bc7e295312 100644 --- a/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts +++ b/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts @@ -3,9 +3,11 @@ * `updateHttpClientConfig`, to be used only if the handler has no logger * of its own. * - * A symbol keeps this off the handlers' public options types. `Symbol.for` - * makes the key shared between duplicate copies of this package. + * A symbol keeps this off the handlers' public options types, and being a + * symbol already distinguishes it from the `"logger"` string key. + * `Symbol.for` means handlers can declare their own copy of this key and + * still compare equal to it. * * @internal */ -export const FALLBACK_LOGGER: unique symbol = Symbol.for("smithy.httpHandler.fallbackLogger"); +export const FALLBACK_LOGGER = Symbol.for("logger"); diff --git a/packages/core/src/submodules/protocols/protocol-http/httpHandler.ts b/packages/core/src/submodules/protocols/protocol-http/httpHandler.ts index 8b3130eca66..d2230d14ead 100644 --- a/packages/core/src/submodules/protocols/protocol-http/httpHandler.ts +++ b/packages/core/src/submodules/protocols/protocol-http/httpHandler.ts @@ -2,13 +2,10 @@ import type { HttpRequest, HttpResponse } from "@smithy/core/transport"; import type { FetchHttpHandlerOptions, HttpHandlerOptions, - Logger, NodeHttpHandlerOptions, RequestHandler, } from "@smithy/types"; -import type { FALLBACK_LOGGER } from "./fallbackLogger"; - /** * @internal */ @@ -19,15 +16,8 @@ export type HttpHandler = RequestHandler< > & { /** * @internal - * - * The key may also be {@link FALLBACK_LOGGER}, with which a client offers its - * logger for use only when the handler has no logger of its own. Handlers - * that predate that key ignore it. */ - updateHttpClientConfig( - key: keyof HttpHandlerConfig | typeof FALLBACK_LOGGER, - value: HttpHandlerConfig[keyof HttpHandlerConfig] | Logger - ): void; + updateHttpClientConfig(key: keyof HttpHandlerConfig, value: HttpHandlerConfig[typeof key]): void; /** * @internal diff --git a/packages/fetch-http-handler/src/fetch-http-handler.spec.ts b/packages/fetch-http-handler/src/fetch-http-handler.spec.ts index 7aaf443590e..07353ea1097 100644 --- a/packages/fetch-http-handler/src/fetch-http-handler.spec.ts +++ b/packages/fetch-http-handler/src/fetch-http-handler.spec.ts @@ -1,5 +1,5 @@ import { AbortController } from "@smithy/abort-controller"; -import { FALLBACK_LOGGER, HttpRequest } from "@smithy/core/protocols"; +import { HttpRequest } from "@smithy/core/protocols"; import { afterAll, afterEach, beforeEach, describe, expect, test as it, vi } from "vitest"; import { FetchHttpHandler, keepAliveSupport } from "./fetch-http-handler"; @@ -98,27 +98,6 @@ const globalFetch = global.fetch; expect(fetchHttpHandler.httpHandlerConfigs()).toEqual({}); }); - it("accepts the fallback logger without adding it to the public config", async () => { - const mockResponse = { - headers: { - entries: vi.fn().mockReturnValue([]), - }, - blob: vi.fn().mockResolvedValue(new Blob(["FOO"])), - }; - (global as any).fetch = vi.fn().mockResolvedValue(mockResponse); - - const fetchHttpHandler = new FetchHttpHandler({ requestTimeout: 200 }); - const clientLogger = { trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; - fetchHttpHandler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); - - await fetchHttpHandler.handle({} as any, {}); - - const configs = fetchHttpHandler.httpHandlerConfigs(); - expect(configs).not.toHaveProperty("logger"); - // unrelated config is untouched by the fallback logger call. - expect(configs.requestTimeout).toBe(200); - }); - it("defaults to response.blob for response.body = null", async () => { const mockResponse = { body: null, diff --git a/packages/fetch-http-handler/src/fetch-http-handler.ts b/packages/fetch-http-handler/src/fetch-http-handler.ts index cf8acbc416f..dccbdbe8478 100644 --- a/packages/fetch-http-handler/src/fetch-http-handler.ts +++ b/packages/fetch-http-handler/src/fetch-http-handler.ts @@ -1,11 +1,5 @@ -import { - FALLBACK_LOGGER, - HttpResponse, - buildQueryString, - type HttpHandler, - type HttpRequest, -} from "@smithy/core/protocols"; -import type { FetchHttpHandlerOptions, HeaderBag, HttpHandlerOptions, Logger, Provider } from "@smithy/types"; +import { HttpResponse, buildQueryString, type HttpHandler, type HttpRequest } from "@smithy/core/protocols"; +import type { FetchHttpHandlerOptions, HeaderBag, HttpHandlerOptions, Provider } from "@smithy/types"; import { createRequest } from "./create-request"; import { requestTimeout as requestTimeoutFn } from "./request-timeout"; @@ -15,7 +9,7 @@ declare let AbortController: any; /** * @public */ -export type { FetchHttpHandlerOptions }; +export { FetchHttpHandlerOptions }; /** * Detection of keepalive support. Can be overridden for testing. @@ -44,10 +38,6 @@ export type AdditionalRequestParameters = { export class FetchHttpHandler implements HttpHandler { private config?: FetchHttpHandlerOptions; private configProvider: Promise; - /** - * Client logger, used only when this handler has no logger of its own. - */ - private fallbackLogger?: Logger; /** * @returns the input if it is an HttpHandler of any class, @@ -210,19 +200,10 @@ export class FetchHttpHandler implements HttpHandler { return Promise.race(raceOfPromises).finally(removeSignalEventListener); } - updateHttpClientConfig(key: typeof FALLBACK_LOGGER, value: Logger): void; - updateHttpClientConfig(key: keyof FetchHttpHandlerOptions, value: FetchHttpHandlerOptions[typeof key]): void; - updateHttpClientConfig( - key: keyof FetchHttpHandlerOptions | typeof FALLBACK_LOGGER, - value: FetchHttpHandlerOptions[keyof FetchHttpHandlerOptions] | Logger - ): void { - if (key === FALLBACK_LOGGER) { - this.fallbackLogger = value as Logger; - return; - } + updateHttpClientConfig(key: keyof FetchHttpHandlerOptions, value: FetchHttpHandlerOptions[typeof key]): void { this.config = undefined; this.configProvider = this.configProvider.then((config) => { - (config as Record)[key as string] = value; + (config as Record)[key] = value; return config; }); } diff --git a/packages/node-http-handler/src/node-http-handler.spec.ts b/packages/node-http-handler/src/node-http-handler.spec.ts index 67f579108b9..882936b94b3 100644 --- a/packages/node-http-handler/src/node-http-handler.spec.ts +++ b/packages/node-http-handler/src/node-http-handler.spec.ts @@ -1,6 +1,6 @@ import http from "node:http"; import https from "node:https"; -import { FALLBACK_LOGGER, HttpRequest } from "@smithy/core/protocols"; +import { HttpRequest } from "@smithy/core/protocols"; import type { NodeHttpHandlerOptions } from "@smithy/types"; import { afterEach, beforeEach, describe, expect, test as it, vi } from "vitest"; @@ -8,6 +8,10 @@ import { NodeHttpHandler } from "./node-http-handler"; import * as setConnectionTimeoutModule from "./set-connection-timeout"; import * as setRequestTimeoutModule from "./set-request-timeout"; import * as setSocketTimeoutModule from "./set-socket-timeout"; + +// Matches the key the client offers its logger under. `Symbol.for` makes this +// the same symbol the handler compares against. +const FALLBACK_LOGGER = Symbol.for("logger") as unknown as keyof NodeHttpHandlerOptions; import { timing } from "./timing"; let { request: hRequest } = http; @@ -563,14 +567,14 @@ describe("NodeHttpHandler", () => { expect(await getEffectiveLogger(handler)).toBe(handlerLogger); }); - it("does not expose the fallback logger on the handler's public config", async () => { + it("stores the fallback logger under the handler's own logger key", async () => { const handler = new NodeHttpHandler(); const clientLogger = createLogger(); handler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); await getEffectiveLogger(handler); - expect(handler.httpHandlerConfigs().logger).toBeUndefined(); + expect(handler.httpHandlerConfigs().logger).toBe(clientLogger); }); }); diff --git a/packages/node-http-handler/src/node-http-handler.ts b/packages/node-http-handler/src/node-http-handler.ts index 7eb0174659b..30dedc5381e 100644 --- a/packages/node-http-handler/src/node-http-handler.ts +++ b/packages/node-http-handler/src/node-http-handler.ts @@ -1,12 +1,6 @@ import type { Agent as hAgentType, request as hRequestType } from "node:http"; import type { RequestOptions, Agent as hsAgentType } from "node:https"; -import { - FALLBACK_LOGGER, - HttpResponse, - buildQueryString, - type HttpHandler, - type HttpRequest, -} from "@smithy/core/protocols"; +import { HttpResponse, buildQueryString, type HttpHandler, type HttpRequest } from "@smithy/core/protocols"; import type { HttpHandlerOptions, Logger, NodeHttpHandlerOptions, Provider } from "@smithy/types"; import { buildAbortError } from "./build-abort-error"; @@ -22,6 +16,14 @@ import { writeRequestBody } from "./write-request-body"; export type { NodeHttpHandlerOptions }; +/** + * Key with which a client offers its logger, to be used only if this handler + * has no logger of its own. `Symbol.for` makes this equal to the client's copy. + * + * @internal + */ +const FALLBACK_LOGGER: symbol = Symbol.for("logger"); + interface ResolvedNodeHttpHandlerConfig extends Omit { httpAgentProvider: () => Promise; httpAgent?: hAgentType; @@ -48,10 +50,6 @@ export class NodeHttpHandler implements HttpHandler { private configProvider: Promise; private socketWarningTimestamp = 0; private externalAgent = false; - /** - * Client logger, used only when this handler has no logger of its own. - */ - private fallbackLogger?: Logger; // Node http handler is hard-coded to http/1.1: https://github.com/nodejs/node/blob/ff5664b83b89c55e4ab5d5f60068fb457f1f5872/lib/_http_server.js#L286 public readonly metadata = { handlerProtocol: "http/1.1" }; @@ -153,7 +151,7 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf } const config = this.config!; - const logger = config.logger ?? this.fallbackLogger; + const logger = config.logger; // determine which http(s) client to use const isSSL = request.protocol === "https:"; @@ -329,18 +327,16 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf }); } - public updateHttpClientConfig(key: typeof FALLBACK_LOGGER, value: Logger): void; - public updateHttpClientConfig(key: keyof NodeHttpHandlerOptions, value: NodeHttpHandlerOptions[typeof key]): void; - public updateHttpClientConfig( - key: keyof NodeHttpHandlerOptions | typeof FALLBACK_LOGGER, - value: NodeHttpHandlerOptions[keyof NodeHttpHandlerOptions] | Logger - ): void { - if (key === FALLBACK_LOGGER) { - this.fallbackLogger = value as Logger; - return; - } + public updateHttpClientConfig(key: keyof NodeHttpHandlerOptions, value: NodeHttpHandlerOptions[typeof key]): void { this.config = undefined; this.configProvider = this.configProvider.then((config) => { + if ((key as unknown) === FALLBACK_LOGGER) { + // Offered by the client: take it only if this handler has no logger of its own. + return { + ...config, + logger: config.logger ?? (value as Logger), + }; + } return { ...config, [key]: value, diff --git a/packages/undici-http-handler/src/undici-http-handler.spec.ts b/packages/undici-http-handler/src/undici-http-handler.spec.ts index 29f2994e680..18d0d7325f5 100644 --- a/packages/undici-http-handler/src/undici-http-handler.spec.ts +++ b/packages/undici-http-handler/src/undici-http-handler.spec.ts @@ -1,10 +1,14 @@ import http, { type IncomingMessage, type Server, type ServerResponse } from "node:http"; import type { AddressInfo } from "node:net"; -import { FALLBACK_LOGGER, HttpRequest } from "@smithy/core/protocols"; +import { HttpRequest } from "@smithy/core/protocols"; import { Agent, getGlobalDispatcher, setGlobalDispatcher, type Dispatcher } from "undici"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; -import { UndiciHttpHandler } from "./undici-http-handler"; +import { UndiciHttpHandler, type UndiciHttpHandlerOptions } from "./undici-http-handler"; + +// Matches the key the client offers its logger under. `Symbol.for` makes this +// the same symbol the handler compares against. +const FALLBACK_LOGGER = Symbol.for("logger") as unknown as keyof UndiciHttpHandlerOptions; const { createServer } = http; @@ -580,12 +584,12 @@ describe("UndiciHttpHandler", () => { expect(handler.httpHandlerConfigs().logger).toBe(logger); }); - it("does not expose the fallback logger on the public config", async () => { + it("stores the fallback logger under the handler's own logger key", async () => { handler = new UndiciHttpHandler(); const clientLogger = createMockLogger(); handler.updateHttpClientConfig(FALLBACK_LOGGER, clientLogger); await handler.handle(createMockRequest()); - expect(handler.httpHandlerConfigs().logger).toBeUndefined(); + expect(handler.httpHandlerConfigs().logger).toBe(clientLogger); }); it("retains existing dispatcher if undefined is passed", () => { diff --git a/packages/undici-http-handler/src/undici-http-handler.ts b/packages/undici-http-handler/src/undici-http-handler.ts index 34a0486af18..7a97c6c2681 100644 --- a/packages/undici-http-handler/src/undici-http-handler.ts +++ b/packages/undici-http-handler/src/undici-http-handler.ts @@ -1,16 +1,18 @@ import type { Readable } from "node:stream"; -import { - FALLBACK_LOGGER, - HttpResponse, - buildQueryString, - type HttpHandler, - type HttpRequest, -} from "@smithy/core/protocols"; +import { HttpResponse, buildQueryString, type HttpHandler, type HttpRequest } from "@smithy/core/protocols"; import type { HttpHandlerOptions, Logger } from "@smithy/types"; import { Agent, Dispatcher, getGlobalDispatcher } from "undici"; import { buildAbortError } from "./build-abort-error"; +/** + * Key with which a client offers its logger, to be used only if this handler + * has no logger of its own. `Symbol.for` makes this equal to the client's copy. + * + * @internal + */ +const FALLBACK_LOGGER: symbol = Symbol.for("logger"); + /** * Duck-type check: returns true if the value looks like a Dispatcher * (has `request`, `close`, and `destroy` methods), as opposed to plain @@ -73,11 +75,6 @@ export class UndiciHttpHandler implements HttpHandler */ private internalAgentOptions?: Agent.Options; - /** - * Client logger, used only when this handler has no logger of its own. - */ - private fallbackLogger?: Logger; - constructor(options?: UndiciHttpHandlerOptions) { if (options?.dispatcher && isDispatcher(options.dispatcher)) { this.config = { ...options, dispatcher: options.dispatcher }; @@ -220,17 +217,13 @@ export class UndiciHttpHandler implements HttpHandler } } - public updateHttpClientConfig(key: typeof FALLBACK_LOGGER, value: Logger): void; public updateHttpClientConfig( key: K, value: UndiciHttpHandlerOptions[K] - ): void; - public updateHttpClientConfig( - key: keyof UndiciHttpHandlerOptions | typeof FALLBACK_LOGGER, - value: UndiciHttpHandlerOptions[keyof UndiciHttpHandlerOptions] | Logger ): void { - if (key === FALLBACK_LOGGER) { - this.fallbackLogger = value as Logger; + if ((key as unknown) === FALLBACK_LOGGER) { + // Offered by the client: take it only if this handler has no logger of its own. + this.config.logger ??= value as Logger; return; } From 6c73415e828d8691522d24389774bbcd477e13e8 Mon Sep 17 00:00:00 2001 From: John Lwin Date: Sat, 8 Aug 2026 20:23:43 -0700 Subject: [PATCH 09/11] fix: drop stale api snapshot entries from bad merge --- api-snapshot/api.json | 37 ------------------- .../src/fetch-http-handler.ts | 2 +- 2 files changed, 1 insertion(+), 38 deletions(-) diff --git a/api-snapshot/api.json b/api-snapshot/api.json index 2fd5219f030..0eec068c178 100644 --- a/api-snapshot/api.json +++ b/api-snapshot/api.json @@ -562,11 +562,9 @@ "ENV_CMDS_RELATIVE_URI": "string", "fromContainerMetadata": "function", "fromInstanceMetadata": "function", - "fromInstanceMetadataRegion": "function", "getInstanceMetadataEndpoint": "function", "httpRequest": "function", "InstanceMetadataCredentials": "type(interface)", - "InstanceMetadataRegionInit": "type(interface)", "providerConfigFromInit": "function", "RemoteProviderConfig": "type(interface)", "RemoteProviderInit": "type(interface)" @@ -777,8 +775,6 @@ }, "@smithy/node-config-provider": { "EnvOptions": "type(interface)", - "FromStaticConfig": "type(union)", - "GetterFromConfig": "type(object)", "GetterFromEnv": "type(object)", "loadConfig": "function", "LoadedConfigSelectors": "type(interface)", @@ -1005,28 +1001,20 @@ "_parseRfc3339DateTimeWithOffset": "function", "_parseRfc7231DateTime": "function", "AutomaticJsonStringConversion": "type(alias)", - "calculateBodyLength": "function", - "ChecksumStream": "function", - "ChecksumStreamInit": "type(interface)", "Client": "function", "collectBody": "function", "Command": "function", "CommandImpl": "type(interface)", - "concatBytes": "function", "ConditionalLazyValueInstruction": "type(object)", "ConditionalValueInstruction": "type(object)", "convertMap": "function", "copyDocumentWithTransform": "function", "createAggregatedClient": "function", - "createBufferedReadable": "function", - "createChecksumStream": "function", "dateToUtcString": "function", "decorateServiceException": "function", "DefaultExtensionRuntimeConfigType": "type(intersection)", "DefaultsMode": "type(union)", "DefaultsModeConfigs": "type(interface)", - "deserializerMiddleware": "function", - "deserializerMiddlewareOption": "object", "DocumentType": "type(union)", "emitWarningIfUnsupportedVersion": "function", "ExceptionOptionType": "type(object)", @@ -1045,24 +1033,12 @@ "extendedEncodeURIComponent": "function", "FilterStatus": "type(alias)", "FilterStatusSupplier": "type(object)", - "fromArrayBuffer": "function", - "fromBase64": "function", - "fromHex": "function", - "fromString": "function", - "fromUtf8": "function", "generateIdempotencyToken": "function", "getArrayIfSingleItem": "function", - "getAwsChunkedEncodingStream": "function", "getDefaultClientConfiguration": "function", "getDefaultExtensionConfiguration": "function", - "getSerdePlugin": "function", "getValueFromTextNode": "function", "handleFloat": "function", - "Hash": "function", - "headStream": "function", - "isArrayBuffer": "function", - "isBlob": "function", - "isReadableStream": "function", "isSerializableHeaderValue": "function", "LazyJsonString": "function", "LazyValueInstruction": "type(object)", @@ -1088,12 +1064,9 @@ "resolveDefaultRuntimeConfig": "function", "resolvedPath": "function", "SdkError": "type(intersection)", - "sdkStreamMixin": "function", "SENSITIVE_STRING": "string", "serializeDateTime": "function", "serializeFloat": "function", - "serializerMiddleware": "function", - "serializerMiddlewareOption": "object", "ServiceException": "function", "ServiceExceptionOptions": "type(interface)", "SimpleValueInstruction": "type(object)", @@ -1104,8 +1077,6 @@ "SourceMappingInstructions": "type(object)", "splitEvery": "function", "splitHeader": "function", - "splitStream": "function", - "streamCollector": "function", "strictParseByte": "function", "strictParseDouble": "function", "strictParseFloat": "function", @@ -1114,17 +1085,9 @@ "strictParseInt32": "function", "strictParseLong": "function", "strictParseShort": "function", - "StringEncoding": "type(union)", "take": "function", "throwDefaultError": "function", - "toBase64": "function", - "toHex": "function", - "toUint8Array": "function", - "toUtf8": "function", - "Uint8ArrayBlobAdapter": "function", "UnfilteredValue": "type(alias)", - "V1OrV2Endpoint": "type(object)", - "v4": "function", "Value": "type(alias)", "ValueFilteringFunction": "type(object)", "ValueMapper": "type(object)", diff --git a/packages/fetch-http-handler/src/fetch-http-handler.ts b/packages/fetch-http-handler/src/fetch-http-handler.ts index dccbdbe8478..88c7546fa6c 100644 --- a/packages/fetch-http-handler/src/fetch-http-handler.ts +++ b/packages/fetch-http-handler/src/fetch-http-handler.ts @@ -9,7 +9,7 @@ declare let AbortController: any; /** * @public */ -export { FetchHttpHandlerOptions }; +export type { FetchHttpHandlerOptions }; /** * Detection of keepalive support. Can be overridden for testing. From 9f3f1eee9cb503fd0822b767b8b9950e0e4e9582 Mon Sep 17 00:00:00 2001 From: John Lwin Date: Tue, 11 Aug 2026 01:32:38 -0700 Subject: [PATCH 10/11] refactor: inline the fallback logger key in handlers --- packages/node-http-handler/src/node-http-handler.ts | 13 +++---------- .../undici-http-handler/src/undici-http-handler.ts | 13 +++---------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/packages/node-http-handler/src/node-http-handler.ts b/packages/node-http-handler/src/node-http-handler.ts index 30dedc5381e..bb0de9a5f57 100644 --- a/packages/node-http-handler/src/node-http-handler.ts +++ b/packages/node-http-handler/src/node-http-handler.ts @@ -16,14 +16,6 @@ import { writeRequestBody } from "./write-request-body"; export type { NodeHttpHandlerOptions }; -/** - * Key with which a client offers its logger, to be used only if this handler - * has no logger of its own. `Symbol.for` makes this equal to the client's copy. - * - * @internal - */ -const FALLBACK_LOGGER: symbol = Symbol.for("logger"); - interface ResolvedNodeHttpHandlerConfig extends Omit { httpAgentProvider: () => Promise; httpAgent?: hAgentType; @@ -330,8 +322,9 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf public updateHttpClientConfig(key: keyof NodeHttpHandlerOptions, value: NodeHttpHandlerOptions[typeof key]): void { this.config = undefined; this.configProvider = this.configProvider.then((config) => { - if ((key as unknown) === FALLBACK_LOGGER) { - // Offered by the client: take it only if this handler has no logger of its own. + if ((key as unknown) === Symbol.for("logger")) { + // A client offers its logger under this key: take it only if this + // handler has no logger of its own. return { ...config, logger: config.logger ?? (value as Logger), diff --git a/packages/undici-http-handler/src/undici-http-handler.ts b/packages/undici-http-handler/src/undici-http-handler.ts index 7a97c6c2681..b3453452cbf 100644 --- a/packages/undici-http-handler/src/undici-http-handler.ts +++ b/packages/undici-http-handler/src/undici-http-handler.ts @@ -5,14 +5,6 @@ import { Agent, Dispatcher, getGlobalDispatcher } from "undici"; import { buildAbortError } from "./build-abort-error"; -/** - * Key with which a client offers its logger, to be used only if this handler - * has no logger of its own. `Symbol.for` makes this equal to the client's copy. - * - * @internal - */ -const FALLBACK_LOGGER: symbol = Symbol.for("logger"); - /** * Duck-type check: returns true if the value looks like a Dispatcher * (has `request`, `close`, and `destroy` methods), as opposed to plain @@ -221,8 +213,9 @@ export class UndiciHttpHandler implements HttpHandler key: K, value: UndiciHttpHandlerOptions[K] ): void { - if ((key as unknown) === FALLBACK_LOGGER) { - // Offered by the client: take it only if this handler has no logger of its own. + if ((key as unknown) === Symbol.for("logger")) { + // A client offers its logger under this key: take it only if this + // handler has no logger of its own. this.config.logger ??= value as Logger; return; } From fc65973f2cb8ed1b193359084c1ec8b51b182b4e Mon Sep 17 00:00:00 2001 From: John Lwin Date: Tue, 11 Aug 2026 01:52:18 -0700 Subject: [PATCH 11/11] refactor(core): inline the fallback logger key and drop fallbackLogger.ts --- .../extensions/httpExtensionConfiguration.spec.ts | 7 ++++++- .../extensions/httpExtensionConfiguration.ts | 8 ++++++-- .../protocols/protocol-http/fallbackLogger.ts | 13 ------------- 3 files changed, 12 insertions(+), 16 deletions(-) delete mode 100644 packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts index d69fb9a847a..04db57020a3 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.spec.ts @@ -1,8 +1,13 @@ import { describe, expect, it, vi } from "vitest"; -import { FALLBACK_LOGGER } from "../fallbackLogger"; import { getHttpHandlerExtensionConfiguration, resolveHttpHandlerRuntimeConfig } from "./httpExtensionConfiguration"; +/** + * The key under which a client offers its logger. Declared locally, as the + * handlers do: `Symbol.for` makes every copy of this key equal. + */ +const FALLBACK_LOGGER = Symbol.for("logger"); + describe("getHttpHandlerExtensionConfiguration", () => { const createMockHandler = () => ({ metadata: { handlerProtocol: "http/1.1" }, diff --git a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts index a122c9e15db..46cdfcc1d35 100644 --- a/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts +++ b/packages/core/src/submodules/protocols/protocol-http/extensions/httpExtensionConfiguration.ts @@ -1,6 +1,5 @@ import type { Logger } from "@smithy/types"; -import { FALLBACK_LOGGER } from "../fallbackLogger"; import type { HttpHandler } from "../httpHandler"; /** @@ -28,12 +27,17 @@ export type HttpHandlerExtensionConfigType = export const getHttpHandlerExtensionConfiguration = ( runtimeConfig: HttpHandlerExtensionConfigType & { logger?: Logger } ) => { + // Offer the client's logger under `Symbol.for("logger")`. A symbol keeps this + // off the handlers' public options types, and being a symbol already + // distinguishes it from the `"logger"` string key. `Symbol.for` means each + // handler can declare its own copy of the key and still compare equal to it. + // // Offered as a fallback only: the handler keeps its own logger if it has one. // A NoOpLogger is not offered at all, so that handlers fall through to their // own console-based defaults instead of being silenced. if (runtimeConfig.logger && runtimeConfig.logger.constructor?.name !== "NoOpLogger") { runtimeConfig.requestHandler?.updateHttpClientConfig?.( - FALLBACK_LOGGER as unknown as keyof HandlerConfig, + Symbol.for("logger") as unknown as keyof HandlerConfig, runtimeConfig.logger as HandlerConfig[keyof HandlerConfig] ); } diff --git a/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts b/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts deleted file mode 100644 index 6bc7e295312..00000000000 --- a/packages/core/src/submodules/protocols/protocol-http/fallbackLogger.ts +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Key with which a client offers its logger to an HttpHandler via - * `updateHttpClientConfig`, to be used only if the handler has no logger - * of its own. - * - * A symbol keeps this off the handlers' public options types, and being a - * symbol already distinguishes it from the `"logger"` string key. - * `Symbol.for` means handlers can declare their own copy of this key and - * still compare equal to it. - * - * @internal - */ -export const FALLBACK_LOGGER = Symbol.for("logger");