From c16e0148eaa753f845e0998314383624653ba61b Mon Sep 17 00:00:00 2001 From: George Fu Date: Tue, 18 Jun 2024 15:21:29 -0400 Subject: [PATCH] fix(node-http-handler): add logger option and clear timeout on errors (#1311) --- .changeset/purple-frogs-guess.md | 6 ++++ .../src/node-http-handler.spec.ts | 7 ++-- .../src/node-http-handler.ts | 32 +++++++++++++------ .../src/http/httpHandlerInitialization.ts | 7 ++++ 4 files changed, 39 insertions(+), 13 deletions(-) create mode 100644 .changeset/purple-frogs-guess.md diff --git a/.changeset/purple-frogs-guess.md b/.changeset/purple-frogs-guess.md new file mode 100644 index 00000000000..c656684da5e --- /dev/null +++ b/.changeset/purple-frogs-guess.md @@ -0,0 +1,6 @@ +--- +"@smithy/node-http-handler": minor +"@smithy/types": patch +--- + +add logger option to node-http-handler parameters, clear socket usage check timeout on error 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 c9dc27ecf6b..6d0b7346294 100644 --- a/packages/node-http-handler/src/node-http-handler.spec.ts +++ b/packages/node-http-handler/src/node-http-handler.spec.ts @@ -759,10 +759,9 @@ describe("NodeHttpHandler", () => { expect(warningTimestamp).toBeGreaterThan(lastTimestamp); expect(console.warn).toHaveBeenCalledWith( - "@smithy/node-http-handler:WARN", - "socket usage at capacity=2 and 4 additional requests are enqueued.", - "See https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/node-configuring-maxsockets.html", - "or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler config." + `@smithy/node-http-handler:WARN - socket usage at capacity=2 and 4 additional requests are enqueued. +See https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/node-configuring-maxsockets.html +or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler config.` ); }); }); diff --git a/packages/node-http-handler/src/node-http-handler.ts b/packages/node-http-handler/src/node-http-handler.ts index 46b81d4f8fc..c3be097ffef 100644 --- a/packages/node-http-handler/src/node-http-handler.ts +++ b/packages/node-http-handler/src/node-http-handler.ts @@ -1,6 +1,6 @@ import { HttpHandler, HttpRequest, HttpResponse } from "@smithy/protocol-http"; import { buildQueryString } from "@smithy/querystring-builder"; -import type { NodeHttpHandlerOptions } from "@smithy/types"; +import type { Logger, NodeHttpHandlerOptions } from "@smithy/types"; import { HttpHandlerOptions, Provider } from "@smithy/types"; import { Agent as hAgent, request as hRequest } from "http"; import { Agent as hsAgent, request as hsRequest, RequestOptions } from "https"; @@ -48,9 +48,15 @@ export class NodeHttpHandler implements HttpHandler { * @internal * * @param agent - http(s) agent in use by the NodeHttpHandler instance. + * @param socketWarningTimestamp - last socket usage check timestamp. + * @param logger - channel for the warning. * @returns timestamp of last emitted warning. */ - public static checkSocketUsage(agent: hAgent | hsAgent, socketWarningTimestamp: number): number { + public static checkSocketUsage( + agent: hAgent | hsAgent, + socketWarningTimestamp: number, + logger: Logger = console + ): number { // note, maxSockets is per origin. const { sockets, requests, maxSockets } = agent; @@ -79,11 +85,10 @@ export class NodeHttpHandler implements HttpHandler { * lockout. */ if (socketsInUse >= maxSockets && requestsEnqueued >= 2 * maxSockets) { - console.warn( - "@smithy/node-http-handler:WARN", - `socket usage at capacity=${socketsInUse} and ${requestsEnqueued} additional requests are enqueued.`, - "See https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/node-configuring-maxsockets.html", - "or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler config." + logger?.warn?.( + `@smithy/node-http-handler:WARN - socket usage at capacity=${socketsInUse} and ${requestsEnqueued} additional requests are enqueued. +See https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/node-configuring-maxsockets.html +or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler config.` ); return Date.now(); } @@ -127,6 +132,7 @@ export class NodeHttpHandler implements HttpHandler { } return new hsAgent({ keepAlive, maxSockets, ...httpsAgent }); })(), + logger: console, }; } @@ -152,6 +158,7 @@ export class NodeHttpHandler implements HttpHandler { }; const reject = async (arg: unknown) => { await writeRequestBodyPromise; + clearTimeout(socketCheckTimeoutId); _reject(arg); }; @@ -175,7 +182,11 @@ export class NodeHttpHandler implements HttpHandler { // This warning will be cancelled if the request resolves. socketCheckTimeoutId = setTimeout( () => { - this.socketWarningTimestamp = NodeHttpHandler.checkSocketUsage(agent, this.socketWarningTimestamp); + this.socketWarningTimestamp = NodeHttpHandler.checkSocketUsage( + agent, + this.socketWarningTimestamp, + this.config!.logger + ); }, this.config.socketAcquisitionWarningTimeout ?? (this.config.requestTimeout ?? 2000) + (this.config.connectionTimeout ?? 1000) @@ -252,7 +263,10 @@ export class NodeHttpHandler implements HttpHandler { }); } - writeRequestBodyPromise = writeRequestBody(req, request, this.config.requestTimeout).catch(_reject); + writeRequestBodyPromise = writeRequestBody(req, request, this.config.requestTimeout).catch((e) => { + clearTimeout(socketCheckTimeoutId); + return _reject(e); + }); }); } diff --git a/packages/types/src/http/httpHandlerInitialization.ts b/packages/types/src/http/httpHandlerInitialization.ts index 66ee686c8e6..dae7c8d9e9a 100644 --- a/packages/types/src/http/httpHandlerInitialization.ts +++ b/packages/types/src/http/httpHandlerInitialization.ts @@ -1,6 +1,8 @@ import type { Agent as hAgent, AgentOptions as hAgentOptions } from "http"; import type { Agent as hsAgent, AgentOptions as hsAgentOptions } from "https"; +import { Logger } from "../logger"; + /** * * This type represents an alternate client constructor option for the entry @@ -60,6 +62,11 @@ export interface NodeHttpHandlerOptions { * You can pass https.Agent or its constructor options. */ httpsAgent?: hsAgent | hsAgentOptions; + + /** + * Optional logger. + */ + logger?: Logger; } /**