From 648d668fcc29dafeb99f90d89087b682457f96b3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 21 Feb 2018 17:52:35 +0100 Subject: [PATCH] http: emit timeout duration overflow warning sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/18906 Reviewed-By: Colin Ihrig Reviewed-By: Ruben Bridgewater Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell --- lib/_http_client.js | 2 ++ .../test-http-timeout-client-warning.js | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 test/parallel/test-http-timeout-client-warning.js diff --git a/lib/_http_client.js b/lib/_http_client.js index 727dc649c016ae..7bf485b47e60aa 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -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]/; @@ -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'); diff --git a/test/parallel/test-http-timeout-client-warning.js b/test/parallel/test-http-timeout-client-warning.js new file mode 100644 index 00000000000000..f11515b95fe340 --- /dev/null +++ b/test/parallel/test-http-timeout-client-warning.js @@ -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(); +}));