Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow client to specify handshake request timeout #1177

Merged
merged 5 commits into from Jul 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/ws.md
Expand Up @@ -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.
Expand Down
10 changes: 10 additions & 0 deletions lib/WebSocket.js
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -632,6 +634,14 @@ function initAsClient (address, protocols, options) {

this._req = httpObj.get(requestOptions);

if (options.handshakeTimeout) {
this._req.setTimeout(options.handshakeTimeout, () => {
Copy link
Member

@lpinca lpinca Jul 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not work exactly as you expect but it's fine in most cases. All request.setTimeout() does is calling socket.setTimeout() on the request socket.

This means that the timeout expires only when there are x milliseconds of inactivity. It's possible that the response arrives after the specified timeout with no 'timeout' event emitted.

this._req.abort();
this.emit('error', new Error('opening handshake has timed out'));
this.finalize(true);
});
}

this._req.on('error', (error) => {
if (this._req.aborted) return;

Expand Down
15 changes: 15 additions & 0 deletions test/WebSocket.test.js
Expand Up @@ -461,6 +461,21 @@ describe('WebSocket', function () {
req.abort();
});
});

it('emits an error if the opening handshake timeout expires', function (done) {
server.once('upgrade', (req, socket) => socket.on('end', socket.end));

const ws = new WebSocket(`ws://localhost:${port}`, null, {
handshakeTimeout: 100
});

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, 'opening handshake has timed out');
done();
});
});
});

describe('connection with query string', function () {
Expand Down