Skip to content

Commit 5510e83

Browse files
authored
fix(node-http-handler): call socket methods if socket is defined (smithy-lang#1392)
1 parent b5c6241 commit 5510e83

File tree

5 files changed

+50
-7
lines changed

5 files changed

+50
-7
lines changed

.changeset/fresh-hairs-work.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+
call socket operations if socket is present in deferred listeners

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,22 @@ describe("setConnectionTimeout", () => {
7474
);
7575
});
7676

77+
it("calls socket operations directly if socket is available", async () => {
78+
const request = {
79+
on: jest.fn(),
80+
socket: {
81+
on: jest.fn(),
82+
connecting: true,
83+
},
84+
destroy() {},
85+
} as any;
86+
setConnectionTimeout(request, () => {}, 1);
87+
jest.runAllTimers();
88+
89+
expect(request.socket.on).toHaveBeenCalled();
90+
expect(request.on).not.toHaveBeenCalled();
91+
});
92+
7793
it("clears timeout if socket gets connected", () => {
7894
clientRequest.on.mock.calls[0][1](mockSocket);
7995

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { ClientRequest } from "http";
2-
import { Socket } from "net";
32

43
const DEFER_EVENT_LISTENER_TIME = 1000;
54

@@ -23,15 +22,21 @@ export const setConnectionTimeout = (
2322
);
2423
}, timeoutInMs - offset);
2524

26-
request.on("socket", (socket: Socket) => {
27-
if (socket.connecting) {
25+
const doWithSocket = (socket: typeof request.socket) => {
26+
if (socket?.connecting) {
2827
socket.on("connect", () => {
2928
clearTimeout(timeoutId);
3029
});
3130
} else {
3231
clearTimeout(timeoutId);
3332
}
34-
});
33+
};
34+
35+
if (request.socket) {
36+
doWithSocket(request.socket);
37+
} else {
38+
request.on("socket", doWithSocket);
39+
}
3540
};
3641

3742
if (timeoutInMs < 2000) {

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,17 @@ describe("setSocketKeepAlive", () => {
4242

4343
expect(setKeepAliveSpy).not.toHaveBeenCalled();
4444
});
45+
46+
it("calls socket operations directly if socket is available", async () => {
47+
const request = {
48+
on: jest.fn(),
49+
socket: {
50+
setKeepAlive: jest.fn(),
51+
},
52+
} as any;
53+
setSocketKeepAlive(request, { keepAlive: true, keepAliveMsecs: 1000 }, 0);
54+
55+
expect(request.socket.setKeepAlive).toHaveBeenCalled();
56+
expect(request.on).not.toHaveBeenCalled();
57+
});
4558
});

packages/node-http-handler/src/set-socket-keep-alive.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,13 @@ export const setSocketKeepAlive = (
1717
}
1818

1919
const registerListener = () => {
20-
request.on("socket", (socket) => {
21-
socket.setKeepAlive(keepAlive, keepAliveMsecs || 0);
22-
});
20+
if (request.socket) {
21+
request.socket.setKeepAlive(keepAlive, keepAliveMsecs || 0);
22+
} else {
23+
request.on("socket", (socket) => {
24+
socket.setKeepAlive(keepAlive, keepAliveMsecs || 0);
25+
});
26+
}
2327
};
2428

2529
if (deferTimeMs === 0) {

0 commit comments

Comments
 (0)