Skip to content

Commit

Permalink
http: emit timeout duration overflow warning sync
Browse files Browse the repository at this point in the history
Emit the `TimeoutOverflowWarning` synchronously, even when
still connecting, to get a better stack trace.

With thanks to Anatoli Papirovski for providing helpful
tips when writing the test.

PR-URL: #18906
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax committed Mar 4, 2018
1 parent d83f830 commit 648d668
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const { urlToOptions, searchParamsSymbol } = require('internal/url');
const { outHeadersKey, ondrain } = require('internal/http');
const { nextTick } = require('internal/process/next_tick');
const errors = require('internal/errors');
const { validateTimerDuration } = require('internal/timers');

const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;

Expand Down Expand Up @@ -678,6 +679,7 @@ function _deferToConnect(method, arguments_, cb) {
}

ClientRequest.prototype.setTimeout = function setTimeout(msecs, callback) {
msecs = validateTimerDuration(msecs);
if (callback) this.once('timeout', callback);

const emitTimeout = () => this.emit('timeout');
Expand Down
21 changes: 21 additions & 0 deletions test/parallel/test-http-timeout-client-warning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';
const common = require('../common');
const http = require('http');
const assert = require('assert');

// Checks that the setTimeout duration overflow warning is emitted
// synchronously and therefore contains a meaningful stacktrace.

process.on('warning', common.mustCall((warning) => {
assert(warning.stack.includes(__filename));
}));

const server = http.createServer((req, resp) => resp.end());
server.listen(common.mustCall(() => {
http.request(`http://localhost:${server.address().port}`)
.setTimeout(2 ** 40)
.on('response', common.mustCall(() => {
server.close();
}))
.end();
}));

0 comments on commit 648d668

Please sign in to comment.