openvibers/openrestream Commits
Plan T0 — await/guard audit for OpenRe.Stream
Changes
@@@ -57,9 +57,16 @@ function createKeyStore({ urls = [], pem = null, fetchImpl = globalThis.fetch, lasync function start() {if (pem) return Promise.resolve(key);const attempt = async () => {-- const k = await fetchOnce();-- if (!k && !key) { retryTimer = setTimeout(attempt, 30000); retryTimer.unref?.(); }-- return k;++ try {++ const k = await fetchOnce();++ if (!k && !key) { retryTimer = setTimeout(() => { attempt().catch(() => {}); }, 30000); retryTimer.unref?.(); }++ return k;++ } catch (err) {++ // The retry timer never awaits the attempt it starts: an unhandled rejection would++ // end the API process (Node 22) over one failed Network key fetch.++ log.warn(`[auth] key retry failed: ${err.message}`);++ return null;++ }};refreshTimer = setInterval(() => { fetchOnce().catch(() => {}); }, 6 * 60 * 60 * 1000);refreshTimer.unref?.();@@@ -4,11 +4,14 @@// and on an RTMP worker that would drop every publisher it carries, so each of these is a regression// test for one dropped promise — remove the `.catch` and the test sees the rejection.const assert = require('assert');++const http = require('http');const path = require('path');++const WebSocket = require('ws');const { runtime, testEnv, tmpDir, child, suite, ROOT } = require('./helpers');const { createWorkerRuntime } = require('../workers/runtime');const { admissionChain } = require('../workers/rtmp-ingest');const { pruneOutbox } = require('../workers/coordinator');++const { createSignaling } = require('../workers/webrtc/signaling');const t = suite('worker-async');@@@ -81,4 +84,34 @@ t('the restream worker boot awaits start(): a register failure is reported, notassert.match(p.output, /\[restream-worker\] failed to start: Error: register boom/);});++t('the signaling broadcaster close handles a failing endSession(): logged, not unhandled', async () => {++ // A browser broadcaster's ws 'close' ends its session through an async call the handler never++ // awaits: without a handler a rejected endSession ends the whole WebRTC worker (Node 22) and++ // drops every other session on it.++ const errors = [];++ const log = { log() {}, warn() {}, error: (m) => errors.push(String(m)) };++ const closed = [];++ const signaling = createSignaling({++ sfu: { closePeer: () => {}, hasRouter: () => true },++ iceServers: [],++ log,++ admit: async () => ({ sessionId: 'ses_000000000000000000000000', peerId: 'bc-1' }),++ endSession: async (sessionId, reason) => { closed.push([sessionId, reason]); throw new Error('end boom'); },++ });++ const server = http.createServer((_req, res) => res.end());++ server.on('upgrade', (req, socket, head) => { signaling.handleUpgrade(req, socket, head).catch(() => {}); });++ await new Promise((r) => server.listen(0, '127.0.0.1', r));++ const ws = new WebSocket(`ws://127.0.0.1:${server.address().port}/b/ork_somekey`);++ const unhandled = await withUnhandled(async () => {++ await new Promise((resolve, reject) => { ws.on('open', resolve); ws.on('error', reject); });++ await new Promise((resolve) => { ws.on('close', resolve); ws.close(); });++ await new Promise((r) => setTimeout(r, 30));++ });++ await new Promise((r) => server.close(r));++ signaling.closeAll();++ assert.deepStrictEqual(unhandled, [], 'a rejection of endSession() must be caught');++ assert.ok(closed.some(([, reason]) => reason === 'broadcaster_disconnected'), `the session end was attempted: ${JSON.stringify(closed)}`);++ assert.ok(errors.some((m) => /broadcaster close could not end ses_.*: Error: end boom/.test(m)), `the failure is logged: ${JSON.stringify(errors)}`);++});++t.run();@@@ -136,7 +136,14 @@ function createSignaling({ sfu, iceServers, admit, endSession, log = console })wss.handleUpgrade(req, socket, head, (ws) => {const peerId = adm.peerId || `bc-${adm.sessionId}`;clients.set(ws, { role: 'broadcaster', sessionId: adm.sessionId, peerId });-- ws.on('close', () => { clients.delete(ws); sfu.closePeer(adm.sessionId, peerId); endSession?.(adm.sessionId, 'broadcaster_disconnected'); });++ // endSession is async: the ws 'close' handler never awaits it, so a rejection would be++ // unhandled and Node would end the worker, dropping every other session it carries.++ ws.on('close', () => {++ clients.delete(ws);++ sfu.closePeer(adm.sessionId, peerId);++ const ending = endSession ? endSession(adm.sessionId, 'broadcaster_disconnected') : null;++ if (ending) Promise.resolve(ending).catch((err) => log.error(`[webrtc] broadcaster close could not end ${adm.sessionId}: ${err && (err.stack || err.message) || err}`));++ });ws.on('error', () => {});safeSend(ws, { type: 'welcome', peerId, role: 'broadcaster', sessionId: adm.sessionId, iceServers: ice() }, log);handleBroadcaster(ws, adm);
@@ -57,9 +57,16 @@ function createKeyStore({ urls = [], pem = null, fetchImpl = globalThis.fetch, l
@@ -57,9 +57,16 @@ function createKeyStore({ urls = [], pem = null, fetchImpl = globalThis.fetch, l
async function start() {
async function start() {
if (pem) return Promise.resolve(key);
if (pem) return Promise.resolve(key);
const attempt = async () => {
const attempt = async () => {
- const k = await fetchOnce();
- if (!k && !key) { retryTimer = setTimeout(attempt, 30000); retryTimer.unref?.(); }
- return k;
+ try {
+ const k = await fetchOnce();
+ if (!k && !key) { retryTimer = setTimeout(() => { attempt().catch(() => {}); }, 30000); retryTimer.unref?.(); }
+ return k;
+ } catch (err) {
+ // The retry timer never awaits the attempt it starts: an unhandled rejection would
+ // end the API process (Node 22) over one failed Network key fetch.
+ log.warn(`[auth] key retry failed: ${err.message}`);
+ return null;
+ }
};
};
refreshTimer = setInterval(() => { fetchOnce().catch(() => {}); }, 6 * 60 * 60 * 1000);
refreshTimer = setInterval(() => { fetchOnce().catch(() => {}); }, 6 * 60 * 60 * 1000);
refreshTimer.unref?.();
refreshTimer.unref?.();
@@ -4,11 +4,14 @@
@@ -4,11 +4,14 @@
// and on an RTMP worker that would drop every publisher it carries, so each of these is a regression
// and on an RTMP worker that would drop every publisher it carries, so each of these is a regression
// test for one dropped promise — remove the `.catch` and the test sees the rejection.
// test for one dropped promise — remove the `.catch` and the test sees the rejection.
const assert = require('assert');
const assert = require('assert');
+const http = require('http');
const path = require('path');
const path = require('path');
+const WebSocket = require('ws');
const { runtime, testEnv, tmpDir, child, suite, ROOT } = require('./helpers');
const { runtime, testEnv, tmpDir, child, suite, ROOT } = require('./helpers');
const { createWorkerRuntime } = require('../workers/runtime');
const { createWorkerRuntime } = require('../workers/runtime');
const { admissionChain } = require('../workers/rtmp-ingest');
const { admissionChain } = require('../workers/rtmp-ingest');
const { pruneOutbox } = require('../workers/coordinator');
const { pruneOutbox } = require('../workers/coordinator');
+const { createSignaling } = require('../workers/webrtc/signaling');
const t = suite('worker-async');
const t = suite('worker-async');
@@ -81,4 +84,34 @@ t('the restream worker boot awaits start(): a register failure is reported, not
@@ -81,4 +84,34 @@ t('the restream worker boot awaits start(): a register failure is reported, not
assert.match(p.output, /\[restream-worker\] failed to start: Error: register boom/);
assert.match(p.output, /\[restream-worker\] failed to start: Error: register boom/);
});
});
+t('the signaling broadcaster close handles a failing endSession(): logged, not unhandled', async () => {
+ // A browser broadcaster's ws 'close' ends its session through an async call the handler never
+ // awaits: without a handler a rejected endSession ends the whole WebRTC worker (Node 22) and
+ // drops every other session on it.
+ const errors = [];
+ const log = { log() {}, warn() {}, error: (m) => errors.push(String(m)) };
+ const closed = [];
+ const signaling = createSignaling({
+ sfu: { closePeer: () => {}, hasRouter: () => true },
+ iceServers: [],
+ log,
+ admit: async () => ({ sessionId: 'ses_000000000000000000000000', peerId: 'bc-1' }),
+ endSession: async (sessionId, reason) => { closed.push([sessionId, reason]); throw new Error('end boom'); },
+ });
+ const server = http.createServer((_req, res) => res.end());
+ server.on('upgrade', (req, socket, head) => { signaling.handleUpgrade(req, socket, head).catch(() => {}); });
+ await new Promise((r) => server.listen(0, '127.0.0.1', r));
+ const ws = new WebSocket(`ws://127.0.0.1:${server.address().port}/b/ork_somekey`);
+ const unhandled = await withUnhandled(async () => {
+ await new Promise((resolve, reject) => { ws.on('open', resolve); ws.on('error', reject); });
+ await new Promise((resolve) => { ws.on('close', resolve); ws.close(); });
+ await new Promise((r) => setTimeout(r, 30));
+ });
+ await new Promise((r) => server.close(r));
+ signaling.closeAll();
+ assert.deepStrictEqual(unhandled, [], 'a rejection of endSession() must be caught');
+ assert.ok(closed.some(([, reason]) => reason === 'broadcaster_disconnected'), `the session end was attempted: ${JSON.stringify(closed)}`);
+ assert.ok(errors.some((m) => /broadcaster close could not end ses_.*: Error: end boom/.test(m)), `the failure is logged: ${JSON.stringify(errors)}`);
+});
+
t.run();
t.run();
@@ -136,7 +136,14 @@ function createSignaling({ sfu, iceServers, admit, endSession, log = console })
@@ -136,7 +136,14 @@ function createSignaling({ sfu, iceServers, admit, endSession, log = console })
wss.handleUpgrade(req, socket, head, (ws) => {
wss.handleUpgrade(req, socket, head, (ws) => {
const peerId = adm.peerId || `bc-${adm.sessionId}`;
const peerId = adm.peerId || `bc-${adm.sessionId}`;
clients.set(ws, { role: 'broadcaster', sessionId: adm.sessionId, peerId });
clients.set(ws, { role: 'broadcaster', sessionId: adm.sessionId, peerId });
- ws.on('close', () => { clients.delete(ws); sfu.closePeer(adm.sessionId, peerId); endSession?.(adm.sessionId, 'broadcaster_disconnected'); });
+ // endSession is async: the ws 'close' handler never awaits it, so a rejection would be
+ // unhandled and Node would end the worker, dropping every other session it carries.
+ ws.on('close', () => {
+ clients.delete(ws);
+ sfu.closePeer(adm.sessionId, peerId);
+ const ending = endSession ? endSession(adm.sessionId, 'broadcaster_disconnected') : null;
+ if (ending) Promise.resolve(ending).catch((err) => log.error(`[webrtc] broadcaster close could not end ${adm.sessionId}: ${err && (err.stack || err.message) || err}`));
+ });
ws.on('error', () => {});
ws.on('error', () => {});
safeSend(ws, { type: 'welcome', peerId, role: 'broadcaster', sessionId: adm.sessionId, iceServers: ice() }, log);
safeSend(ws, { type: 'welcome', peerId, role: 'broadcaster', sessionId: adm.sessionId, iceServers: ice() }, log);
handleBroadcaster(ws, adm);
handleBroadcaster(ws, adm);