From f191970b8cbb4e234ede015fce8a4258a4530633 Mon Sep 17 00:00:00 2001 From: b-ma Date: Thu, 23 May 2024 16:02:21 +0200 Subject: [PATCH] fix: remove disconnection trigger when pong is not received in time, just warn instead --- src/client/Socket.js | 6 +++-- src/server/Plugin.js | 28 +++++++++++++++------- src/server/Server.js | 6 +++-- src/server/Socket.js | 11 ++++++--- src/server/Sockets.js | 28 ++++++++++++---------- src/server/audit-network-latency.worker.js | 7 +++--- 6 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/client/Socket.js b/src/client/Socket.js index 37b196fe..a462e5e3 100644 --- a/src/client/Socket.js +++ b/src/client/Socket.js @@ -210,8 +210,10 @@ class Socket { * JSON compatible data types (i.e. string, number, boolean, object, array and null). */ send(channel, ...args) { - const msg = packStringMessage(channel, ...args); - this.#socket.send(msg); + if (this.#socket.readyState === 1) { + const msg = packStringMessage(channel, ...args); + this.#socket.send(msg); + } } /** diff --git a/src/server/Plugin.js b/src/server/Plugin.js index e3ce1d2c..b242de88 100644 --- a/src/server/Plugin.js +++ b/src/server/Plugin.js @@ -33,6 +33,9 @@ import BasePlugin from '../common/BasePlugin.js'; * @inheritdoc */ class Plugin extends BasePlugin { + #server = null; + #clients = new Set(); + /** * @param {server.Server} server - The soundworks server instance. * @param {string} id - User defined id of the plugin as defined in @@ -40,16 +43,25 @@ class Plugin extends BasePlugin { */ constructor(server, id) { super(id); + this.#server = server; + } - /** - * Instance of soundworks server. - * @type {server.Server} - * @see {@link server.Server} - */ - this.server = server; + /** + * Instance of soundworks server. + * @type {server.Server} + * @see {@link server.Server} + */ + get server() { + return this.#server; + } - /** @private */ - this.clients = new Set(); + /** + * Set of the clients registered in the plugin. + * @type {Set} + * @see {@link server.Client} + */ + get clients() { + return this.#clients; } /** diff --git a/src/server/Server.js b/src/server/Server.js index fd80dd85..7d394e42 100644 --- a/src/server/Server.js +++ b/src/server/Server.js @@ -24,6 +24,7 @@ import ContextManager from './ContextManager.js'; import PluginManager from './PluginManager.js'; import StateManager from './StateManager.js'; import { + kSocketClientId, kSocketTerminate, } from './Socket.js'; import Sockets from './Sockets.js'; @@ -839,6 +840,7 @@ Invalid certificate files, please check your: */ _onSocketConnection(role, socket, connectionToken) { const client = new Client(role, socket); + socket[kSocketClientId] = client.id; const roles = Object.keys(this.config.app.clients); // this has been validated @@ -912,8 +914,8 @@ WARNING Version discrepancies between server and "${role}" client: + server: ${this.version} | client: ${version} -This might lead to unexpected behavior, you should consider to update your -dependancies on both your server and clients. +This might lead to unexpected behavior, you should consider to re-install your +dependencies on both your server and clients. !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!`); } diff --git a/src/server/Socket.js b/src/server/Socket.js index 73a69f18..2c324b32 100644 --- a/src/server/Socket.js +++ b/src/server/Socket.js @@ -14,6 +14,7 @@ import { kSocketsRemoveFromAllRooms, } from './Sockets.js'; +export const kSocketClientId = Symbol('soundworks:socket-client-id'); export const kSocketTerminate = Symbol('soundworks:socket-terminate'); /** @@ -87,9 +88,13 @@ class Socket { this.#heartbeatId = setInterval(() => { if (isAlive === false) { - // emit a 'close' event to go trough all the disconnection pipeline + // Emit a 'close' event to go trough all the disconnection pipeline + // // @note - this seems to create false positive disconnections when - this.#dispatchEvent('close'); + // client is busy, e.g. when loading large sound files so let's just warn + // until we gather more feedback + // this.#dispatchEvent('close'); + console.warn(`[Socket] client (id: ${this[kSocketClientId]}) did not respond to ping message in time, interval: ${PING_INTERVAL}`); return; } @@ -198,7 +203,7 @@ class Socket { if (this.#socket.readyState === 1) { this.#socket.send(msg, (err) => { if (err) { - console.error('error sending msg:', channel, args, err.message); + console.error('[Socket] error sending msg:', channel, args, err.message); } }); } diff --git a/src/server/Sockets.js b/src/server/Sockets.js index cdec7de8..498bd8c3 100644 --- a/src/server/Sockets.js +++ b/src/server/Sockets.js @@ -1,8 +1,8 @@ +import querystring from 'node:querystring'; import { Worker, } from 'node:worker_threads'; -import querystring from 'querystring'; import { default as WebSocket, WebSocketServer, @@ -11,8 +11,9 @@ import { import Socket, { kSocketTerminate, } from './Socket.js'; -import networkLatencyWorker from './audit-network-latency.worker.js'; +// @note - fs.readFileSync creates some cwd() issues... +import networkLatencyWorker from './audit-network-latency.worker.js'; export const kSocketsRemoveFromAllRooms = Symbol('soundworks:sockets-remove-from-all-rooms'); export const kSocketsLatencyStatsWorker = Symbol('soundworks:sockets-latency-stats-worker'); @@ -77,8 +78,7 @@ class Sockets { }); this.#wsServer.on('connection', (ws, req) => { - const queryString = querystring.decode(req.url.split('?')[1]); - const { role, token } = queryString; + const { role, token } = querystring.parse(req.url.split('?')[1]); const socket = new Socket(ws, this); socket.addToRoom('*'); @@ -89,8 +89,7 @@ class Sockets { // Prevent socket with protected role to connect is token is invalid server.httpServer.on('upgrade', async (req, socket, head) => { - const queryString = querystring.decode(req.url.split('?')[1]); - const { role, token } = queryString; + const { role, token } = querystring.parse(req.url.split('?')[1]); if (server.isProtected(role)) { // we don't have any IP in the upgrade request object, @@ -107,7 +106,7 @@ class Sockets { } /** - * Terminate all existing sockets + * Terminate all existing sockets. * @private */ terminate() { @@ -118,6 +117,15 @@ class Sockets { sockets.forEach(socket => socket[kSocketTerminate]()); } + /** + * Remove given socket from all rooms. + */ + [kSocketsRemoveFromAllRooms](socket) { + for (let [_, room] of this.#rooms) { + room.delete(socket); + } + } + /** * Add a socket to a room. * @@ -150,12 +158,6 @@ class Sockets { } } - [kSocketsRemoveFromAllRooms](socket) { - for (let [_key, room] of this.#rooms) { - room.delete(socket); - } - } - /** * Send a message to all clients os given room(s). If no room is specified, * the message is sent to all clients. diff --git a/src/server/audit-network-latency.worker.js b/src/server/audit-network-latency.worker.js index e5e76046..ba5924f9 100644 --- a/src/server/audit-network-latency.worker.js +++ b/src/server/audit-network-latency.worker.js @@ -1,5 +1,6 @@ -// NOTICE: we use this syntax so that the server can be bundled to cjs -// with esbuild, so we can ship a cjs server bundle into Max. +// @note - We use this common js syntax so that the server can be bundled to cjs +// which allows to build a server that can be accepted by Max `node.script` +// // Should move back to regular esm module once Max has fixed its loader export default ` const { parentPort } = require('node:worker_threads'); @@ -11,6 +12,7 @@ let averageLatencyPeriod = 2; let intervalId = null; let meanLatency = 0; +// workaround that sc-utils is pure emascript module let getTime; let inited = new Promise(async (resolve) => { module = await import('@ircam/sc-utils'); @@ -25,7 +27,6 @@ parentPort.on('message', async msg => { averageLatencyPeriod = msg.value.averageLatencyPeriod; await inited; - // launch compute in its own loop so that the number of computation is // decoupled from the number of connected clients clearInterval(intervalId);