Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/pass-client-logger-to-request-handler.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@smithy/core": minor
"@smithy/node-http-handler": minor
"@smithy/undici-http-handler": minor
---

feat: pass client logger to request handlers
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
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("passes logger to httpHandler via updateHttpClientConfig", () => {
const handler = createMockHandler();
const logger = createMockLogger();

getHttpHandlerExtensionConfiguration({ httpHandler: handler, logger } as any);

expect(handler.updateHttpClientConfig).toHaveBeenCalledWith("logger", logger);
});

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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export type HttpHandlerExtensionConfigType<HandlerConfig extends object = {}> =
export const getHttpHandlerExtensionConfiguration = <HandlerConfig extends object = {}>(
runtimeConfig: HttpHandlerExtensionConfigType<HandlerConfig>
) => {
runtimeConfig.httpHandler?.updateHttpClientConfig("logger" as keyof HandlerConfig, (runtimeConfig as any).logger);

return {
setHttpHandler(handler: HttpHandler<HandlerConfig>): void {
runtimeConfig.httpHandler = handler;
Expand Down
53 changes: 53 additions & 0 deletions packages/node-http-handler/src/node-http-handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions packages/node-http-handler/src/node-http-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 as NodeHttpHandlerOptions["logger"]) };
}
return {
...config,
[key]: value,
Expand Down
13 changes: 10 additions & 3 deletions packages/undici-http-handler/src/undici-http-handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
6 changes: 5 additions & 1 deletion packages/undici-http-handler/src/undici-http-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,11 @@ export class UndiciHttpHandler implements HttpHandler<UndiciHttpHandlerOptions>
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;
}

Expand Down