Skip to content

Commit 33760cc

Browse files
apapirovskiBridgeAR
authored andcommitted
http2: remove unused onTimeout, add timeout tests
Remove unused onTimeout on Http2Session and Http2Stream because the correct _onTimeout is already declared and in use. Expand timeout tests to handle edge cases and additional arguments. PR-URL: #15539 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent b2eb987 commit 33760cc

File tree

3 files changed

+30
-19
lines changed

3 files changed

+30
-19
lines changed

lib/internal/http2/core.js

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2138,23 +2138,8 @@ const setTimeout = {
21382138
return this;
21392139
}
21402140
};
2141-
2142-
const onTimeout = {
2143-
configurable: false,
2144-
enumerable: false,
2145-
value: function() {
2146-
process.nextTick(emit.bind(this, 'timeout'));
2147-
}
2148-
};
2149-
2150-
Object.defineProperties(Http2Stream.prototype, {
2151-
setTimeout,
2152-
onTimeout
2153-
});
2154-
Object.defineProperties(Http2Session.prototype, {
2155-
setTimeout,
2156-
onTimeout
2157-
});
2141+
Object.defineProperty(Http2Stream.prototype, 'setTimeout', setTimeout);
2142+
Object.defineProperty(Http2Session.prototype, 'setTimeout', setTimeout);
21582143

21592144
// --------------------------------------------------------------------
21602145

test/parallel/test-http2-server-startup.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ assert.doesNotThrow(() => {
5454
if (client)
5555
client.end();
5656
}));
57-
server.setTimeout(common.platformTimeout(1000));
57+
server.setTimeout(common.platformTimeout(1000), common.mustCall());
5858
server.listen(0, common.mustCall(() => {
5959
const port = server.address().port;
6060
client = net.connect(port, common.mustCall());
@@ -70,7 +70,7 @@ assert.doesNotThrow(() => {
7070
if (client)
7171
client.end();
7272
}));
73-
server.setTimeout(common.platformTimeout(1000));
73+
server.setTimeout(common.platformTimeout(1000), common.mustCall());
7474
server.listen(0, common.mustCall(() => {
7575
const port = server.address().port;
7676
client = tls.connect({

test/parallel/test-http2-timeouts.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,32 @@ server.on('stream', common.mustCall((stream) => {
1414
stream.respond({ ':status': 200 });
1515
stream.end('hello world');
1616
}));
17+
18+
// check that expected errors are thrown with wrong args
19+
common.expectsError(
20+
() => stream.setTimeout('100'),
21+
{
22+
code: 'ERR_INVALID_ARG_TYPE',
23+
type: TypeError,
24+
message: 'The "msecs" argument must be of type number'
25+
}
26+
);
27+
common.expectsError(
28+
() => stream.setTimeout(0, Symbol('test')),
29+
{
30+
code: 'ERR_INVALID_CALLBACK',
31+
type: TypeError,
32+
message: 'Callback must be a function'
33+
}
34+
);
35+
common.expectsError(
36+
() => stream.setTimeout(100, {}),
37+
{
38+
code: 'ERR_INVALID_CALLBACK',
39+
type: TypeError,
40+
message: 'Callback must be a function'
41+
}
42+
);
1743
}));
1844
server.listen(0);
1945

0 commit comments

Comments
 (0)