DataChannel.OnOpen() fires before ACK is received

[jwhited]

Your environment.

Both peers are Pion

• Version: 3b6c071

What did you do?

Opened a DataChannel between two peers and wrote data to it from the dialing side once DataChannel.OnOpen() fired. A SettingEngine with DetachDataChannels() is being used on both sides.

Dialing side:

	// create peerConnection & offer
	settingEngine := webrtc.SettingEngine{}
	settingEngine.DetachDataChannels()
	api := webrtc.NewAPI(webrtc.WithSettingEngine(settingEngine))
	config := webrtc.Configuration{
		ICEServers: []webrtc.ICEServer{
			{
				URLs: []string{"stun:stun.l.google.com:19302"},
			},
		},
	}
	peerConnection, err := api.NewPeerConnection(config)
	if err != nil {
		return fmt.Errorf("error creating peer connection: %v", err)
	}
	offer, err := peerConnection.CreateOffer(nil)
	if err != nil {
		return fmt.Errorf("error creating peer connection offer: %v",
			err)
	}
	err = peerConnection.SetLocalDescription(offer)
	if err != nil {
		return fmt.Errorf(
			"error setting local description on peer connection: %v",
			err)
	}

	// [...] signaling and waiting for PeerConnection to open

	// PeerConnection is open

	ordered := false
	maxRetransmits := uint16(0)
	dc, err := peerConnection.CreateDataChannel(fmt.Sprintf("%s:%d",
		remoteHost, remotePort), &webrtc.DataChannelInit{
		Ordered:           &ordered,
		MaxRetransmits:    &maxRetransmits,
		MaxPacketLifeTime: nil,
	})
	if err != nil {
		return fmt.Errorf("error creating dataChannel: %v", err)
	}

	dc.OnOpen(func() {
		// get an io.ReadWriteCloser from the dataChannel
		rwc, err := dc.Detach()
		if err != nil {
			fmt.Printf("error detaching dataChannel: %v\n", err)
			return
		}

		// remote side logs when we write here:
		// Failed to accept data channel: unexpected packet type: WebRTC binary
		// but if we sleep for 1 second it works
		rwc.Write([]byte("hello"))
	})

What did you expect?

Data was transported successfully

What happened?

The remote side dropped the data and the following was logged:

Failed to accept data channel: unexpected packet type: WebRTC binary

If I add a 1s sleep in OnOpen() before writing there are no issues

Based on discussion in Slack there is an assumption that this bug is specific to the use of the SettingEngine.DetachDataChannels(), but I haven't confirmed this yet.

[Sean-Der]

@jwhited Thanks for the report!

I don't think the issue is specific to detached, but maybe just timing related.

The fix is that we shouldn't consider the channel Open until channelAck is processed.

We should then add a OnOpen to datachannel.DataChannel that we fire when we get it. Then only pion/webrtc should fire the event.

I think it is great first task @jwhited and happy to help you get it merged. If you don't have the bandwidth I can fix it right now though! Shouldn't take me more then 30 mins

[jwhited]

I'll give it a shot @Sean-Der

[Sean-Der]

Hey @jwhited

Anyway I can help with this? If you don't have the bandwidth I am happy to take it on

[jwhited]

@Sean-Der ended up moving to a reliable dataChannel design for other reasons and this is no longer an issue due to timing changes and/or retrans. I may not have time in the short term to get a PR in, feel free fix it

[normanr]

I'm coming across this too (and I'm not using detached channels). Is it because of the lack or ordering or lack of reliability (or both)?

[normanr]

heh, pion/datachannel DataChannel.handleDCEP has a TODO for channelAck: referencing https://tools.ietf.org/html/draft-ietf-rtcweb-data-protocol-09#section-5.2. Note that messages may be sent before before ACK is received, but must be sent ordered.

also as part of this: SCTPTransport.acceptDataChannels triggers OnError, but nothing is subscribed to it (and external parties can't because pion/webrtc DataChannel.Transport() returns nil).

[AeroNotix]

@Sean-Der any movement on this?

[chrisprobst]

@Sean-Der We came across this as well. We will try the sleep workaround, however, this does not seem right through. Is there any movement on this issue? We are in the process of migrating to v2 (we tested v3 as well, but it does not work as well).

Is there a better workaround currently?

We are actually doing the exact same stuff like in the issue, we send data on OnData() and we get:
ortc ERROR: 2020/09/24 15:21:56 Failed to accept data channel: unexpected packet type: WebRTC Binary

However, it is actually sufficient that the browser sends this message, so using a sleep might not work on the Go side. :-(

Any thoughts on this?

[chrisprobst]

Ok, we found something. We were using negotiated channels, using OnDataChannel, this bug does not occur. Maybe that helps :-).

[grahn]

I'm seeing this issue with webrtc v3.0.11.

Has this been fixed in an upcoming version of datachannel? I noticed commit cc39fb6310540b11169368f4df9badc332f1a46a in datachannel removed the TODO about handling the channel ACK, but it's not clear to me that anything actually changed as far as handling a channel ACK.

webrtc v3.0.11 uses datachannel v1.4.21, which is older than that commit.

[edaniels]

Can also report seeing this issue in 3.0.11. I can reproduce it about 2.5% of the time in tests.

[edaniels]

@grahn I go get: upgraded github.com/pion/datachannel v1.4.21 => v1.4.22-0.20210420230629-6daf0fdcfcc0 and I'm not seeing any issues :). @Sean-Der, would it be crazy to upgrade webrtc to an official datachannel v1.4.22?

[AeroNotix]

This bug prevents me from using unordered channels and ultimately improving the performance of our application.

@Sean-Der are you able to summarise how one would go about triaging and finding out where the issue lies?

[joshughes]

This bug also is preventing me from using unordered channels