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

Change WebSocket#{p{i,o}ng,send}() behavior #1532

Merged
merged 1 commit into from
Apr 25, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 0 additions & 25 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ can use one of the many wrappers available on npm, like
- [Server broadcast](#server-broadcast)
- [echo.websocket.org demo](#echowebsocketorg-demo)
- [Other examples](#other-examples)
- [Error handling best practices](#error-handling-best-practices)
- [FAQ](#faq)
- [How to get the IP address of the client?](#how-to-get-the-ip-address-of-the-client)
- [How to detect and close broken connections?](#how-to-detect-and-close-broken-connections)
Expand Down Expand Up @@ -309,30 +308,6 @@ examples folder.

Otherwise, see the test cases.

## Error handling best practices

```js
// If the WebSocket is closed before the following send is attempted
ws.send('something');

// Errors (both immediate and async write errors) can be detected in an optional
// callback. The callback is also the only way of being notified that data has
// actually been sent.
ws.send('something', function ack(error) {
// If error is not defined, the send has been completed, otherwise the error
// object will indicate what failed.
});

// Immediate errors can also be handled with `try...catch`, but **note** that
// since sends are inherently asynchronous, socket write failures will *not* be
// captured when this technique is used.
try {
ws.send('something');
} catch (e) {
/* handle error */
}
```

## FAQ

### How to get the IP address of the client?
Expand Down
88 changes: 61 additions & 27 deletions lib/websocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const {
kWebSocket,
NOOP
} = require('./constants');
const { toBuffer } = require('./buffer-util');

const readyStates = ['CONNECTING', 'OPEN', 'CLOSING', 'CLOSED'];
const protocolVersions = [8, 13];
Expand Down Expand Up @@ -57,6 +58,7 @@ class WebSocket extends EventEmitter {
this._socket = null;

if (address !== null) {
this._bufferedAmount = 0;
this._isServer = false;
this._redirects = 0;

Expand Down Expand Up @@ -112,7 +114,7 @@ class WebSocket extends EventEmitter {
* @type {Number}
*/
get bufferedAmount() {
if (!this._socket) return 0;
if (!this._socket) return this._bufferedAmount;

//
// `socket.bufferSize` is `undefined` if the socket is closed.
Expand Down Expand Up @@ -252,6 +254,10 @@ class WebSocket extends EventEmitter {
* @public
*/
ping(data, mask, cb) {
if (this.readyState === WebSocket.CONNECTING) {
throw new Error('WebSocket is not open: readyState 0 (CONNECTING)');
}

if (typeof data === 'function') {
cb = data;
data = mask = undefined;
Expand All @@ -260,17 +266,13 @@ class WebSocket extends EventEmitter {
mask = undefined;
}

if (this.readyState !== WebSocket.OPEN) {
const err = new Error(
`WebSocket is not open: readyState ${this.readyState} ` +
`(${readyStates[this.readyState]})`
);
if (typeof data === 'number') data = data.toString();

if (cb) return cb(err);
throw err;
if (this.readyState !== WebSocket.OPEN) {
sendAfterClose(this, data, cb);
return;
}

if (typeof data === 'number') data = data.toString();
if (mask === undefined) mask = !this._isServer;
this._sender.ping(data || EMPTY_BUFFER, mask, cb);
}
Expand All @@ -284,6 +286,10 @@ class WebSocket extends EventEmitter {
* @public
*/
pong(data, mask, cb) {
if (this.readyState === WebSocket.CONNECTING) {
throw new Error('WebSocket is not open: readyState 0 (CONNECTING)');
}

if (typeof data === 'function') {
cb = data;
data = mask = undefined;
Expand All @@ -292,17 +298,13 @@ class WebSocket extends EventEmitter {
mask = undefined;
}

if (this.readyState !== WebSocket.OPEN) {
const err = new Error(
`WebSocket is not open: readyState ${this.readyState} ` +
`(${readyStates[this.readyState]})`
);
if (typeof data === 'number') data = data.toString();

if (cb) return cb(err);
throw err;
if (this.readyState !== WebSocket.OPEN) {
sendAfterClose(this, data, cb);
return;
}

if (typeof data === 'number') data = data.toString();
if (mask === undefined) mask = !this._isServer;
this._sender.pong(data || EMPTY_BUFFER, mask, cb);
}
Expand All @@ -312,31 +314,31 @@ class WebSocket extends EventEmitter {
*
* @param {*} data The message to send
* @param {Object} options Options object
* @param {Boolean} options.compress Specifies whether or not to compress `data`
* @param {Boolean} options.compress Specifies whether or not to compress
* `data`
* @param {Boolean} options.binary Specifies whether `data` is binary or text
* @param {Boolean} options.fin Specifies whether the fragment is the last one
* @param {Boolean} options.mask Specifies whether or not to mask `data`
* @param {Function} cb Callback which is executed when data is written out
* @public
*/
send(data, options, cb) {
if (this.readyState === WebSocket.CONNECTING) {
throw new Error('WebSocket is not open: readyState 0 (CONNECTING)');
}

if (typeof options === 'function') {
cb = options;
options = {};
}

if (this.readyState !== WebSocket.OPEN) {
const err = new Error(
`WebSocket is not open: readyState ${this.readyState} ` +
`(${readyStates[this.readyState]})`
);
if (typeof data === 'number') data = data.toString();

if (cb) return cb(err);
throw err;
if (this.readyState !== WebSocket.OPEN) {
sendAfterClose(this, data, cb);
return;
}

if (typeof data === 'number') data = data.toString();

const opts = Object.assign(
{
binary: typeof data !== 'string',
Expand Down Expand Up @@ -723,6 +725,38 @@ function abortHandshake(websocket, stream, message) {
}
}

/**
* Handle cases where the `ping()`, `pong()`, or `send()` methods are called
* when the `readyState` attribute is `CLOSING` or `CLOSED`.
*
* @param {WebSocket} websocket The WebSocket instance
* @param {*} data The data to send
* @param {Function} cb Callback
* @private
*/
function sendAfterClose(websocket, data, cb) {
if (data) {
const length = toBuffer(data).length;

//
// The `_bufferedAmount` property is used only when the peer is a client and
// the opening handshake fails. Under these circumstances, in fact, the
// `setSocket()` method is not called, so the `_socket` and `_sender`
// properties are set to `null`.
//
if (websocket._socket) websocket._sender._bufferedBytes += length;
else websocket._bufferedAmount += length;
}

if (cb) {
const err = new Error(
`WebSocket is not open: readyState ${websocket.readyState} ` +
`(${readyStates[websocket.readyState]})`
);
cb(err);
}
}

/**
* The listener of the `Receiver` `'conclude'` event.
*
Expand Down
Loading