paritytech/smoldot

WebRTC opening failure panic

tomaka opened this issue · 6 comments

When a WebRTC connection fails to open, there seems to be a panic here:

let stream = lock.streams.get_mut(&(connection_id, stream_id)).unwrap();

The problem is simply caused by the fact that the browser calls the close or error callback on the data channel when it fails to open.

Because we only register the data channel after the open callback is called, it's not there yet.

This is a bit tricky to solve, because the definition of "open" of a WebRTC connection is a bit hacky.

I think that this is simply solved by making connection_stream_open return the stream ID, and make connection_stream_opened interpret already-assigned IDs as "outbound" and unknown IDs as "inbound".

I'll wait for #2925 in order to not introduce annoying merge conflicts.

Actually, the idea is that we should just try opening the substream again.
There is no gain is actually tracking the pending substream openings compared to using a timeout.

Related comment from the spec:

Implementations SHOULD setup all the necessary callbacks (e.g.
[`ondatachannel`](https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/datachannel_event))
before starting the Noise handshake. This is to avoid scenarios like one where
_A_ initiates a stream before _B_ got a chance to set the `ondatachannel`
callback. This would result in _B_ ignoring all the messages coming from _A_
targeting that stream.

Related comment from the spec:

That's not really related.
The problem here was that Firefox calls onclose or onerror on the handshake substream because we fail to reach the server, and our implementation didn't consider this possibility. I was assuming that onclose/onerror would only be called after the substream has been opened, and that we'd get a onconnectionstatechange event instead for the reach failure.