Replace simple-peer with native WebRTC API by ale18V · Pull Request #1265 · epfml/disco

ale18V · 2026-10-20T13:24:10Z

Description

Resolves #1246.

Root Cause

During decentralized training between Chrome and Firefox, Chrome threw:
TypeError: Failed to execute 'send' on 'RTCDataChannel': Trying to send message larger than max-message-size.

SDP Asymmetry: Firefox advertises a=max-message-size:1073741823 (~1 GiB). The previous Peer parsed this remote SDP through a regex and set its send chunk size to ~1 GiB.

However, this did not take into account the forward way max chunk size: Chromium's WebRTC implementation enforces a local transport send cap of 256 KiB (pc.sctp.maxMessageSize). Attempting to send a multi-megabyte chunk (such as GPT weights) in one go caused Chromium's RTCDataChannel send check to throw.

Chunk Framing Limit: The previous chunk ID and chunk total headers were encoded as uint8, which capped transfers at 255 chunks. This is too small for large models as we'd have at most roughly 256^2 KiB ~ 64 MiB.

Changes

simple-peer is unmaintained and relied on legacy event patterns and polyfills. The native WebRTC API provides builtin maxMessageSize which is already the minimum between outward and inward chunk size.
So we just drop simple peer which is outdated and not useful anymore.

- Replace unmaintained simple-peer with standard RTCPeerConnection and RTCDataChannel - Query pc.sctp.maxMessageSize and clamp to 256 KiB cross-browser safety limit to prevent Chrome 'Trying to send message larger than max-message-size' error when peering with Firefox - Widen chunk ID header from uint8 to uint32BE, removing the 255 chunk size limit for large payloads such as GPT weights - Implement bufferedAmountLow event backpressure and candidate queuing - Add unit tests verifying 10MB payload byte-for-byte reassembly and >255 chunk transfer Fixes #1246

Rely directly on W3C standard pc.sctp.maxMessageSize and cross-browser safety default instead of regex matching raw SDP strings.

…ariables - Introduce Packet data class with encode, decode, and factory methods - Store message ID, chunk ID, total chunk count sizes, and header sizes in explicit constants - Restore asChunkID helper function - Replace hardcoded RFC 8841 default with DEFAULT_MAX_MESSAGE_SIZE variable - Add unit tests for Packet serialization and header decoding

Extract framing, encoding, decoding, and packet-level constants into packet.ts with dedicated unit test suite in packet.spec.ts.

Simplify SignalData to use W3C standard RTCSessionDescriptionInit and drop obsolete simple-peer internal transceiverRequest and renegotiate types.

ale18V · 2026-10-20T22:06:31Z

While I was at it, I refactored a bit more the code:

I created a packet class for the custom protocol over WebRTC
Since the drain logic was a one-off that was used only before shutdown in the connection pool, I just integrated it into the disconnect method as an optional gracePeriod time parameter

Please lmk what you think

JulienVig

Thanks for the refactor! The main blocker is that there is no STUN server specified anymore, without which webRTC connections can't work on different networks. Other than that I only have minor implementation points.

Can you update outdated files mentioning simple-peer? I saw FAQ.md, vite.config.ts and README.md still mentioned it
Can you also add this paragraph in DECENTRALIZED.md:

For history tracking: we did a back and forth between [@roamhq/wrtc](https://github.com/WonderInventions/node-webrtc) and [node-datachannel](https://github.com/murat-dogan/node-datachannel) and are currently relying on `@roamhq/wrtc` but `node-datachannel` remains a viable alternative.
We first started with `@roamhq/wrtc` but [faced segfaults](https://github.com/WonderInventions/node-webrtc/issues/7) so we migrated to `node-datachannel` in #652. 
With #1079 we moved back to `@roamhq/wrtc` once the segfault issue had been fixed and for [better compatibility](https://github.com/murat-dogan/node-datachannel#webrtc-polyfills) with the WebRTC wrapper [simple-peer](https://github.com/feross/simple-peer). 
Finally, #1265 is now dropping `simple-peer` because it is unmaintained. We are staying with `@roamhq/wrtc` for now but `node-datachannel`seems to be more actively maintained so it remains a viable future alternative to `@roamhq/wrtc` if needed.

Finally can also you mention the MDN guide on WebRTC in DECENTRALIZED.md for anyone wanting to work on this part of the project?

JulienVig · 2026-10-21T14:18:21Z

-      )
-        this.unacknowledged = this.unacknowledged.delete(data.readUInt16BE());
-    });
+    this.pc = new wrtc.RTCPeerConnection();


simple-peer used to provide default STUN server, make sure to add them here otherwise webrtc will only work on LAN

JulienVig · 2026-10-21T14:20:16Z


-  private chunk(b: Buffer): Seq.Indexed<Buffer> {
+  private get maxChunkSize(): number {
+    return this.pc.sctp?.maxMessageSize || SAFE_MAX_MESSAGE_SIZE;


I think there should be a max chunk size otherwise two firefox would exchange 1GB chunks, bypassing the water mark threshold

The builtin variable maxMessageSize should already be safe (i.e 1GB for firefox <-> firefox and 256KiB for Chrome <-> Firefox)

I meant that we probably want to avoid sending 1GB chunks and break it down in smaller ones no?

JulienVig · 2026-10-21T15:21:49Z

+      if (state === "failed" || state === "closed") {
+        this.emit("close", undefined);


We should probably call destroy on "failed"

JulienVig · 2026-10-21T15:22:13Z

+    this.channel.onerror = (event) => {
+      const message =
+        "error" in event && event.error instanceof Error
+          ? event.error.message
+          : "DataChannel error";
+      this.emit("error", new Error(message));
+    };


same here, we should probably call destroy()

JulienVig · 2026-10-21T15:24:35Z

+    };
+
+    this.channel.onclose = () => {
+      this.emit("close", undefined);


the "close" event can be emitted multiple times, from here, from destroy from onconnectionstatechange, we should first check isDestroyed before emitting and destroy should remove the listeners like onclose and onconnectionstatechange

JulienVig · 2026-10-21T16:16:58Z

+      const payload = Buffer.alloc(data.length - Packet.FIRST_HEADER_SIZE);
+      data.copy(payload, 0, Packet.FIRST_HEADER_SIZE);


Suggested change

const payload = Buffer.alloc(data.length - Packet.FIRST_HEADER_SIZE);

data.copy(payload, 0, Packet.FIRST_HEADER_SIZE);

const payload = data.subarray(Packet.FIRST_HEADER_SIZE);

Avoid writing each byte twice

JulienVig · 2026-10-21T16:17:17Z

+    const payload = Buffer.alloc(data.length - Packet.HEADER_SIZE);
+    data.copy(payload, 0, Packet.HEADER_SIZE);


Suggested change

const payload = Buffer.alloc(data.length - Packet.HEADER_SIZE);

data.copy(payload, 0, Packet.HEADER_SIZE);

const payload = data.subarray(Packet.HEADER_SIZE);

JulienVig · 2026-10-21T16:21:29Z

+  async disconnect(gracePeriod = 0): Promise<void> {
+    if (gracePeriod > 0) {
+      await this.#peer.drain(gracePeriod);


The gracePeriod argument is only used once by peer_pool with a hardcoded constant of 10s (SHUTDOWN_GRACE_PERIOD). The default drain timeout is already hardcoded to 10s (MAX_DRAIN_TIME) so you might as well remove the gracePeriod argument since it doesn't change anything

JulienVig · 2026-10-21T16:24:23Z

+
+  it("handles payloads with more than 255 chunks", async () => {
+    // Override maxChunkSize to small size (1024 bytes) to test >255 chunk count boundary
+    Object.defineProperty(peer1, "maxChunkSize", { get: () => 1024 });


You could make maxChunkSize a constructor option instead of overriding a private attribute

JulienVig · 2026-10-21T16:25:43Z

+      throw new Error(
+        `chunk ${packet.messageID}:${packet.chunkID} already received`,
+      );


I know it was already the case before your PR but I think we should emit an error here rather than throwing, no? Same thing for the other errors in this function

ale18V added 6 commits October 20, 2026 15:23

Remove redundant SDP regex parsing for max message size

cd3daad

Rely directly on W3C standard pc.sctp.maxMessageSize and cross-browser safety default instead of regex matching raw SDP strings.

Move Packet protocol data class and tests to dedicated packet.ts module

f0ad4db

Extract framing, encoding, decoding, and packet-level constants into packet.ts with dedicated unit test suite in packet.spec.ts.

Use standard RTCSessionDescriptionInit in SignalData

7fb9810

Simplify SignalData to use W3C standard RTCSessionDescriptionInit and drop obsolete simple-peer internal transceiverRequest and renegotiate types.

Refactor some stuff

30decf4

ale18V force-pushed the fix/issue-1246-webrtc-message-size branch 2 times, most recently from a1391aa to cf115e5 Compare October 20, 2026 19:34

ale18V added 2 commits October 20, 2026 21:37

Update peer.spec.ts to use waitAcks instead of drain

8d1fb45

Move drain in disconnect function

2e31e55

ale18V force-pushed the fix/issue-1246-webrtc-message-size branch from cf115e5 to 2e31e55 Compare October 20, 2026 19:47

ale18V added 2 commits October 20, 2026 23:47

Add gracePeriod to PeerConnection.disconnect and drain during shutdown

52842e6

Format packet.ts with Prettier

3c3a164

ale18V requested a review from JulienVig October 20, 2026 22:04

ale18V changed the title ~~Replace simple-peer with native WebRTC and fix message size (fixes #1246)~~ Replace simple-peer with native WebRTC API Oct 20, 2026

JulienVig requested changes Oct 21, 2026

View reviewed changes

		if (state === "failed" \|\| state === "closed") {
		this.emit("close", undefined);

		const payload = Buffer.alloc(data.length - Packet.FIRST_HEADER_SIZE);
		data.copy(payload, 0, Packet.FIRST_HEADER_SIZE);

	const payload = Buffer.alloc(data.length - Packet.FIRST_HEADER_SIZE);
	data.copy(payload, 0, Packet.FIRST_HEADER_SIZE);
	const payload = data.subarray(Packet.FIRST_HEADER_SIZE);

		const payload = Buffer.alloc(data.length - Packet.HEADER_SIZE);
		data.copy(payload, 0, Packet.HEADER_SIZE);

	const payload = Buffer.alloc(data.length - Packet.HEADER_SIZE);
	data.copy(payload, 0, Packet.HEADER_SIZE);
	const payload = data.subarray(Packet.HEADER_SIZE);

Conversation

ale18V commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Root Cause

Changes

Uh oh!

ale18V commented Oct 20, 2026

Uh oh!

JulienVig left a comment

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 branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ale18V commented Oct 20, 2026 •

edited

Loading