Skip to content

Commit b361f95

Browse files
committed
test: refactor two http client timeout tests
Refactor test-http-client-set-timeout and test-http-client-timeout-option-with-agent. PR-URL: nodejs#25473 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 40a8a73 commit b361f95

File tree

2 files changed

+51
-84
lines changed

2 files changed

+51
-84
lines changed
+36-34
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,48 @@
11
'use strict';
2-
const common = require('../common');
32

4-
// Test that `req.setTimeout` will fired exactly once.
3+
// Test that the `'timeout'` event is emitted exactly once if the `timeout`
4+
// option and `request.setTimeout()` are used together.
55

6-
const assert = require('assert');
7-
const http = require('http');
6+
const { expectsError, mustCall } = require('../common');
7+
const { strictEqual } = require('assert');
8+
const { createServer, get } = require('http');
89

9-
const HTTP_CLIENT_TIMEOUT = 2000;
10-
11-
const options = {
12-
method: 'GET',
13-
port: undefined,
14-
host: '127.0.0.1',
15-
path: '/',
16-
timeout: HTTP_CLIENT_TIMEOUT,
17-
};
18-
19-
const server = http.createServer(() => {
10+
const server = createServer(() => {
2011
// Never respond.
2112
});
2213

23-
server.listen(0, options.host, function() {
24-
doRequest();
25-
});
26-
27-
function doRequest() {
28-
options.port = server.address().port;
29-
const req = http.request(options);
30-
req.setTimeout(HTTP_CLIENT_TIMEOUT / 2);
31-
req.on('error', () => {
32-
// This space is intentionally left blank.
14+
server.listen(0, mustCall(() => {
15+
const req = get({
16+
port: server.address().port,
17+
timeout: 2000,
3318
});
34-
req.on('close', common.mustCall(() => server.close()));
3519

36-
let timeout_events = 0;
37-
req.on('timeout', common.mustCall(() => {
38-
timeout_events += 1;
20+
req.setTimeout(1000);
21+
22+
req.on('socket', mustCall((socket) => {
23+
strictEqual(socket.timeout, 2000);
24+
25+
socket.on('connect', mustCall(() => {
26+
strictEqual(socket.timeout, 1000);
27+
28+
// Reschedule the timer to not wait 1 sec and make the test finish faster.
29+
socket.setTimeout(10);
30+
strictEqual(socket.timeout, 10);
31+
}));
32+
}));
33+
34+
req.on('error', expectsError({
35+
type: Error,
36+
code: 'ECONNRESET',
37+
message: 'socket hang up'
3938
}));
40-
req.end();
4139

42-
setTimeout(function() {
40+
req.on('close', mustCall(() => {
41+
server.close();
42+
}));
43+
44+
req.on('timeout', mustCall(() => {
45+
strictEqual(req.socket.listenerCount('timeout'), 0);
4346
req.destroy();
44-
assert.strictEqual(timeout_events, 1);
45-
}, common.platformTimeout(HTTP_CLIENT_TIMEOUT));
46-
}
47+
}));
48+
}));
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,23 @@
11
'use strict';
2-
const common = require('../common');
32

4-
// Test that when http request uses both timeout and agent,
5-
// timeout will work as expected.
3+
// Test that the request `timeout` option has precedence over the agent
4+
// `timeout` option.
65

7-
const assert = require('assert');
8-
const http = require('http');
6+
const { mustCall } = require('../common');
7+
const { Agent, get } = require('http');
8+
const { strictEqual } = require('assert');
99

10-
const HTTP_AGENT_TIMEOUT = 1000;
11-
const HTTP_CLIENT_TIMEOUT = 3000;
12-
13-
const agent = new http.Agent({ timeout: HTTP_AGENT_TIMEOUT });
14-
const options = {
15-
method: 'GET',
16-
port: undefined,
17-
host: '127.0.0.1',
18-
path: '/',
19-
timeout: HTTP_CLIENT_TIMEOUT,
20-
agent,
21-
};
22-
23-
const server = http.createServer(() => {
24-
// Never respond.
25-
});
26-
27-
server.listen(0, options.host, () => {
28-
doRequest();
10+
const request = get({
11+
agent: new Agent({ timeout: 50 }),
12+
lookup: () => {},
13+
timeout: 100
2914
});
3015

31-
function doRequest() {
32-
options.port = server.address().port;
33-
const start = process.hrtime.bigint();
34-
const req = http.request(options);
35-
req.on('error', () => {
36-
// This space is intentionally left blank.
37-
});
38-
req.on('close', common.mustCall(() => server.close()));
16+
request.on('socket', mustCall((socket) => {
17+
strictEqual(socket.timeout, 100);
3918

40-
let timeout_events = 0;
41-
req.on('timeout', common.mustCall(() => {
42-
timeout_events += 1;
43-
const duration = process.hrtime.bigint() - start;
44-
// The timeout event cannot be precisely timed. It will delay
45-
// some number of milliseconds.
46-
assert.ok(
47-
duration >= BigInt(HTTP_CLIENT_TIMEOUT * 1e6),
48-
`duration ${duration}ms less than timeout ${HTTP_CLIENT_TIMEOUT}ms`
49-
);
50-
}));
51-
req.end();
19+
const listeners = socket.listeners('timeout');
5220

53-
setTimeout(() => {
54-
req.destroy();
55-
assert.strictEqual(timeout_events, 1);
56-
// Ensure the `timeout` event fired only once.
57-
}, common.platformTimeout(HTTP_CLIENT_TIMEOUT * 2));
58-
}
21+
strictEqual(listeners.length, 1);
22+
strictEqual(listeners[0], request.timeoutCb);
23+
}));

0 commit comments

Comments
 (0)