Webrtc-pc: RTCDataChannel.send during 'closing' state

[lgrahl]

Let's look at the following scenario: Peer A and B have an open data channel. If peer A closes the channel, peer B's channel will go into the closing state until all pending messages of both peers have been sent. If peer B calls the .send method, it will throw an exception since the channel is not in the open state any more. But peer B has no way of knowing that unless it polls .readyState.

Do we need to address that with an... (I'm reluctant to say that) event?

[aboba]

Can you submit a PR?

[lgrahl]

As an alternative: What if we simply say to discard the message passed to the .send call in case .readyState is closing? Edit: Mh, no, on second thought I don't like that at all.

[lgrahl]

Adding an onclosing event would be overkill in my opinion. I would like to hear more opinions (and hopefully suggestions) before creating a PR.

[aboba]

@lgrahl Looking at RFC 6525 and Section 6.7 of draft-ietf-rtcweb-data-channel, one way to handle this would be to have receipt of an incoming stream reset cause .readyState to transition to closed. The receipt of an incoming stream reset causes an outgoing stream reset to be sent. While after sending an outgoing stream reset the peer could transition .readyState to closing and await receipt of a re-configuration response with a result of success prior to transitioning .readyState to closed, after sending the outgoing stream reset, data can neither be sent nor received so closed might be more appropriate.

"6.7. Closing a Data Channel

Closing of a data channel MUST be signaled by resetting the
corresponding outgoing streams [RFC6525]. This means that if one
side decides to close the data channel, it resets the corresponding
outgoing stream. When the peer sees that an incoming stream was
reset, it also resets its corresponding outgoing stream. Once this
is completed, the data channel is closed. Resetting a stream sets
the Stream Sequence Numbers (SSNs) of the stream back to 'zero' with
a corresponding notification to the application layer that the reset
has been performed. Streams are available for reuse after a reset
has been performed.

[RFC6525] also guarantees that all the messages are delivered (or
abandoned) before the stream is reset."

[lgrahl]

That would probably resolve the issue with .send but open another one: The close event is an indication that a stream ID can be reused. However, if we set the closed state earlier, the application doesn't know when an ID can be reused.

[jhaenchen]

@aboba or @lgrahl Excuse me if I'm misinterpreting this, but how would this procedure work for a hard-quit peer, i.e. a browser peer that has been forced to quit and so does not send any reset or anything? I am still stuck in a closing state in this scenario and thought it might be relevant here.

To elaborate:

I have two connected peers, they share a RTCPeerConnection and a DataChannel. The other peer then force-quits their browser. I receive oniceconnectionstatechange with peerConnection.iceConnectionState = 'failed'. Because of this event, I want to clean up the RTCDataChannel, so I call channel.close() and so dataChannel.readyState = 'closing'. Here's where the issue starts: presumably because the RTCPeerConnection that supports the data channel is no longer connected, the reset signal and any queuing operations will fail and onclose is not fired. As I type this I wonder if it's more of a chromium bug or if the specification does not provide for this scenario.

[lgrahl]

@jhaenchen There's no need for cleanup in that case. I was going to write that:

When the ICE connection state goes into failed, all data channels should be in the closed state immediately. If they aren't, it's a bug in the implementation.

But I haven't found a procedure in the spec that defines what should happen once the peer connection state goes into failed. @aboba, @jan-ivar, anyone? :sweat_smile: But to prevent becoming sidetracked in this issue, perhaps one of you could open a new issue when responding.

Anyhow, even if the procedure is missing, there is no need to do cleanup and the data channels should be in the closed state. Whether or not the close event should fire is up for debate. However, I would argue that the event should fire because it is triggered by the remote side.

[aboba]

@lgrahl Could we fire the onclosed event when readyState transitions to "closing"?

[lgrahl]

We discussed that before, see https://github.com/w3c/webrtc-pc/issues/1827#issuecomment-385131434 (but I also forgot that we had this discussion :sweat_smile:). It is unknown whether there is any application relying on that mechanism.

Another suboptimal alternative would be to say "well, .send can throw if the remote is closing, deal with it".

The least breaking would be to add an onclosing event but... I'm really reluctant to add a new event for a corner case that 99% of applications don't care about.

[jhaenchen]

I use the onclose event to discern if I should reinitiate a connection. If a connection fails and the onclose event doesn’t fire, the only way I can tell is to monitor the ice connection state for a particular state which seems more brittle than relying on an event.

And it seems that onclose may not fire for user-initiated events either? I’m struggling to understand how best to utilize this event handler.

[jan-ivar]

@jhaenchen are you still basing observations on Chrome? Do you get different results in Firefox? This sounds like https://github.com/w3c/webrtc-pc/issues/1020#issuecomment-423280588 again.

[jan-ivar]

onclosing seems like the simplest solution to me.

[jhaenchen]

@jan-ivar that does seem like the bug I’m experiencing. Didn’t see the comment you referenced either, thanks for bringing that to my attention!

[aboba]

Outcome of TPAC was to add "onclosing".