Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(node-http-handler): configure disableConcurrentStreams in NodeHttp2Handler #2553

Merged
merged 20 commits into from
Jul 9, 2021
Merged
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d846629
chore: add disableSessionCache configuration
trivikr Jul 7, 2021
3e7f681
fix(node-http-handler): disable session cache if configured
trivikr Jul 7, 2021
0c95448
fix: prevent creating session if request already aborted
trivikr Jul 7, 2021
7f91d27
test: move h2 tests for error in new section
trivikr Jul 7, 2021
79a3e8b
chore: replace test name with NodeHttp2Handler.name
trivikr Jul 7, 2021
175b21c
chore: move existing tests under without options section
trivikr Jul 7, 2021
af10a75
test: when disableSessionCache is set
trivikr Jul 7, 2021
234a65d
test: update connectionPool tests to check connect calls
trivikr Jul 7, 2021
5eba686
test: move sessionTimeout tests to its own section
trivikr Jul 7, 2021
86906fa
test: sessionTimeout with different disableSessionCache values
trivikr Jul 7, 2021
c70a88b
test: add tests for requestTimeout
trivikr Jul 8, 2021
bf37020
test: use arrow functions in server.mock
trivikr Jul 8, 2021
494c8ea
test: fix request close on session when aborted
trivikr Jul 8, 2021
bf9f612
chore: clear connections array on destroy
trivikr Jul 8, 2021
632334a
chore: rename 'connection' to 'session'
trivikr Jul 8, 2021
9c4307d
chore: use sessionCache and sessionList
trivikr Jul 8, 2021
05673bb
chore: close session on response when disableSessionCache=true
trivikr Jul 8, 2021
b58c89e
chore: destroy session on sessionTimeout
trivikr Jul 8, 2021
241403b
chore: rename disableSessionCache to disableConcurrentStreams
trivikr Jul 8, 2021
2d1cade
chore: remove sessionList
trivikr Jul 9, 2021
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
Prev Previous commit
Next Next commit
test: fix request close on session when aborted
Fixes: #487
  • Loading branch information
trivikr committed Jul 8, 2021
commit 494c8ea1144fc2a4f5de14c9c9857b220198f051
35 changes: 9 additions & 26 deletions packages/node-http-handler/src/node-http2-handler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AbortController } from "@aws-sdk/abort-controller";
import { HttpRequest } from "@aws-sdk/protocol-http";
import { rejects } from "assert";
import http2, { constants, Http2Stream } from "http2";
@@ -251,10 +252,11 @@ describe(NodeHttp2Handler.name, () => {
});

it("will not create request on session if request already aborted", async () => {
trivikr marked this conversation as resolved.
Show resolved Hide resolved
// Create a session by sending a request.
await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});

// @ts-ignore: access private property
const session: ClientHttp2Session = nodeH2Handler.connectionPool.get(`${protocol}//${hostname}:${port}`);
const session: ClientHttp2Session = nodeH2Handler.connections[0];
const requestSpy = jest.spyOn(session, "request");

await expect(
@@ -268,39 +270,20 @@ describe(NodeHttp2Handler.name, () => {
expect(requestSpy.mock.calls.length).toBe(0);
});

/* Commenting out as the test is flaky https://github.com/aws/aws-sdk-js-v3/issues/487
it("will close request on session when aborted", async () => {
await nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {});

// @ts-ignore: access private property
const session: ClientHttp2Session = nodeH2Handler.connectionPool.get(
`${protocol}//${hostname}:${port}`
);
const requestSpy = jest.spyOn(session, "request");

const abortController = new AbortController();
// Delay response so that onabort is called earlier
setTimeout(() => {
mockH2Server.removeAllListeners("request");
mockH2Server.on("request", (request: any, response: any) => {
abortController.abort();
}, 0);
mockH2Server.on(
"request",
async () =>
new Promise(resolve => {
setTimeout(() => {
resolve(createResponseFunction(mockResponse));
}, 1000);
})
);

return createResponseFunction(mockResponse);
});

await expect(
nodeH2Handler.handle(new HttpRequest(getMockReqOptions()), {
abortSignal: abortController.signal
abortSignal: abortController.signal,
})
).rejects.toHaveProperty("name", "AbortError");
expect(requestSpy.mock.calls.length).toBe(1);
});
*/
});
});