From 80a71f033d176e7975d7237c9edfa4cde330ecdb Mon Sep 17 00:00:00 2001 From: Ido schachter Date: Tue, 25 Jul 2017 14:43:24 +0300 Subject: [PATCH 1/5] allow client to specify handshake request timeout --- doc/ws.md | 1 + lib/WebSocket.js | 10 ++++++++++ test/WebSocket.test.js | 24 ++++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/doc/ws.md b/doc/ws.md index 4da6e4ea7..9e5e6f413 100644 --- a/doc/ws.md +++ b/doc/ws.md @@ -170,6 +170,7 @@ This class represents a WebSocket. It extends the `EventEmitter`. - `protocols` {String|Array} The list of subprotocols. - `options` {Object} - `protocol` {String} Value of the `Sec-WebSocket-Protocol` header. + - `handshakeTimeout` {Number} Timeout in milliseconds for the handshake request. - `perMessageDeflate` {Boolean|Object} Enable/disable permessage-deflate. - `localAddress` {String} Local interface to bind for network connections. - `protocolVersion` {Number} Value of the `Sec-WebSocket-Version` header. diff --git a/lib/WebSocket.js b/lib/WebSocket.js index 320c725e1..fb5a7d531 100644 --- a/lib/WebSocket.js +++ b/lib/WebSocket.js @@ -471,6 +471,7 @@ function initAsServerClient (socket, head, options) { * @param {Object} options Connection options * @param {String} options.protocol Value of the `Sec-WebSocket-Protocol` header * @param {(Boolean|Object)} options.perMessageDeflate Enable/disable permessage-deflate + * @param {Number} options.handshakeTimeout Timeout in milliseconds for the handshake request * @param {String} options.localAddress Local interface to bind for network connections * @param {Number} options.protocolVersion Value of the `Sec-WebSocket-Version` header * @param {Object} options.headers An object containing request headers @@ -493,6 +494,7 @@ function initAsClient (address, protocols, options) { protocolVersion: protocolVersions[1], protocol: protocols.join(','), perMessageDeflate: true, + handshakeTimeout: null, localAddress: null, headers: null, family: null, @@ -632,6 +634,14 @@ function initAsClient (address, protocols, options) { this._req = httpObj.get(requestOptions); + if (options.handshakeTimeout) { + this._req.setTimeout(options.handshakeTimeout, () => { + this._req.abort(); + this.emit('error', new Error('timeout')); + this.finalize(true); + }); + } + this._req.on('error', (error) => { if (this._req.aborted) return; diff --git a/test/WebSocket.test.js b/test/WebSocket.test.js index 0ebfbe52d..96f93725f 100644 --- a/test/WebSocket.test.js +++ b/test/WebSocket.test.js @@ -461,6 +461,30 @@ describe('WebSocket', function () { req.abort(); }); }); + + it('error is emitted if server response exceeds timeout', function (done) { + const timeout = 100; + server.once('upgrade', (req, socket) => { + setTimeout(() => { + socket.end( + `HTTP/1.1 401 ${http.STATUS_CODES[401]}\r\n` + + 'Connection: close\r\n' + + 'Content-type: text/html\r\n' + + `Content-Length: ${http.STATUS_CODES[401].length}\r\n` + + '\r\n' + ); + }, timeout * 1.5); + }); + + const ws = new WebSocket(`ws://localhost:${port}`, null, { handshakeTimeout: timeout }); + + ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here')); + ws.on('error', (err) => { + assert.ok(err instanceof Error); + assert.strictEqual(err.message, 'timeout'); + done(); + }); + }); }); describe('connection with query string', function () { From 719dca5c542a1962e6631d33093e0b867697a2f8 Mon Sep 17 00:00:00 2001 From: Ido schachter Date: Tue, 25 Jul 2017 17:58:26 +0300 Subject: [PATCH 2/5] pr fixes: - changes name of test - changes the error message that is returned and also adds an error code --- lib/WebSocket.js | 4 +++- test/WebSocket.test.js | 11 +++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/WebSocket.js b/lib/WebSocket.js index fb5a7d531..314e77ffa 100644 --- a/lib/WebSocket.js +++ b/lib/WebSocket.js @@ -636,8 +636,10 @@ function initAsClient (address, protocols, options) { if (options.handshakeTimeout) { this._req.setTimeout(options.handshakeTimeout, () => { + const err = new Error('opening handshake has timed out'); + err.code = 'TIMEOUT'; this._req.abort(); - this.emit('error', new Error('timeout')); + this.emit('error', err); this.finalize(true); }); } diff --git a/test/WebSocket.test.js b/test/WebSocket.test.js index 96f93725f..8aa385d0d 100644 --- a/test/WebSocket.test.js +++ b/test/WebSocket.test.js @@ -462,7 +462,7 @@ describe('WebSocket', function () { }); }); - it('error is emitted if server response exceeds timeout', function (done) { + it('emits an error if the opening handshake timeout expires', function (done) { const timeout = 100; server.once('upgrade', (req, socket) => { setTimeout(() => { @@ -476,12 +476,15 @@ describe('WebSocket', function () { }, timeout * 1.5); }); - const ws = new WebSocket(`ws://localhost:${port}`, null, { handshakeTimeout: timeout }); + const ws = new WebSocket(`ws://localhost:${port}`, null, { + handshakeTimeout: timeout + }); - ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here')); + ws.on('open', () => assert.fail(null, null, 'connect shouldn\'t be raised here')); ws.on('error', (err) => { assert.ok(err instanceof Error); - assert.strictEqual(err.message, 'timeout'); + assert.strictEqual(err.code, 'TIMEOUT'); + assert.strictEqual(err.message, 'opening handshake has timed out'); done(); }); }); From 113346a8c1debf294fd906dd1d8b14b94b730583 Mon Sep 17 00:00:00 2001 From: Ido schachter Date: Tue, 25 Jul 2017 19:07:37 +0300 Subject: [PATCH 3/5] pr fixes: removed code --- lib/WebSocket.js | 4 +--- test/WebSocket.test.js | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/WebSocket.js b/lib/WebSocket.js index 314e77ffa..760ae34fa 100644 --- a/lib/WebSocket.js +++ b/lib/WebSocket.js @@ -636,10 +636,8 @@ function initAsClient (address, protocols, options) { if (options.handshakeTimeout) { this._req.setTimeout(options.handshakeTimeout, () => { - const err = new Error('opening handshake has timed out'); - err.code = 'TIMEOUT'; this._req.abort(); - this.emit('error', err); + this.emit('error', new Error('opening handshake has timed out')); this.finalize(true); }); } diff --git a/test/WebSocket.test.js b/test/WebSocket.test.js index 8aa385d0d..c5f99b465 100644 --- a/test/WebSocket.test.js +++ b/test/WebSocket.test.js @@ -483,7 +483,6 @@ describe('WebSocket', function () { ws.on('open', () => assert.fail(null, null, 'connect shouldn\'t be raised here')); ws.on('error', (err) => { assert.ok(err instanceof Error); - assert.strictEqual(err.code, 'TIMEOUT'); assert.strictEqual(err.message, 'opening handshake has timed out'); done(); }); From 87321b36162ece113e6687b08d1271b49e8babe8 Mon Sep 17 00:00:00 2001 From: Ido schachter Date: Tue, 25 Jul 2017 19:31:41 +0300 Subject: [PATCH 4/5] pr test fixes: proper close of server socket --- test/WebSocket.test.js | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/test/WebSocket.test.js b/test/WebSocket.test.js index c5f99b465..ffa86365b 100644 --- a/test/WebSocket.test.js +++ b/test/WebSocket.test.js @@ -464,17 +464,7 @@ describe('WebSocket', function () { it('emits an error if the opening handshake timeout expires', function (done) { const timeout = 100; - server.once('upgrade', (req, socket) => { - setTimeout(() => { - socket.end( - `HTTP/1.1 401 ${http.STATUS_CODES[401]}\r\n` + - 'Connection: close\r\n' + - 'Content-type: text/html\r\n' + - `Content-Length: ${http.STATUS_CODES[401].length}\r\n` + - '\r\n' - ); - }, timeout * 1.5); - }); + server.once('upgrade', (req, socket) => socket.on('end', socket.end)); const ws = new WebSocket(`ws://localhost:${port}`, null, { handshakeTimeout: timeout From 3803a4b220d94f925f0bbda08bc981dbd594f389 Mon Sep 17 00:00:00 2001 From: Ido schachter Date: Tue, 25 Jul 2017 19:34:14 +0300 Subject: [PATCH 5/5] pr test fixes: removed single used variable --- test/WebSocket.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/WebSocket.test.js b/test/WebSocket.test.js index ffa86365b..1aa45d12e 100644 --- a/test/WebSocket.test.js +++ b/test/WebSocket.test.js @@ -463,11 +463,10 @@ describe('WebSocket', function () { }); it('emits an error if the opening handshake timeout expires', function (done) { - const timeout = 100; server.once('upgrade', (req, socket) => socket.on('end', socket.end)); const ws = new WebSocket(`ws://localhost:${port}`, null, { - handshakeTimeout: timeout + handshakeTimeout: 100 }); ws.on('open', () => assert.fail(null, null, 'connect shouldn\'t be raised here'));