Skip to content

Commit

Permalink
[major] Remove WebSocket#pause() and WebSocket#resume()
Browse files Browse the repository at this point in the history
This is in preparation for read backpressure handling when
permessage-deflate is enabled. The user should not interfere by
pausing/resuming the underlying `net.Socket` stream.
  • Loading branch information
lpinca committed Jan 5, 2018
1 parent 1c783c2 commit a206e98
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 101 deletions.
8 changes: 0 additions & 8 deletions doc/ws.md
Expand Up @@ -344,10 +344,6 @@ listener receives a `MessageEvent` named "message".
An event listener to be called when the connection is established. The listener
receives an `OpenEvent` named "open".

### websocket.pause()

Pause the socket.

### websocket.ping([data[, mask]][, callback])

- `data` {Any} The data to send in the ping frame.
Expand Down Expand Up @@ -387,10 +383,6 @@ The current state of the connection. This is one of the ready state constants.

Removes an event listener emulating the `EventTarget` interface.

### websocket.resume()

Resume the socket.

### websocket.send(data[, options][, callback])

- `data` {Any} The data to send.
Expand Down
32 changes: 0 additions & 32 deletions lib/websocket.js
Expand Up @@ -225,38 +225,6 @@ class WebSocket extends EventEmitter {
this.removeAllListeners();
}

/**
* Pause the socket stream.
*
* @public
*/
pause () {
if (this.readyState !== WebSocket.OPEN) {
throw new Error(
`WebSocket is not open: readyState ${this.readyState} ` +
`(${readyStates[this.readyState]})`
);
}

this._socket.pause();
}

/**
* Resume the socket stream
*
* @public
*/
resume () {
if (this.readyState !== WebSocket.OPEN) {
throw new Error(
`WebSocket is not open: readyState ${this.readyState} ` +
`(${readyStates[this.readyState]})`
);
}

this._socket.resume();
}

/**
* Start a closing handshake.
*
Expand Down
61 changes: 0 additions & 61 deletions test/websocket.test.js
Expand Up @@ -682,67 +682,6 @@ describe('WebSocket', function () {
});
});

describe('#pause and #resume', function () {
it('throws an error when `readyState` is not `OPEN` (pause)', function () {
const ws = new WebSocket('ws://localhost', { agent: new CustomAgent() });

assert.throws(
() => ws.pause(),
/^Error: WebSocket is not open: readyState 0 \(CONNECTING\)$/
);
});

it('throws an error when `readyState` is not `OPEN` (resume)', function () {
const ws = new WebSocket('ws://localhost', { agent: new CustomAgent() });

assert.throws(
() => ws.resume(),
/^Error: WebSocket is not open: readyState 0 \(CONNECTING\)$/
);
});

it('pauses the underlying stream', function (done) {
// this test is sort-of racecondition'y, since an unlikely slow connection
// to localhost can cause the test to succeed even when the stream pausing
// isn't working as intended. that is an extremely unlikely scenario, though
// and an acceptable risk for the test.
let openCount = 0;
let serverClient;
let client;

const onOpen = () => {
if (++openCount !== 2) return;

let paused = true;
serverClient.on('message', () => {
assert.ok(!paused);
wss.close(done);
});
serverClient.pause();

setTimeout(() => {
paused = false;
serverClient.resume();
}, 200);

client.send('foo');
};

const wss = new WebSocket.Server({ port: 0 }, () => {
const port = wss._server.address().port;
const ws = new WebSocket(`ws://localhost:${port}`);

serverClient = ws;
serverClient.on('open', onOpen);
});

wss.on('connection', (ws) => {
client = ws;
onOpen();
});
});
});

describe('#ping', function () {
it('throws an error if `readyState` is not `OPEN`', function (done) {
const ws = new WebSocket('ws://localhost', {
Expand Down

3 comments on commit a206e98

@cbratschi
Copy link

Choose a reason for hiding this comment

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

Too bad those two methods were removed. We are using ws in combination with Express where we have to pause the WebSocket first until the Express bindings are ready.

@lpinca
Copy link
Member Author

@lpinca lpinca commented on a206e98 Jan 17, 2018

Choose a reason for hiding this comment

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

I wouldn't recommend it but you can still achieve the same result by using the private _socket property.

ws._socket.pause();

@jacott
Copy link

@jacott jacott commented on a206e98 May 20, 2018

Choose a reason for hiding this comment

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

@cbratschi you could use request.socket instead which is not private.

Please sign in to comment.