Skip to content

Commit 7f681f0

Browse files
AllanZhengYPAllanZhengYP
authored
chore(node-http-handler): h2 handler accept options in provider (#3579)
1 parent 94be85a commit 7f681f0

File tree

3 files changed

+81
-34
lines changed

3 files changed

+81
-34
lines changed

‎packages/node-http-handler/src/node-http2-handler.spec.ts‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@ describe(NodeHttp2Handler.name, () => {
4343
mockH2Server.close();
4444
});
4545

46-
describe("without options", () => {
46+
describe.each([
47+
["undefined", undefined],
48+
["empty object", {}],
49+
["undefined provider", async () => void 0],
50+
["empty object provider", async () => ({})],
51+
])("without options in constructor parameter of %s", (_, option) => {
4752
let createdSessions!: ClientHttp2Session[];
4853
const connectReal = http2.connect;
4954
let connectSpy!: jest.SpiedFunction<typeof http2.connect>;
@@ -58,7 +63,7 @@ describe(NodeHttp2Handler.name, () => {
5863
return session;
5964
});
6065

61-
nodeH2Handler = new NodeHttp2Handler();
66+
nodeH2Handler = new NodeHttp2Handler(option);
6267
});
6368

6469
const closeConnection = async (response: HttpResponse) => {
@@ -357,36 +362,48 @@ describe(NodeHttp2Handler.name, () => {
357362
const requestTimeout = 200;
358363

359364
describe("does not throw error when request not timed out", () => {
360-
it("disableConcurrentStreams: false (default)", async () => {
365+
it.each([
366+
["static object", { requestTimeout }],
367+
["object provider", async () => ({ requestTimeout })],
368+
])("disableConcurrentStreams: false (default) in constructor parameter of %s", async (_, options) => {
361369
mockH2Server.removeAllListeners("request");
362370
mockH2Server.on("request", createResponseFunctionWithDelay(mockResponse, requestTimeout - 100));
363371

364-
nodeH2Handler = new NodeHttp2Handler({ requestTimeout });
372+
nodeH2Handler = new NodeHttp2Handler(options);
365373
await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
366374
});
367375

368-
it("disableConcurrentStreams: true", async () => {
376+
it.each([
377+
["static object", { requestTimeout, disableConcurrentStreams: true }],
378+
["object provider", async () => ({ requestTimeout, disableConcurrentStreams: true })],
379+
])("disableConcurrentStreams: true in constructor parameter of %s", async (_, options) => {
369380
mockH2Server.removeAllListeners("request");
370381
mockH2Server.on("request", createResponseFunctionWithDelay(mockResponse, requestTimeout - 100));
371382

372-
nodeH2Handler = new NodeHttp2Handler({ requestTimeout, disableConcurrentStreams: true });
383+
nodeH2Handler = new NodeHttp2Handler(options);
373384
await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
374385
});
375386
});
376387

377388
describe("throws timeoutError on requestTimeout", () => {
378-
it("disableConcurrentStreams: false (default)", async () => {
389+
it.each([
390+
["static object", { requestTimeout }],
391+
["object provider", async () => ({ requestTimeout })],
392+
])("disableConcurrentStreams: false (default) in constructor parameter of %s", async (_, options) => {
379393
mockH2Server.removeAllListeners("request");
380394
mockH2Server.on("request", createResponseFunctionWithDelay(mockResponse, requestTimeout + 100));
381395

382-
nodeH2Handler = new NodeHttp2Handler({ requestTimeout });
396+
nodeH2Handler = new NodeHttp2Handler(options);
383397
await rejects(nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {}), {
384398
name: "TimeoutError",
385399
message: `Stream timed out because of no activity for ${requestTimeout} ms`,
386400
});
387401
});
388402

389-
it("disableConcurrentStreams: true", async () => {
403+
it.each([
404+
["object provider", async () => ({ requestTimeout })],
405+
["static object", { requestTimeout }],
406+
])("disableConcurrentStreams: true in constructor parameter of %s", async () => {
390407
mockH2Server.removeAllListeners("request");
391408
mockH2Server.on("request", createResponseFunctionWithDelay(mockResponse, requestTimeout + 100));
392409

@@ -403,8 +420,11 @@ describe(NodeHttp2Handler.name, () => {
403420
const sessionTimeout = 200;
404421

405422
describe("destroys sessions on sessionTimeout", () => {
406-
it("disableConcurrentStreams: false (default)", async () => {
407-
nodeH2Handler = new NodeHttp2Handler({ sessionTimeout });
423+
it.each([
424+
["object provider", async () => ({ sessionTimeout })],
425+
["static object", { sessionTimeout }],
426+
])("disableConcurrentStreams: false (default) in constructor parameter of %s", async (_, options) => {
427+
nodeH2Handler = new NodeHttp2Handler(options);
408428
await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
409429

410430
const authority = `${protocol}//${hostname}:${port}`;
@@ -419,11 +439,14 @@ describe(NodeHttp2Handler.name, () => {
419439
expect(nodeH2Handler.sessionCache.get(authority).length).toStrictEqual(0);
420440
});
421441

422-
it("disableConcurrentStreams: true", async () => {
442+
it.each([
443+
["object provider", async () => ({ sessionTimeout, disableConcurrentStreams: true })],
444+
["static object", { sessionTimeout, disableConcurrentStreams: true }],
445+
])("disableConcurrentStreams: true in constructor parameter of %s", async (_, options) => {
423446
let session;
424447
const authority = `${protocol}//${hostname}:${port}`;
425448

426-
nodeH2Handler = new NodeHttp2Handler({ sessionTimeout, disableConcurrentStreams: true });
449+
nodeH2Handler = new NodeHttp2Handler(options);
427450

428451
mockH2Server.removeAllListeners("request");
429452
mockH2Server.on("request", (request: any, response: any) => {
@@ -487,11 +510,12 @@ describe(NodeHttp2Handler.name, () => {
487510
);
488511
});
489512

490-
describe("disableConcurrentStreams", () => {
513+
describe.each([
514+
["object provider", async () => ({ disableConcurrentStreams: true })],
515+
["static object", { disableConcurrentStreams: true }],
516+
])("disableConcurrentStreams in constructor parameter of %s", (_, options) => {
491517
beforeEach(() => {
492-
nodeH2Handler = new NodeHttp2Handler({
493-
disableConcurrentStreams: true,
494-
});
518+
nodeH2Handler = new NodeHttp2Handler(options);
495519
});
496520

497521
describe("number calls to http2.connect", () => {

‎packages/node-http-handler/src/node-http2-handler.ts‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { HttpHandler, HttpRequest, HttpResponse } from "@aws-sdk/protocol-http";
22
import { buildQueryString } from "@aws-sdk/querystring-builder";
3-
import { HttpHandlerOptions } from "@aws-sdk/types";
3+
import { HttpHandlerOptions, Provider } from "@aws-sdk/types";
44
import { ClientHttp2Session, connect, constants } from "http2";
55

66
import { getTransformedHeaders } from "./get-transformed-headers";
@@ -33,17 +33,24 @@ export interface NodeHttp2HandlerOptions {
3333
}
3434

3535
export class NodeHttp2Handler implements HttpHandler {
36-
private readonly requestTimeout?: number;
37-
private readonly sessionTimeout?: number;
38-
private readonly disableConcurrentStreams?: boolean;
36+
private config?: NodeHttp2HandlerOptions;
37+
private readonly configProvider: Promise<NodeHttp2HandlerOptions>;
3938

4039
public readonly metadata = { handlerProtocol: "h2" };
4140
private sessionCache: Map<string, ClientHttp2Session[]>;
4241

43-
constructor({ requestTimeout, sessionTimeout, disableConcurrentStreams }: NodeHttp2HandlerOptions = {}) {
44-
this.requestTimeout = requestTimeout;
45-
this.sessionTimeout = sessionTimeout;
46-
this.disableConcurrentStreams = disableConcurrentStreams;
42+
constructor(options?: NodeHttp2HandlerOptions | Provider<NodeHttp2HandlerOptions | void>) {
43+
this.configProvider = new Promise((resolve, reject) => {
44+
if (typeof options === "function") {
45+
options()
46+
.then((opts) => {
47+
resolve(opts || {});
48+
})
49+
.catch(reject);
50+
} else {
51+
resolve(options || {});
52+
}
53+
});
4754
this.sessionCache = new Map<string, ClientHttp2Session[]>();
4855
}
4956

@@ -54,7 +61,11 @@ export class NodeHttp2Handler implements HttpHandler {
5461
this.sessionCache.clear();
5562
}
5663

57-
handle(request: HttpRequest, { abortSignal }: HttpHandlerOptions = {}): Promise<{ response: HttpResponse }> {
64+
async handle(request: HttpRequest, { abortSignal }: HttpHandlerOptions = {}): Promise<{ response: HttpResponse }> {
65+
if (!this.config) {
66+
this.config = await this.configProvider;
67+
}
68+
const { requestTimeout, disableConcurrentStreams } = this.config;
5869
return new Promise((resolve, rejectOriginal) => {
5970
// It's redundant to track fulfilled because promises use the first resolution/rejection
6071
// but avoids generating unnecessary stack traces in the "close" event handler.
@@ -71,10 +82,10 @@ export class NodeHttp2Handler implements HttpHandler {
7182

7283
const { hostname, method, port, protocol, path, query } = request;
7384
const authority = `${protocol}//${hostname}${port ? `:${port}` : ""}`;
74-
const session = this.getSession(authority, this.disableConcurrentStreams || false);
85+
const session = this.getSession(authority, disableConcurrentStreams || false);
7586

7687
const reject = (err: Error) => {
77-
if (this.disableConcurrentStreams) {
88+
if (disableConcurrentStreams) {
7889
this.destroySession(session);
7990
}
8091
fulfilled = true;
@@ -100,15 +111,14 @@ export class NodeHttp2Handler implements HttpHandler {
100111
});
101112
fulfilled = true;
102113
resolve({ response: httpResponse });
103-
if (this.disableConcurrentStreams) {
114+
if (disableConcurrentStreams) {
104115
// Gracefully closes the Http2Session, allowing any existing streams to complete
105116
// on their own and preventing new Http2Stream instances from being created.
106117
session.close();
107118
this.deleteSessionFromCache(authority, session);
108119
}
109120
});
110121

111-
const requestTimeout = this.requestTimeout;
112122
if (requestTimeout) {
113123
req.setTimeout(requestTimeout, () => {
114124
req.close();
@@ -141,7 +151,7 @@ export class NodeHttp2Handler implements HttpHandler {
141151
// an 'error' event will have also been emitted.
142152
req.on("close", () => {
143153
session.unref();
144-
if (this.disableConcurrentStreams) {
154+
if (disableConcurrentStreams) {
145155
session.destroy();
146156
}
147157
if (!fulfilled) {
@@ -162,6 +172,7 @@ export class NodeHttp2Handler implements HttpHandler {
162172
*/
163173
private getSession(authority: string, disableConcurrentStreams: boolean): ClientHttp2Session {
164174
const sessionCache = this.sessionCache;
175+
165176
const existingSessions = sessionCache.get(authority) || [];
166177

167178
// If concurrent streams are not disabled, we can use the existing session.
@@ -179,9 +190,8 @@ export class NodeHttp2Handler implements HttpHandler {
179190
newSession.on("error", destroySessionCb);
180191
newSession.on("frameError", destroySessionCb);
181192

182-
const sessionTimeout = this.sessionTimeout;
183-
if (sessionTimeout) {
184-
newSession.setTimeout(sessionTimeout, destroySessionCb);
193+
if (this.config?.sessionTimeout) {
194+
newSession.setTimeout(this.config.sessionTimeout, destroySessionCb);
185195
}
186196

187197
existingSessions.push(newSession);

‎packages/smithy-client/src/defaults-mode.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,20 @@ export type ResolvedDefaultsMode = Exclude<DefaultsMode, "auto">;
5050
* @internal
5151
*/
5252
export interface DefaultsModeConfigs {
53+
/**
54+
* The retry mode describing how the retry strategy control the traffic flow.
55+
*/
5356
retryMode?: string;
57+
/**
58+
* The maximum time in milliseconds that the connection phase of a request
59+
* may take before the connection attempt is abandoned.
60+
*/
5461
connectionTimeout?: number;
62+
/**
63+
* This timeout measures the time between when the first byte is sent over an
64+
* established, open connection and when the last byte is received from the
65+
* service. If the response is not received by the timeout, then the request
66+
* is considered timed out.
67+
*/
5568
requestTimeout?: number;
5669
}

0 commit comments

Comments
 (0)