1

Scenario

We are processing the signaling of WebRTC. We employ MQTT as the signaling protocol. Here is the pseudo code:

mqttClient.on('message', messageHandler)

async messageHandler(peerConnection, topic, message) {
  switch (message.type) {
    case 'offer':
      const stream = await getUserMedia({ video: true })
      stream.getTracks().forEach(track => peerConnection.addTrack(track, stream))
      await peerConnection.setRemoteDescription(message.offer)
      const answer = await peerConnection.createAnswer()
      peerConnection.setLocalDescription(answer)
      mqttClient.publish(destinationTopic, answer)
      break
    case 'candidate':
      peerConnection.addIceCandidate(message.candidate);
  }
}

We see that it is a standard WebRTC callee flow:

  1. Receive offer.
  2. Add stream.
  3. Set remote description.
  4. Create answer.
  5. Set local description.
  6. Send answer.
  7. Receive candidate.

It seems good.

Analysis

But it will fail. The WebRTC API complains with something like this: setRemoteDescription needs to called before addIceCandidate.

The culprit is this line: const stream = await getUserMedia({ video: true }). Here is the flow:

  1. Offer Received.
  2. getUserMedia() is invoked. It is async and will take a relatively long time.
  3. The whole function messageHandler yields.
  4. messageHandler is invoked again, this time, it process candidate.

When we set the call back to register the message handler, mqttClient.on('message', messageHandler), it assumes the handler is a SYNC function. If we set an ASYNC callback, it wouldn't wait for the completion of the callback. So leads to the bug. When the procedure of processing offer does not complete (still getting media), the processing of candidate starts.

Questions

  1. Why does MQTT.js (and other javascript standard runtime APIs such as event emitter, foreach/map/reduce, etc.) not support async callback (wait for the completion of the previous async callback)?
  2. Why does setRemoteDescription need to called before addIceCandidate?

Destination

  1. How to fix the bug under the current circumstance?
  2. What is the final solution in the future?
Jim Jin
  • 1,249
  • 1
  • 15
  • 28
  • Can't you just call `peerConnection.setRemoteDescription(message.offer)` first, before getting user media? – Bergi Apr 28 '23 at 15:10
  • The function `setRemoteDescription()` is also async. So there is still possible to invoke `addIceCandidate()` first. – Jim Jin Apr 28 '23 at 15:15
  • Should be fine though, does the error say "*`setRemoteDescription` needs to be **called** before `addIceCandidate` (is called)*" or "*`setRemoteDescription` needs to finish before `addIceCandidate` is called*"? I would assume the former. – Bergi Apr 28 '23 at 15:23
  • I will try it and get you posted. – Jim Jin Apr 28 '23 at 15:37
  • What do you mean async callback? You can invoke async functions in callbacks with no problem. Do you mean make event emitters synchronous? As in forcing `mqttClient.on()` to not process the event queue until the previous event have been completely handled? – slebetman May 04 '23 at 06:18
  • For example, we need to fetch several urls. But, here, the requirement is that the order of the fetches matter. We need fetch url0 first. After the completion of it, we fetch url1, then url2. If we use `foreach()` like this: `['url0', 'url1', 'url2'].foreach(url => fetch(url))`, the three fetches would be generated simultaneously, and the order of the completion is not assured. We need async callback to assure the order as if it were passed a sync callback. – Jim Jin May 04 '23 at 13:48
  • The async middleware of KOA2 is the most similar counterpart. – Jim Jin May 04 '23 at 13:55

1 Answers1

1

This has been touched in the WebRTC working group recently here. Thank you for a concrete example of when this can go wrong!

To fix this in the current instance, you can call setRemoteDescription (an async operation on the peerconnection) before getUserMedia (an async operation, but not integrated with the peerconnection's queueing mechanism).

The reason the remote description is required is that it contains information that goes along with the candidates such as the ICE usernamefragment. The ICE candidate itself is not meaningful without the context that is established by setRemoteDescription.

Philipp Hancke
  • 15,855
  • 2
  • 23
  • 31
  • Thanks. I'll try to invoke setRemoteDescription as early as possible. But I insist to think that it is a huge limitation to force calling setRemoteDescription before addIceCandidate. Hope the invoke order dependency of these 2 APIs would be removed in the future. The ICE candidate processing should be postponed when there is no context in WebRTC API side. – Jim Jin Apr 28 '23 at 16:01
  • This is essentially a problem with your signaling not queueing operations properly which is not a WebRTC problem. If you want a "physical example" you are expecting that starting to talk when the other sides phone is ringing makes sense. It doesn't... – Philipp Hancke Apr 28 '23 at 18:30
  • I agree with you. So the other problem in question is that there is no async callback for any Javascript runtime(neither browser nor node.js, from foreach/map/reduce to mqtt.js). Will async callback be added to the language spec? – Jim Jin Apr 29 '23 at 08:06