Skip to content

Commit

Permalink
[major] Use an options object instead of positional arguments
Browse files Browse the repository at this point in the history 
Make the `Receiver` constructor take a single options object argument
instead of multiple positional arguments.
  • Loading branch information
@lpinca
lpinca committed Aug 11, 2021
1 parent 7f0b5c4 commit 1e938f1
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 62 deletions.
6 changes: 5 additions & 1 deletion bench/parser.benchmark.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ const binaryFrame3 = createBinaryFrame(200 * 1024);
const binaryFrame4 = createBinaryFrame(1024 * 1024);

const suite = new benchmark.Suite();
const receiver = new Receiver('nodebuffer', {}, true);
const receiver = new Receiver({
binaryType: 'nodebuffer',
extensions: {},
isServer: true
});

suite.add('ping frame (5 bytes payload)', {
defer: true,
Expand Down
22 changes: 12 additions & 10 deletions lib/receiver.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,22 @@ class Receiver extends Writable {
/**
* Creates a Receiver instance.
*
* @param {String} [binaryType=nodebuffer] The type for binary data
* @param {Object} [extensions] An object containing the negotiated extensions
* @param {Boolean} [isServer=false] Specifies whether to operate in client or
* server mode
* @param {Number} [maxPayload=0] The maximum allowed message length
* @param {Object} [options] Options object
* @param {String} [options.binaryType=nodebuffer] The type for binary data
* @param {Object} [options.extensions] An object containing the negotiated
* extensions
* @param {Boolean} [options.isServer=false] Specifies whether to operate in
* client or server mode
* @param {Number} [options.maxPayload=0] The maximum allowed message length
*/
constructor(binaryType, extensions, isServer, maxPayload) {
constructor(options = {}) {
super();

this._binaryType = binaryType || BINARY_TYPES[0];
this._binaryType = options.binaryType || BINARY_TYPES[0];
this._extensions = options.extensions || {};
this._isServer = !!options.isServer;
this._maxPayload = options.maxPayload | 0;
this[kWebSocket] = undefined;
this._extensions = extensions || {};
this._isServer = !!isServer;
this._maxPayload = maxPayload | 0;

this._bufferedBytes = 0;
this._buffers = [];
Expand Down
10 changes: 5 additions & 5 deletions lib/websocket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,12 @@ class WebSocket extends EventEmitter {
* @private
*/
setSocket(socket, head, maxPayload) {
const receiver = new Receiver(
this.binaryType,
this._extensions,
this._isServer,
const receiver = new Receiver({
binaryType: this.binaryType,
extensions: this._extensions,
isServer: this._isServer,
maxPayload
);
});

this._sender = new Sender(socket, this._extensions);
this._receiver = receiver;
Expand Down
98 changes: 52 additions & 46 deletions test/receiver.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('Receiver', () => {
});

it('parses a masked text message', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });

receiver.on('message', (data, isBinary) => {
assert.deepStrictEqual(data, Buffer.from('5:::{"name":"echo"}'));
Expand All @@ -61,7 +61,7 @@ describe('Receiver', () => {
});

it('parses a masked text message longer than 125 B', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });
const msg = Buffer.from('A'.repeat(200));

const list = Sender.frame(msg, {
Expand All @@ -85,7 +85,7 @@ describe('Receiver', () => {
});

it('parses a really long masked text message', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });
const msg = Buffer.from('A'.repeat(64 * 1024));

const list = Sender.frame(msg, {
Expand All @@ -108,7 +108,7 @@ describe('Receiver', () => {
});

it('parses a 300 B fragmented masked text message', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });
const msg = Buffer.from('A'.repeat(300));

const fragment1 = msg.slice(0, 150);
Expand Down Expand Up @@ -142,7 +142,7 @@ describe('Receiver', () => {
});

it('parses a ping message', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });
const msg = Buffer.from('Hello');

const list = Sender.frame(msg, {
Expand Down Expand Up @@ -175,7 +175,7 @@ describe('Receiver', () => {
});

it('parses a 300 B fragmented masked text message with a ping in the middle (1/2)', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });
const msg = Buffer.from('A'.repeat(300));
const pingMessage = Buffer.from('Hello');

Expand Down Expand Up @@ -225,7 +225,7 @@ describe('Receiver', () => {
});

it('parses a 300 B fragmented masked text message with a ping in the middle (2/2)', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });
const msg = Buffer.from('A'.repeat(300));
const pingMessage = Buffer.from('Hello');

Expand Down Expand Up @@ -285,7 +285,7 @@ describe('Receiver', () => {
});

it('parses a 100 B masked binary message', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });
const msg = crypto.randomBytes(100);

const list = Sender.frame(msg, {
Expand All @@ -308,7 +308,7 @@ describe('Receiver', () => {
});

it('parses a 256 B masked binary message', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });
const msg = crypto.randomBytes(256);

const list = Sender.frame(msg, {
Expand All @@ -331,7 +331,7 @@ describe('Receiver', () => {
});

it('parses a 200 KiB masked binary message', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });
const msg = crypto.randomBytes(200 * 1024);

const list = Sender.frame(msg, {
Expand Down Expand Up @@ -380,8 +380,10 @@ describe('Receiver', () => {
const perMessageDeflate = new PerMessageDeflate();
perMessageDeflate.accept([{}]);

const receiver = new Receiver(undefined, {
'permessage-deflate': perMessageDeflate
const receiver = new Receiver({
extensions: {
'permessage-deflate': perMessageDeflate
}
});
const buf = Buffer.from('Hello');

Expand All @@ -403,8 +405,10 @@ describe('Receiver', () => {
const perMessageDeflate = new PerMessageDeflate();
perMessageDeflate.accept([{}]);

const receiver = new Receiver(undefined, {
'permessage-deflate': perMessageDeflate
const receiver = new Receiver({
extensions: {
'permessage-deflate': perMessageDeflate
}
});
const buf1 = Buffer.from('foo');
const buf2 = Buffer.from('bar');
Expand Down Expand Up @@ -451,7 +455,7 @@ describe('Receiver', () => {
});

it('resets `totalPayloadLength` only on final frame (unfragmented)', (done) => {
const receiver = new Receiver(undefined, {}, false, 10);
const receiver = new Receiver({ maxPayload: 10 });

receiver.on('message', (data, isBinary) => {
assert.strictEqual(receiver._totalPayloadLength, 0);
Expand All @@ -465,7 +469,7 @@ describe('Receiver', () => {
});

it('resets `totalPayloadLength` only on final frame (fragmented)', (done) => {
const receiver = new Receiver(undefined, {}, false, 10);
const receiver = new Receiver({ maxPayload: 10 });

receiver.on('message', (data, isBinary) => {
assert.strictEqual(receiver._totalPayloadLength, 0);
Expand All @@ -481,7 +485,7 @@ describe('Receiver', () => {
});

it('resets `totalPayloadLength` only on final frame (fragmented + ping)', (done) => {
const receiver = new Receiver(undefined, {}, false, 10);
const receiver = new Receiver({ maxPayload: 10 });
let data;

receiver.on('ping', (buf) => {
Expand All @@ -506,8 +510,10 @@ describe('Receiver', () => {
const perMessageDeflate = new PerMessageDeflate();
perMessageDeflate.accept([{}]);

const receiver = new Receiver(undefined, {
'permessage-deflate': perMessageDeflate
const receiver = new Receiver({
extensions: {
'permessage-deflate': perMessageDeflate
}
});
const results = [];
const push = results.push.bind(results);
Expand Down Expand Up @@ -549,8 +555,10 @@ describe('Receiver', () => {
const perMessageDeflate = new PerMessageDeflate();
perMessageDeflate.accept([{}]);

const receiver = new Receiver(undefined, {
'permessage-deflate': perMessageDeflate
const receiver = new Receiver({
extensions: {
'permessage-deflate': perMessageDeflate
}
});

receiver.on('error', (err) => {
Expand Down Expand Up @@ -675,8 +683,10 @@ describe('Receiver', () => {
const perMessageDeflate = new PerMessageDeflate();
perMessageDeflate.accept([{}]);

const receiver = new Receiver(undefined, {
'permessage-deflate': perMessageDeflate
const receiver = new Receiver({
extensions: {
'permessage-deflate': perMessageDeflate
}
});

receiver.on('error', (err) => {
Expand Down Expand Up @@ -711,7 +721,7 @@ describe('Receiver', () => {
});

it('emits an error if a frame has the MASK bit off (server mode)', (done) => {
const receiver = new Receiver(undefined, {}, true);
const receiver = new Receiver({ isServer: true });

receiver.on('error', (err) => {
assert.ok(err instanceof RangeError);
Expand All @@ -728,7 +738,7 @@ describe('Receiver', () => {
});

it('emits an error if a frame has the MASK bit on (client mode)', (done) => {
const receiver = new Receiver(undefined, {}, false);
const receiver = new Receiver();

receiver.on('error', (err) => {
assert.ok(err instanceof RangeError);
Expand Down Expand Up @@ -806,8 +816,10 @@ describe('Receiver', () => {
const perMessageDeflate = new PerMessageDeflate();
perMessageDeflate.accept([{}]);

const receiver = new Receiver(undefined, {
'permessage-deflate': perMessageDeflate
const receiver = new Receiver({
extensions: {
'permessage-deflate': perMessageDeflate
}
});
const buf = Buffer.from([0xce, 0xba, 0xe1, 0xbd]);

Expand Down Expand Up @@ -884,7 +896,7 @@ describe('Receiver', () => {
});

it('emits an error if a frame payload length is bigger than `maxPayload`', (done) => {
const receiver = new Receiver(undefined, {}, true, 20 * 1024);
const receiver = new Receiver({ isServer: true, maxPayload: 20 * 1024 });
const msg = crypto.randomBytes(200 * 1024);

const list = Sender.frame(msg, {
Expand Down Expand Up @@ -912,14 +924,11 @@ describe('Receiver', () => {
const perMessageDeflate = new PerMessageDeflate({}, false, 25);
perMessageDeflate.accept([{}]);

const receiver = new Receiver(
undefined,
{
'permessage-deflate': perMessageDeflate
},
false,
25
);
const receiver = new Receiver({
extensions: { 'permessage-deflate': perMessageDeflate },
isServer: false,
maxPayload: 25
});
const buf = Buffer.from('A'.repeat(50));

receiver.on('error', (err) => {
Expand All @@ -942,14 +951,11 @@ describe('Receiver', () => {
const perMessageDeflate = new PerMessageDeflate({}, false, 25);
perMessageDeflate.accept([{}]);

const receiver = new Receiver(
undefined,
{
'permessage-deflate': perMessageDeflate
},
false,
25
);
const receiver = new Receiver({
extensions: { 'permessage-deflate': perMessageDeflate },
isServer: false,
maxPayload: 25
});
const buf = Buffer.from('A'.repeat(15));

receiver.on('error', (err) => {
Expand Down Expand Up @@ -1002,7 +1008,7 @@ describe('Receiver', () => {
});

it("honors the 'arraybuffer' binary type", (done) => {
const receiver = new Receiver('arraybuffer');
const receiver = new Receiver({ binaryType: 'arraybuffer' });
const frags = [
crypto.randomBytes(19221),
crypto.randomBytes(954),
Expand All @@ -1028,7 +1034,7 @@ describe('Receiver', () => {
});

it("honors the 'fragments' binary type", (done) => {
const receiver = new Receiver('fragments');
const receiver = new Receiver({ binaryType: 'fragments' });
const frags = [
crypto.randomBytes(17),
crypto.randomBytes(419872),
Expand Down

1 comment on commit 1e938f1

@lpinca
Copy link
Member Author

@lpinca lpinca commented on 1e938f1 Aug 11, 2021

Choose a reason for hiding this comment

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

This is technically a semver-major change because the Receiver class is exported but I'm planning to include it in the next minor version for the following reasons:

  • The latest major version was released only 13 days ago.
  • The Receiver class and its methods are completely undocumented.
  • It is unlikely that the Receiver constructor is used directly.
  • If it turns out to be a real issue, it is easy to fix with a patch that restores backwards compatibility.

Please sign in to comment.