Skip to content

Commit 5e73108

Browse files
authored
fix(node-http-handler): call socket.setTimeout instead of request.setTimeout if socket established (#1477)
1 parent 35aad30 commit 5e73108

File tree

3 files changed

+39
-3
lines changed

3 files changed

+39
-3
lines changed

.changeset/short-onions-shop.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@smithy/node-http-handler": patch
3+
---
4+
5+
fix delayed calling of setTimeout on requests

packages/node-http-handler/src/set-socket-timeout.spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,31 @@ describe("setSocketTimeout", () => {
4242
expect(clientRequest.setTimeout).toHaveBeenLastCalledWith(0, expect.any(Function));
4343
});
4444

45+
describe("event listener registration deferral", () => {
46+
const clientRequestWithSocket: any = {
47+
destroy: vi.fn(),
48+
setTimeout: vi.fn(),
49+
socket: {
50+
setTimeout: vi.fn(),
51+
},
52+
};
53+
54+
it("calls setTimeout on the socket if it is available after deferral", async () => {
55+
const eventListenerMinimumTimeoutToDefer = 6000;
56+
const deferralTimeout = 3000;
57+
const expectedDeferredSocketTimeout = eventListenerMinimumTimeoutToDefer - deferralTimeout;
58+
setSocketTimeout(clientRequestWithSocket, vi.fn(), eventListenerMinimumTimeoutToDefer);
59+
60+
vi.runAllTimers();
61+
62+
expect(clientRequestWithSocket.socket.setTimeout).toHaveBeenCalledTimes(1);
63+
expect(clientRequestWithSocket.socket.setTimeout).toHaveBeenLastCalledWith(
64+
expectedDeferredSocketTimeout,
65+
expect.any(Function)
66+
);
67+
});
68+
});
69+
4570
it(`destroys the request on timeout`, () => {
4671
setSocketTimeout(clientRequest, vi.fn(), 1);
4772
expect(clientRequest.destroy).not.toHaveBeenCalled();

packages/node-http-handler/src/set-socket-timeout.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,17 @@ export const setSocketTimeout = (
1010
timeoutInMs = 0
1111
): NodeJS.Timeout | number => {
1212
const registerTimeout = (offset: number) => {
13-
request.setTimeout(timeoutInMs - offset, () => {
14-
// destroy the request
13+
const timeout = timeoutInMs - offset;
14+
const onTimeout = () => {
1515
request.destroy();
1616
reject(Object.assign(new Error(`Connection timed out after ${timeoutInMs} ms`), { name: "TimeoutError" }));
17-
});
17+
};
18+
19+
if (request.socket) {
20+
request.socket.setTimeout(timeout, onTimeout);
21+
} else {
22+
request.setTimeout(timeout, onTimeout);
23+
}
1824
};
1925

2026
if (0 < timeoutInMs && timeoutInMs < 6000) {

0 commit comments

Comments
 (0)