Skip to content

Commit

Permalink
http2: remove unused onTimeout, add timeout tests
Browse files Browse the repository at this point in the history
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: nodejs/node#15539
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
apapirovski authored and addaleax committed Sep 30, 2017
1 parent c458d18 commit ebe8c88
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 19 deletions.
19 changes: 2 additions & 17 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -2138,23 +2138,8 @@ const setTimeout = {
return this;
}
};

const onTimeout = {
configurable: false,
enumerable: false,
value: function() {
process.nextTick(emit.bind(this, 'timeout'));
}
};

Object.defineProperties(Http2Stream.prototype, {
setTimeout,
onTimeout
});
Object.defineProperties(Http2Session.prototype, {
setTimeout,
onTimeout
});
Object.defineProperty(Http2Stream.prototype, 'setTimeout', setTimeout);
Object.defineProperty(Http2Session.prototype, 'setTimeout', setTimeout);

// --------------------------------------------------------------------

Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-server-startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ assert.doesNotThrow(() => {
if (client)
client.end();
}));
server.setTimeout(common.platformTimeout(1000));
server.setTimeout(common.platformTimeout(1000), common.mustCall());
server.listen(0, common.mustCall(() => {
const port = server.address().port;
client = net.connect(port, common.mustCall());
Expand All @@ -70,7 +70,7 @@ assert.doesNotThrow(() => {
if (client)
client.end();
}));
server.setTimeout(common.platformTimeout(1000));
server.setTimeout(common.platformTimeout(1000), common.mustCall());
server.listen(0, common.mustCall(() => {
const port = server.address().port;
client = tls.connect({
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-http2-timeouts.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,32 @@ server.on('stream', common.mustCall((stream) => {
stream.respond({ ':status': 200 });
stream.end('hello world');
}));

// check that expected errors are thrown with wrong args
common.expectsError(
() => stream.setTimeout('100'),
{
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "msecs" argument must be of type number'
}
);
common.expectsError(
() => stream.setTimeout(0, Symbol('test')),
{
code: 'ERR_INVALID_CALLBACK',
type: TypeError,
message: 'Callback must be a function'
}
);
common.expectsError(
() => stream.setTimeout(100, {}),
{
code: 'ERR_INVALID_CALLBACK',
type: TypeError,
message: 'Callback must be a function'
}
);
}));
server.listen(0);

Expand Down

0 comments on commit ebe8c88

Please sign in to comment.