Skip to content

Commit

Permalink
fix(node-http-handler): call socket methods if socket is defined
Browse files Browse the repository at this point in the history
  • Loading branch information
kuhe committed Sep 6, 2024
1 parent c3d0955 commit 848e552
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/fresh-hairs-work.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@smithy/node-http-handler": patch
---

call socket operations if socket is present in deferred listeners
16 changes: 16 additions & 0 deletions packages/node-http-handler/src/set-connection-timeout.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,22 @@ describe("setConnectionTimeout", () => {
);
});

it("calls socket operations directly if socket is available", async () => {
const request = {
on: jest.fn(),
socket: {
on: jest.fn(),
connecting: true,
},
destroy() {},
} as any;
setConnectionTimeout(request, () => {}, 1);
jest.runAllTimers();

expect(request.socket.on).toHaveBeenCalled();
expect(request.on).not.toHaveBeenCalled();
});

it("clears timeout if socket gets connected", () => {
clientRequest.on.mock.calls[0][1](mockSocket);

Expand Down
11 changes: 8 additions & 3 deletions packages/node-http-handler/src/set-connection-timeout.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { ClientRequest } from "http";
import { Socket } from "net";

const DEFER_EVENT_LISTENER_TIME = 1000;

Expand All @@ -23,15 +22,21 @@ export const setConnectionTimeout = (
);
}, timeoutInMs - offset);

request.on("socket", (socket: Socket) => {
const doWithSocket = (socket: typeof request.socket) => {
if (socket.connecting) {
socket.on("connect", () => {
clearTimeout(timeoutId);
});
} else {
clearTimeout(timeoutId);
}
});
};

if (request.socket) {
doWithSocket(request.socket);
} else {
request.on("socket", doWithSocket);
}
};

if (timeoutInMs < 2000) {
Expand Down
13 changes: 13 additions & 0 deletions packages/node-http-handler/src/set-socket-keep-alive.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,17 @@ describe("setSocketKeepAlive", () => {

expect(setKeepAliveSpy).not.toHaveBeenCalled();
});

it("calls socket operations directly if socket is available", async () => {
const request = {
on: jest.fn(),
socket: {
setKeepAlive: jest.fn(),
},
} as any;
setSocketKeepAlive(request, { keepAlive: true, keepAliveMsecs: 1000 }, 0);

expect(request.socket.setKeepAlive).toHaveBeenCalled();
expect(request.on).not.toHaveBeenCalled();
});
});
10 changes: 7 additions & 3 deletions packages/node-http-handler/src/set-socket-keep-alive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ export const setSocketKeepAlive = (
}

const registerListener = () => {
request.on("socket", (socket) => {
socket.setKeepAlive(keepAlive, keepAliveMsecs || 0);
});
if (request.socket) {
request.socket.setKeepAlive(keepAlive, keepAliveMsecs || 0);
} else {
request.on("socket", (socket) => {
socket.setKeepAlive(keepAlive, keepAliveMsecs || 0);
});
}
};

if (deferTimeMs === 0) {
Expand Down

0 comments on commit 848e552

Please sign in to comment.