Skip to content

Commit 3f1190c

Browse files
AllanZhengYPAllanZhengYP
authored
test(node-http-handler): clean up ref()&unref() tests (#3554)
1 parent a5fa267 commit 3f1190c

File tree

1 file changed

+49
-70
lines changed

1 file changed

+49
-70
lines changed

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

+49-70
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { AbortController } from "@aws-sdk/abort-controller";
2-
import { HttpRequest } from "@aws-sdk/protocol-http";
2+
import { HttpRequest, HttpResponse } from "@aws-sdk/protocol-http";
33
import { rejects } from "assert";
44
import http2, { ClientHttp2Session, ClientHttp2Stream, constants, Http2Stream } from "http2";
55
import { Duplex } from "stream";
@@ -61,6 +61,25 @@ describe(NodeHttp2Handler.name, () => {
6161
nodeH2Handler = new NodeHttp2Handler();
6262
});
6363

64+
const closeConnection = async (response: HttpResponse) => {
65+
const responseBody = response.body as ClientHttp2Stream;
66+
const closePromise = new Promise((resolve) => responseBody.once("close", resolve));
67+
responseBody.destroy();
68+
await closePromise;
69+
};
70+
71+
// Keeping node alive while request is open.
72+
const expectSessionCreatedAndReferred = (session: ClientHttp2Session, requestCount = 1) => {
73+
expect(session.ref).toHaveBeenCalledTimes(requestCount);
74+
expect(session.unref).toHaveBeenCalledTimes(1);
75+
};
76+
77+
// No longer keeping node alive
78+
const expectSessionCreatedAndUnreffed = (session: ClientHttp2Session, requestCount = 1) => {
79+
expect(session.ref).toHaveBeenCalledTimes(requestCount);
80+
expect(session.unref).toHaveBeenCalledTimes(requestCount + 1);
81+
};
82+
6483
afterEach(() => {
6584
nodeH2Handler.destroy();
6685
});
@@ -76,94 +95,62 @@ describe(NodeHttp2Handler.name, () => {
7695

7796
it("is one when request is made", async () => {
7897
// Make single request.
79-
const response = await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
80-
const responseBody = response.response.body as ClientHttp2Stream;
98+
const { response } = await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
8199

82100
const authority = `${protocol}//${hostname}:${port}`;
83101
expect(connectSpy).toHaveBeenCalledTimes(1);
84102
expect(connectSpy).toHaveBeenCalledWith(authority);
85103

86-
// Keeping node alive while request is open.
87-
expect(createdSessions[0].ref).toHaveBeenCalledTimes(1);
88-
expect(createdSessions[0].unref).toHaveBeenCalledTimes(1);
89-
90-
const closed = new Promise<void>((resolve) => responseBody.once("close", resolve));
91-
(response.response.body as ClientHttp2Stream).destroy();
92-
await closed;
93-
94-
// No longer keeping node alive
95-
expect(createdSessions[0].ref).toHaveBeenCalledTimes(1);
96-
expect(createdSessions[0].unref).toHaveBeenCalledTimes(2);
104+
expectSessionCreatedAndReferred(createdSessions[0]);
105+
await closeConnection(response);
106+
expectSessionCreatedAndUnreffed(createdSessions[0]);
97107
});
98108

99109
it("is one if multiple requests are made on same URL", async () => {
100110
const connectSpy = jest.spyOn(http2, "connect");
101111

102112
// Make two requests.
103-
const response1 = await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
104-
const response1Body = response1.response.body as ClientHttp2Stream;
105-
const response2 = await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
106-
const response2Body = response2.response.body as ClientHttp2Stream;
113+
const { response: response1 } = await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
114+
const { response: response2 } = await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
107115

108116
const authority = `${protocol}//${hostname}:${port}`;
109117
expect(connectSpy).toHaveBeenCalledTimes(1);
110118
expect(connectSpy).toHaveBeenCalledWith(authority);
111119

112-
// Keeping node alive while requests are open.
113-
expect(createdSessions[0].ref).toHaveBeenCalledTimes(2);
114-
expect(createdSessions[0].unref).toHaveBeenCalledTimes(1);
115-
116-
const closed1 = new Promise<void>((resolve) => response1Body.once("close", resolve));
117-
response1Body.destroy();
118-
await closed1;
119-
const closed2 = new Promise<void>((resolve) => response2Body.once("close", resolve));
120-
response2Body.destroy();
121-
await closed2;
122-
123-
// No longer keeping node alive
124-
expect(createdSessions[0].ref).toHaveBeenCalledTimes(2);
125-
expect(createdSessions[0].unref).toHaveBeenCalledTimes(3);
120+
expectSessionCreatedAndReferred(createdSessions[0], 2);
121+
await closeConnection(response1);
122+
await closeConnection(response2);
123+
expectSessionCreatedAndUnreffed(createdSessions[0], 2);
126124
});
127125

128126
it("is many if requests are made on different URLs", async () => {
129127
const connectSpy = jest.spyOn(http2, "connect");
130128

131129
// Make first request on default URL.
132-
const response1 = await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
133-
const response1Body = response1.response.body as ClientHttp2Stream;
130+
const { response: response1 } = await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});
134131

135132
const port2 = port + 1;
136133
const mockH2Server2 = createMockHttp2Server().listen(port2);
137134
mockH2Server2.on("request", createResponseFunction(mockResponse));
138135

139136
// Make second request on URL with port2.
140-
const response2 = await nodeH2Handler.handle(new HttpRequest({ ...getMockReqOptions(), port: port2 }), {});
141-
const response2Body = response2.response.body as ClientHttp2Stream;
137+
const { response: response2 } = await nodeH2Handler.handle(
138+
new HttpRequest({ ...getMockReqOptions(), port: port2 }),
139+
{}
140+
);
142141

143142
const authorityPrefix = `${protocol}//${hostname}`;
144143
expect(connectSpy).toHaveBeenCalledTimes(2);
145144
expect(connectSpy).toHaveBeenNthCalledWith(1, `${authorityPrefix}:${port}`);
146145
expect(connectSpy).toHaveBeenNthCalledWith(2, `${authorityPrefix}:${port2}`);
147146
mockH2Server2.close();
148147

149-
// Keeping node alive while requests are open.
150-
expect(createdSessions[0].ref).toHaveBeenCalledTimes(1);
151-
expect(createdSessions[0].unref).toHaveBeenCalledTimes(1);
152-
expect(createdSessions[1].ref).toHaveBeenCalledTimes(1);
153-
expect(createdSessions[1].unref).toHaveBeenCalledTimes(1);
154-
155-
const closed1 = new Promise<void>((resolve) => response1Body.once("close", resolve));
156-
response1Body.destroy();
157-
await closed1;
158-
const closed2 = new Promise<void>((resolve) => response2Body.once("close", resolve));
159-
response2Body.destroy();
160-
await closed2;
161-
162-
// No longer keeping node alive
163-
expect(createdSessions[0].ref).toHaveBeenCalledTimes(1);
164-
expect(createdSessions[0].unref).toHaveBeenCalledTimes(2);
165-
expect(createdSessions[1].ref).toHaveBeenCalledTimes(1);
166-
expect(createdSessions[1].unref).toHaveBeenCalledTimes(2);
148+
expectSessionCreatedAndReferred(createdSessions[0]);
149+
expectSessionCreatedAndReferred(createdSessions[1]);
150+
await closeConnection(response1);
151+
await closeConnection(response2);
152+
expectSessionCreatedAndUnreffed(createdSessions[0]);
153+
expectSessionCreatedAndUnreffed(createdSessions[1]);
167154
});
168155
});
169156

@@ -214,12 +201,9 @@ describe(NodeHttp2Handler.name, () => {
214201

215202
// Not keeping node alive
216203
expect(createdSessions).toHaveLength(3);
217-
expect(createdSessions[0].ref).toHaveBeenCalledTimes(1);
218-
expect(createdSessions[0].unref).toHaveBeenCalledTimes(2);
219-
expect(createdSessions[1].ref).toHaveBeenCalledTimes(1);
220-
expect(createdSessions[1].unref).toHaveBeenCalledTimes(2);
221-
expect(createdSessions[2].ref).toHaveBeenCalledTimes(1);
222-
expect(createdSessions[2].unref).toHaveBeenCalledTimes(2);
204+
expectSessionCreatedAndUnreffed(createdSessions[0]);
205+
expectSessionCreatedAndUnreffed(createdSessions[1]);
206+
expectSessionCreatedAndUnreffed(createdSessions[2]);
223207

224208
// should be able to recover from goaway after reconnecting to a server
225209
// that doesn't send goaway, and reuse the TCP connection (Http2Session)
@@ -230,8 +214,7 @@ describe(NodeHttp2Handler.name, () => {
230214

231215
// Keeping node alive
232216
expect(createdSessions).toHaveLength(4);
233-
expect(createdSessions[3].ref).toHaveBeenCalledTimes(1);
234-
expect(createdSessions[3].unref).toHaveBeenCalledTimes(1);
217+
expectSessionCreatedAndReferred(createdSessions[3]);
235218

236219
// ...and validate that the mocked response is received
237220
const responseBody = await new Promise((resolve) => {
@@ -248,8 +231,7 @@ describe(NodeHttp2Handler.name, () => {
248231

249232
// Not keeping node alive
250233
expect(createdSessions).toHaveLength(4);
251-
expect(createdSessions[3].ref).toHaveBeenCalledTimes(1);
252-
expect(createdSessions[3].unref).toHaveBeenCalledTimes(2);
234+
expectSessionCreatedAndUnreffed(createdSessions[3]);
253235
});
254236

255237
it("handles connections destroyed by servers", async () => {
@@ -294,12 +276,9 @@ describe(NodeHttp2Handler.name, () => {
294276

295277
// Not keeping node alive
296278
expect(createdSessions).toHaveLength(3);
297-
expect(createdSessions[0].ref).toHaveBeenCalledTimes(1);
298-
expect(createdSessions[0].unref).toHaveBeenCalledTimes(2);
299-
expect(createdSessions[1].ref).toHaveBeenCalledTimes(1);
300-
expect(createdSessions[1].unref).toHaveBeenCalledTimes(2);
301-
expect(createdSessions[2].ref).toHaveBeenCalledTimes(1);
302-
expect(createdSessions[2].unref).toHaveBeenCalledTimes(2);
279+
expectSessionCreatedAndUnreffed(createdSessions[0]);
280+
expectSessionCreatedAndUnreffed(createdSessions[1]);
281+
expectSessionCreatedAndUnreffed(createdSessions[2]);
303282
});
304283
});
305284

0 commit comments

Comments
 (0)