From 13dd5686e61efc8459af94cc10c7e5395753ad38 Mon Sep 17 00:00:00 2001 From: mertushka <34413473+mertushka@users.noreply.github.com> Date: Wed, 11 Jun 2025 17:36:02 +0300 Subject: [PATCH 01/14] fix(polyfill)(RTCDataChannel): fix readyState race condition and improve typings --- src/polyfill/RTCDataChannel.ts | 96 +++++++++++++++++++--------------- 1 file changed, 53 insertions(+), 43 deletions(-) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index 4300361..4e459fe 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -16,8 +16,6 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT #label: string; #protocol: string; - #closeRequested = false; - // events onbufferedamountlow: globalThis.RTCDataChannel['onbufferedamountlow'] = null; onclose: globalThis.RTCDataChannel['onclose'] = null; @@ -49,14 +47,18 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT this.#dataChannel.onClosed(() => { // Simulate closing event - if (!this.#closeRequested) { + if (this.#readyState === 'closed') return; + + if (this.#readyState !== 'closing') { this.#readyState = 'closing'; this.dispatchEvent(new Event('closing')); } setImmediate(() => { - this.#readyState = 'closed'; - this.dispatchEvent(new Event('close')); + if (this.#readyState !== 'closed') { + this.#readyState = 'closed'; + this.dispatchEvent(new Event('close')); + } }); }); @@ -79,32 +81,21 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT this.#dataChannel.onMessage((data) => { if (ArrayBuffer.isView(data)) { - if (this.binaryType == 'arraybuffer') data = data.buffer; - else data = Buffer.from(data.buffer); + data = + this.binaryType === 'arraybuffer' + ? (data.buffer as ArrayBuffer) + : Buffer.from(data.buffer); } this.dispatchEvent(new MessageEvent('message', { data })); }); // forward events to properties - this.addEventListener('message', (e) => { - if (this.onmessage) this.onmessage(e as MessageEvent); - }); - this.addEventListener('bufferedamountlow', (e) => { - if (this.onbufferedamountlow) this.onbufferedamountlow(e); - }); - this.addEventListener('error', (e) => { - if (this.onerror) this.onerror(e as RTCErrorEvent); - }); - this.addEventListener('close', (e) => { - if (this.onclose) this.onclose(e); - }); - this.addEventListener('closing', (e) => { - if (this.onclosing) this.onclosing(e); - }); - this.addEventListener('open', (e) => { - if (this.onopen) this.onopen(e); - }); + this.addEventListener('open', (e) => this.onopen?.(e)); + this.addEventListener('message', (e) => this.onmessage?.(e as MessageEvent)); + this.addEventListener('error', (e) => this.onerror?.(e as RTCErrorEvent)); + this.addEventListener('close', (e) => this.onclose?.(e)); + this.addEventListener('bufferedamountlow', (e) => this.onbufferedamountlow?.(e)); } set binaryType(type) { @@ -168,37 +159,56 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT return this.#readyState; } - send(data): void { - if (this.#readyState !== 'open') { + send(data: string | Blob | ArrayBuffer | ArrayBufferView): void { + if (this.#readyState !== 'open' || !this.#dataChannel.isOpen()) { throw new exceptions.InvalidStateError( "Failed to execute 'send' on 'RTCDataChannel': RTCDataChannel.readyState is not 'open'", ); } - // Needs network error, type error implemented - if (typeof data === 'string') { - this.#dataChannel.sendMessage(data); - } else if (data instanceof Blob) { - data.arrayBuffer().then((ab) => { + try { + // Needs network error, type error implemented + if (typeof data === 'string') { + this.#dataChannel.sendMessage(data); + } else if (data instanceof Blob) { + data.arrayBuffer().then((ab) => { + if (process?.versions?.bun) { + this.#dataChannel.sendMessageBinary(Buffer.from(ab)); + } else { + this.#dataChannel.sendMessageBinary(new Uint8Array(ab)); + } + }); + } else if (data instanceof Uint8Array) { + this.#dataChannel.sendMessageBinary(data); + } else { if (process?.versions?.bun) { - this.#dataChannel.sendMessageBinary(Buffer.from(ab)); + this.#dataChannel.sendMessageBinary(Buffer.from(data as ArrayBuffer)); } else { - this.#dataChannel.sendMessageBinary(new Uint8Array(ab)); + this.#dataChannel.sendMessageBinary(new Uint8Array(data as ArrayBuffer)); } - }); - } else if (data instanceof Uint8Array) { - this.#dataChannel.sendMessageBinary(data); - } else { - if (process?.versions?.bun) { - this.#dataChannel.sendMessageBinary(Buffer.from(data)); - } else { - this.#dataChannel.sendMessageBinary(new Uint8Array(data)); } + } catch (error: any) { + if (error?.message?.includes('DataChannel is closed')) { + this.#readyState = 'closed'; + this.dispatchEvent(new Event('close')); + throw new exceptions.InvalidStateError( + "Failed to execute 'send' on 'RTCDataChannel': RTCDataChannel.send failed because the DataChannel is closed", + ); + } + throw error; } } close(): void { - this.#closeRequested = true; + if (this.#readyState === 'closing' || this.#readyState === 'closed') { + throw new exceptions.InvalidStateError( + "Failed to execute 'close' on 'RTCDataChannel': The DataChannel is already closing or has been closed", + ); + } + + this.#readyState = 'closing'; + this.dispatchEvent(new Event('closing')); + setImmediate(() => { this.#dataChannel.close(); }); From 6e22bd110dbe6d4f6f8ea1b4922990fc2990b1ae Mon Sep 17 00:00:00 2001 From: mertushka Date: Sat, 14 Jun 2025 02:06:33 +0300 Subject: [PATCH 02/14] revert: unstandard closing event --- src/polyfill/RTCDataChannel.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index 4e459fe..07871d7 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -95,6 +95,7 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT this.addEventListener('message', (e) => this.onmessage?.(e as MessageEvent)); this.addEventListener('error', (e) => this.onerror?.(e as RTCErrorEvent)); this.addEventListener('close', (e) => this.onclose?.(e)); + this.addEventListener('closing', (e) => this.onclosing?.(e)); this.addEventListener('bufferedamountlow', (e) => this.onbufferedamountlow?.(e)); } From 37e52838192876722e3943bb3d2f876b0a1f19c5 Mon Sep 17 00:00:00 2001 From: mertushka Date: Sat, 14 Jun 2025 19:37:59 +0300 Subject: [PATCH 03/14] chore: remove unnecessary checks --- src/polyfill/RTCDataChannel.ts | 43 +++++++++++++--------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index 07871d7..d8a403c 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -161,42 +161,31 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT } send(data: string | Blob | ArrayBuffer | ArrayBufferView): void { - if (this.#readyState !== 'open' || !this.#dataChannel.isOpen()) { + if (this.#readyState !== 'open') { throw new exceptions.InvalidStateError( "Failed to execute 'send' on 'RTCDataChannel': RTCDataChannel.readyState is not 'open'", ); } - try { - // Needs network error, type error implemented - if (typeof data === 'string') { - this.#dataChannel.sendMessage(data); - } else if (data instanceof Blob) { - data.arrayBuffer().then((ab) => { - if (process?.versions?.bun) { - this.#dataChannel.sendMessageBinary(Buffer.from(ab)); - } else { - this.#dataChannel.sendMessageBinary(new Uint8Array(ab)); - } - }); - } else if (data instanceof Uint8Array) { - this.#dataChannel.sendMessageBinary(data); - } else { + // Needs network error, type error implemented + if (typeof data === 'string') { + this.#dataChannel.sendMessage(data); + } else if (data instanceof Blob) { + data.arrayBuffer().then((ab) => { if (process?.versions?.bun) { - this.#dataChannel.sendMessageBinary(Buffer.from(data as ArrayBuffer)); + this.#dataChannel.sendMessageBinary(Buffer.from(ab)); } else { - this.#dataChannel.sendMessageBinary(new Uint8Array(data as ArrayBuffer)); + this.#dataChannel.sendMessageBinary(new Uint8Array(ab)); } + }); + } else if (data instanceof Uint8Array) { + this.#dataChannel.sendMessageBinary(data); + } else { + if (process?.versions?.bun) { + this.#dataChannel.sendMessageBinary(Buffer.from(data)); + } else { + this.#dataChannel.sendMessageBinary(new Uint8Array(data)); } - } catch (error: any) { - if (error?.message?.includes('DataChannel is closed')) { - this.#readyState = 'closed'; - this.dispatchEvent(new Event('close')); - throw new exceptions.InvalidStateError( - "Failed to execute 'send' on 'RTCDataChannel': RTCDataChannel.send failed because the DataChannel is closed", - ); - } - throw error; } } From c62b15c9ab2a1ff301d41081e5530b70ea68726e Mon Sep 17 00:00:00 2001 From: mertushka Date: Sat, 14 Jun 2025 19:41:19 +0300 Subject: [PATCH 04/14] fix: close existing datachannels before closing peerconnection --- src/polyfill/RTCPeerConnection.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/polyfill/RTCPeerConnection.ts b/src/polyfill/RTCPeerConnection.ts index d71ade6..6aa8fb0 100644 --- a/src/polyfill/RTCPeerConnection.ts +++ b/src/polyfill/RTCPeerConnection.ts @@ -354,6 +354,13 @@ export default class RTCPeerConnection extends EventTarget implements globalThis } close(): void { + for (const dc of this.#dataChannels) { + if (dc.readyState !== 'closed') { + dc.close(); + } + } + this.#dataChannels.clear(); + this.#peerConnection.close(); } From 05381db77f77c73f48ed9959aad1ddd90b5ca5cf Mon Sep 17 00:00:00 2001 From: mertushka Date: Tue, 17 Jun 2025 17:26:42 +0300 Subject: [PATCH 05/14] fix(polyfill): returning instead of throwing on closing/closed state --- src/polyfill/RTCDataChannel.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index d8a403c..311bd57 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -190,11 +190,7 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT } close(): void { - if (this.#readyState === 'closing' || this.#readyState === 'closed') { - throw new exceptions.InvalidStateError( - "Failed to execute 'close' on 'RTCDataChannel': The DataChannel is already closing or has been closed", - ); - } + if (this.#readyState === 'closing' || this.#readyState === 'closed') return; this.#readyState = 'closing'; this.dispatchEvent(new Event('closing')); From 5100ad99331402e220380e21c78f1f2fbc7608c2 Mon Sep 17 00:00:00 2001 From: mertushka Date: Tue, 17 Jun 2025 17:28:52 +0300 Subject: [PATCH 06/14] fix(types): add explicit ArrayBuffer cast for sendMessageBinary calls --- src/polyfill/RTCDataChannel.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index 311bd57..ba7beaa 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -182,9 +182,9 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT this.#dataChannel.sendMessageBinary(data); } else { if (process?.versions?.bun) { - this.#dataChannel.sendMessageBinary(Buffer.from(data)); + this.#dataChannel.sendMessageBinary(Buffer.from(data as ArrayBuffer)); } else { - this.#dataChannel.sendMessageBinary(new Uint8Array(data)); + this.#dataChannel.sendMessageBinary(new Uint8Array(data as ArrayBuffer)); } } } From 0ec58d203d72a3c7eb28368e6fe43858e22958e4 Mon Sep 17 00:00:00 2001 From: mertushka Date: Thu, 19 Jun 2025 04:36:47 +0300 Subject: [PATCH 07/14] chore: remove unneeded closing check --- src/polyfill/RTCDataChannel.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index ba7beaa..c73186e 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -49,11 +49,6 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT // Simulate closing event if (this.#readyState === 'closed') return; - if (this.#readyState !== 'closing') { - this.#readyState = 'closing'; - this.dispatchEvent(new Event('closing')); - } - setImmediate(() => { if (this.#readyState !== 'closed') { this.#readyState = 'closed'; From ddce92f8459070e0902131061d7dea5d9479bb40 Mon Sep 17 00:00:00 2001 From: mertushka Date: Thu, 19 Jun 2025 04:38:41 +0300 Subject: [PATCH 08/14] fix: also check for closing data channels --- src/polyfill/RTCPeerConnection.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/polyfill/RTCPeerConnection.ts b/src/polyfill/RTCPeerConnection.ts index 6aa8fb0..0c4f331 100644 --- a/src/polyfill/RTCPeerConnection.ts +++ b/src/polyfill/RTCPeerConnection.ts @@ -355,11 +355,10 @@ export default class RTCPeerConnection extends EventTarget implements globalThis close(): void { for (const dc of this.#dataChannels) { - if (dc.readyState !== 'closed') { - dc.close(); + if (dc.readyState !== 'closed' && dc.readyState !== 'closing') { + dc.close(); // Note: This is currently incorrect per the spec, we should set readyState to 'closed' directly without invoking the 'closing' procedure: https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close } } - this.#dataChannels.clear(); this.#peerConnection.close(); } From b291be237b8eb48099a1aacba4731c002251f629 Mon Sep 17 00:00:00 2001 From: mertushka Date: Thu, 19 Jun 2025 04:41:32 +0300 Subject: [PATCH 09/14] fix: prevent another race condition window --- src/polyfill/RTCDataChannel.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index c73186e..c09ba06 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -191,7 +191,9 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT this.dispatchEvent(new Event('closing')); setImmediate(() => { - this.#dataChannel.close(); + if (this.#readyState !== 'closed') { + this.#dataChannel.close(); + } }); } } From 2168c78a282d123df8f0482f6db5f3b93f975ec8 Mon Sep 17 00:00:00 2001 From: mertushka Date: Thu, 19 Jun 2025 04:46:08 +0300 Subject: [PATCH 10/14] fix: dataChannel onClosed race condition --- src/polyfill/RTCDataChannel.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index c09ba06..e4dcfdd 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -46,15 +46,10 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT }); this.#dataChannel.onClosed(() => { - // Simulate closing event if (this.#readyState === 'closed') return; - setImmediate(() => { - if (this.#readyState !== 'closed') { - this.#readyState = 'closed'; - this.dispatchEvent(new Event('close')); - } - }); + this.#readyState = 'closed'; + this.dispatchEvent(new Event('close')); }); this.#dataChannel.onError((msg) => { From f3c71ec44e50122b94b1e26c735b363f801d8cf1 Mon Sep 17 00:00:00 2001 From: mertushka Date: Thu, 19 Jun 2025 19:47:49 +0300 Subject: [PATCH 11/14] fix: match the spec behaviour --- src/polyfill/RTCDataChannel.ts | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index e4dcfdd..5a5180c 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -16,6 +16,8 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT #label: string; #protocol: string; + #closeRequested = false; + // events onbufferedamountlow: globalThis.RTCDataChannel['onbufferedamountlow'] = null; onclose: globalThis.RTCDataChannel['onclose'] = null; @@ -48,8 +50,21 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT this.#dataChannel.onClosed(() => { if (this.#readyState === 'closed') return; - this.#readyState = 'closed'; - this.dispatchEvent(new Event('close')); + if (!this.#closeRequested) { + // if close was not requested, we emit 'closing' before 'close' to match the spec behavior + this.#readyState = 'closing'; + this.dispatchEvent(new Event('closing')); + } else { + // close was requested, so we skip 'closing' event to match the spec behavior + this.#readyState = 'closing'; + } + + setImmediate(() => { + if (this.#readyState !== 'closed') { + this.#readyState = 'closed'; + this.dispatchEvent(new Event('close')); + } + }); }); this.#dataChannel.onError((msg) => { @@ -182,13 +197,11 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT close(): void { if (this.#readyState === 'closing' || this.#readyState === 'closed') return; + this.#closeRequested = true; this.#readyState = 'closing'; - this.dispatchEvent(new Event('closing')); setImmediate(() => { - if (this.#readyState !== 'closed') { - this.#dataChannel.close(); - } + this.#dataChannel.close(); }); } } From dc6a887020b4b7becdd3bb8d5e5e78aaa54d7345 Mon Sep 17 00:00:00 2001 From: mertushka Date: Thu, 19 Jun 2025 20:21:46 +0300 Subject: [PATCH 12/14] chore: remove comment --- src/polyfill/RTCPeerConnection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/polyfill/RTCPeerConnection.ts b/src/polyfill/RTCPeerConnection.ts index 0c4f331..9c07992 100644 --- a/src/polyfill/RTCPeerConnection.ts +++ b/src/polyfill/RTCPeerConnection.ts @@ -356,7 +356,7 @@ export default class RTCPeerConnection extends EventTarget implements globalThis close(): void { for (const dc of this.#dataChannels) { if (dc.readyState !== 'closed' && dc.readyState !== 'closing') { - dc.close(); // Note: This is currently incorrect per the spec, we should set readyState to 'closed' directly without invoking the 'closing' procedure: https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close + dc.close(); } } From 0081313082036f1b9b7c8c94aec3cccbd302e5aa Mon Sep 17 00:00:00 2001 From: mertushka Date: Sat, 21 Jun 2025 22:38:27 +0300 Subject: [PATCH 13/14] fix(RTCPeerConnection): implement spec-compliant abrupt data channel close --- src/polyfill/RTCPeerConnection.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/polyfill/RTCPeerConnection.ts b/src/polyfill/RTCPeerConnection.ts index 9c07992..d2fdb66 100644 --- a/src/polyfill/RTCPeerConnection.ts +++ b/src/polyfill/RTCPeerConnection.ts @@ -356,7 +356,12 @@ export default class RTCPeerConnection extends EventTarget implements globalThis close(): void { for (const dc of this.#dataChannels) { if (dc.readyState !== 'closed' && dc.readyState !== 'closing') { - dc.close(); + Object.defineProperty(dc, 'readyState', { + value: 'closed', + writable: false, + enumerable: true, + configurable: true, + }); } } From 9cbbf63ca3446c48ab183b00448beca5d6a29525 Mon Sep 17 00:00:00 2001 From: mertushka Date: Sat, 21 Jun 2025 22:40:14 +0300 Subject: [PATCH 14/14] fix(RTCDataChannel): improve close event handling logic --- src/polyfill/RTCDataChannel.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/polyfill/RTCDataChannel.ts b/src/polyfill/RTCDataChannel.ts index 5a5180c..b4d2ec3 100644 --- a/src/polyfill/RTCDataChannel.ts +++ b/src/polyfill/RTCDataChannel.ts @@ -54,9 +54,6 @@ export default class RTCDataChannel extends EventTarget implements globalThis.RT // if close was not requested, we emit 'closing' before 'close' to match the spec behavior this.#readyState = 'closing'; this.dispatchEvent(new Event('closing')); - } else { - // close was requested, so we skip 'closing' event to match the spec behavior - this.#readyState = 'closing'; } setImmediate(() => {