Custom websocket by MarhiievHE · Pull Request #515 · metarhia/metacom

MarhiievHE · 2025-08-25T18:49:41Z

tests and linter show no problems (npm t)
tests are added/updated for bug fixes and new features
code is properly formatted (npm run fix)
description of changes is added in CHANGELOG.md
update .d.ts typings

tshemsedinov · 2025-08-27T09:59:48Z

lib/server.js


    if (balancer) return;
-    this.wsServer = new ws.Server({ server: this.httpServer });
+    this.wsServer = new WebsocketServer(this.httpServer);


I'd propose to be compatible with ws:

Suggested change

this.wsServer = new WebsocketServer(this.httpServer);

this.wsServer = new WebsocketServer({ server: this.httpServer });

tshemsedinov · 2025-08-27T11:33:35Z

lib/websocket/constants.js

+  GOING_AWAY: {
+    reason: 'Going away',
+    buf: Buffer.from([
+      0x88, 0xc, 0x3, 0xe9, 0x47, 0x6f, 0x69, 0x6e, 0x67, 0x20, 0x61, 0x77,
+      0x61, 0x79,
+    ]),
+  },


Let's hold strings as ascii strings and all unprintable as hex, for example:

Suggested change

GOING_AWAY: {

reason: 'Going away',

buf: Buffer.from([

0x88, 0xc, 0x3, 0xe9, 0x47, 0x6f, 0x69, 0x6e, 0x67, 0x20, 0x61, 0x77,

0x61, 0x79,

]),

},

GOING_AWAY: {

reason: 'Going away',

buf: Buffer.concat([

Buffer.from([0x88, 0x0c, 0x03, 0xe9]),

Buffer.from('Going away', 'utf8'),

]),

},

tshemsedinov · 2025-08-27T11:35:51Z

lib/websocket/frame.js

+  CLOSE_CODES,
+  TWO_32,
+} = require('./constants');
+const { Result } = require('./utils/result');


Always add extensions in require

Suggested change

const { Result } = require('./utils/result');

const { Result } = require('./utils/result.js');

tshemsedinov · 2025-08-27T11:36:46Z

lib/websocket/frame.js

+module.exports = {
+  Frame,
+};


Suggested change

module.exports = {

Frame,

};

module.exports = { Frame };

And it's done

tshemsedinov · 2025-08-27T12:13:46Z

test/websocket/utils/protocolClient.js

+  on(event, cb) {
+    if (event === 'open') this._openCb = cb;
+    else if (event === 'message') this._messageCb = (buf) => cb(buf);
+    else if (event === 'frame') this._frameCb = cb;
+    else if (event === 'ping') this._pingCb = (buf) => cb(buf);
+    else if (event === 'pong') this._pongCb = (buf) => cb(buf);
+    else if (event === 'close') this._closeCb = cb;
+  }


We can just use EventEmitter

Yes, that’s a good idea — much cleaner than mocking everything. Done

tshemsedinov · 2025-08-27T12:15:54Z

test/websocket/utils/protocolClient.js

+// Methods: send(data), sendFrame(opcode, payload, { fin=true, mask=true }),
+// sendText, sendBinary, ping, close
+class ProtocolClient {
+  constructor(url) {


Wery long constructor, maybe we can decompose, for example, extracting collection:
{ [opcode]: [handler] }

tshemsedinov · 2025-08-27T12:21:04Z

lib/websocket/utils/result.js

@@ -0,0 +1,35 @@
+'use strict';


Should be moved to metautil

Should I move the Result class to metautil, or do you think it would be better to introduce an algebraic data types?

Yes, please find an issue there or just create new in metautil and PR

tshemsedinov · 2026-03-02T17:23:43Z

lib/websocket/utils/result.js

@@ -0,0 +1,35 @@
+'use strict';


Yes, please find an issue there or just create new in metautil and PR

tshemsedinov · 2026-03-02T17:36:29Z

lib/websocket/connection.js

+        this.emit(
+          'error',
+          new Error('Protocol error: Unexpected CONTINUATION without start'),
+        );
+        return void this.#close(
+          Frame.protocolErrorClose(
+            PROTOCOL_ERROR_SUBTYPES.COMMON,
+            this.#isClient,
+          ),
+        );


Use intermediate variables here and in other places

Suggested change

this.emit(

'error',

new Error('Protocol error: Unexpected CONTINUATION without start'),

);

return void this.#close(

Frame.protocolErrorClose(

PROTOCOL_ERROR_SUBTYPES.COMMON,

this.#isClient,

),

);

const error = new Error('Protocol error: Unexpected CONTINUATION without start');

this.emit('error', error);

const frame = Frame.protocolErrorClose(

PROTOCOL_ERROR_SUBTYPES.COMMON,

this.#isClient,

);

return void this.#close(frame);

tshemsedinov · 2026-03-02T17:40:13Z

lib/websocket/frame.js

+  static close(code = 1000, reason = '') {
+    if (code === null || code === undefined) {
+      const payload = Buffer.alloc(0);
+      return new this(true, OPCODES.CLOSE, false, payload, null);


Suggested change

return new this(true, OPCODES.CLOSE, false, payload, null);

return new Frame(true, OPCODES.CLOSE, false, payload, null);

tshemsedinov · 2026-03-02T17:43:47Z

lib/websocket/frame.js

+  static emptyPingBuffer(isClient = false) {
+    const buf = isClient ? Buffer.from(CLIENT_EMPTY_PING) : EMPTY_PING;
+    if (isClient) crypto.randomBytes(4).copy(buf, 2);
+    return buf;
+  }


It looks like we can preallocate this buffer to avoid calling method

tshemsedinov · 2026-03-02T17:45:28Z

lib/websocket/frame.js

+} = require('./constants.js');
+const { Result } = require('./utils/result.js');
+
+class Frame {


Think about naming improvement for this class static methods, it looks like all those methods are factories, so createPong will be better and createCloseError will be better than errorClose

tshemsedinov · 2026-03-02T17:52:13Z

test/websocket/unit/connection.test.js

+const { Connection } = require('../../../lib/websocket/connection.js');
+const { Frame } = require('../../../lib/websocket/frame.js');
+const {
+  OPCODES,
+  CLOSE_TIMEOUT,
+} = require('../../../lib/websocket/constants.js');
+const { FrameParser } = require('../../../lib/websocket/frameParser.js');
+const { MockSocket } = require('../utils/mockSocket.js');


Such long /../ constructions are not ok, let's find alternative way

This will be better require('../../../metacom.js'); but I'll propose something better later, need a time to clone and check repo locally

tshemsedinov reviewed Aug 27, 2025

View reviewed changes

tshemsedinov changed the title ~~WIP: Custom websocket~~ Custom websocket Jan 8, 2026

MarhiievHE added 9 commits February 24, 2026 15:52

Add websocket lib implementation

aa03ea4

Add websocket lib tests

ad30e1f

Fix 'pong'

4386461

Update server constructor to be compatible with ws

d9937ad

Refactor close frames

06be461

Fix code style

8ef1181

Refactor to use EventEmitter

e422ba4

Decompose constructor

83dccd1

Remove unnecessary conversion to string

67deceb

MarhiievHE force-pushed the custom_websocket branch from c577f9d to 67deceb Compare February 24, 2026 14:52

Remove unnecessary return statement

bc5aa50

tshemsedinov reviewed Mar 2, 2026

View reviewed changes

This comment was marked as off-topic.

Sign in to view

	this.wsServer = new WebsocketServer(this.httpServer);
	this.wsServer = new WebsocketServer({ server: this.httpServer });

	const { Result } = require('./utils/result');
	const { Result } = require('./utils/result.js');

	return new this(true, OPCODES.CLOSE, false, payload, null);
	return new Frame(true, OPCODES.CLOSE, false, payload, null);

Uh oh!

Conversation

MarhiievHE commented Aug 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants