Skip to content

Commit

Permalink
fix(node-http-handler): add logger option and clear timeout on errors (
Browse files Browse the repository at this point in the history
  • Loading branch information
kuhe authored Jun 18, 2024
1 parent 9ff2027 commit c16e014
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 13 deletions.
6 changes: 6 additions & 0 deletions .changeset/purple-frogs-guess.md
Original file line number Diff line number Diff line change
@@ -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
7 changes: 3 additions & 4 deletions packages/node-http-handler/src/node-http-handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.`
);
});
});
Expand Down
32 changes: 23 additions & 9 deletions packages/node-http-handler/src/node-http-handler.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -48,9 +48,15 @@ export class NodeHttpHandler implements HttpHandler<NodeHttpHandlerOptions> {
* @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;

Expand Down Expand Up @@ -79,11 +85,10 @@ export class NodeHttpHandler implements HttpHandler<NodeHttpHandlerOptions> {
* 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();
}
Expand Down Expand Up @@ -127,6 +132,7 @@ export class NodeHttpHandler implements HttpHandler<NodeHttpHandlerOptions> {
}
return new hsAgent({ keepAlive, maxSockets, ...httpsAgent });
})(),
logger: console,
};
}

Expand All @@ -152,6 +158,7 @@ export class NodeHttpHandler implements HttpHandler<NodeHttpHandlerOptions> {
};
const reject = async (arg: unknown) => {
await writeRequestBodyPromise;
clearTimeout(socketCheckTimeoutId);
_reject(arg);
};

Expand All @@ -175,7 +182,11 @@ export class NodeHttpHandler implements HttpHandler<NodeHttpHandlerOptions> {
// 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)
Expand Down Expand Up @@ -252,7 +263,10 @@ export class NodeHttpHandler implements HttpHandler<NodeHttpHandlerOptions> {
});
}

writeRequestBodyPromise = writeRequestBody(req, request, this.config.requestTimeout).catch(_reject);
writeRequestBodyPromise = writeRequestBody(req, request, this.config.requestTimeout).catch((e) => {
clearTimeout(socketCheckTimeoutId);
return _reject(e);
});
});
}

Expand Down
7 changes: 7 additions & 0 deletions packages/types/src/http/httpHandlerInitialization.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -60,6 +62,11 @@ export interface NodeHttpHandlerOptions {
* You can pass https.Agent or its constructor options.
*/
httpsAgent?: hsAgent | hsAgentOptions;

/**
* Optional logger.
*/
logger?: Logger;
}

/**
Expand Down

0 comments on commit c16e014

Please sign in to comment.