simple-peer icon indicating copy to clipboard operation
simple-peer copied to clipboard

`Ignoring unsupported ICE candidate` logs when rapidly creating/destroying peers

Open dguenther opened this issue 4 years ago • 0 comments

What version of this package are you using?

9.11.0

What operating system, Node.js, and npm version?

  • macOS 11.3
  • Node.js 14.16.1
  • npm 6.14.2
  • wrtc 0.4.7

What happened?

Rapidly creating/destroying SimplePeer instances causes the console to warn with Ignoring unsupported ICE candidate: https://github.com/feross/simple-peer/blob/d972548299a50f836ca91c36e39304ef0f9474b7/index.js#L234-L235

Adding warn(err) before the existing warning displays the error as [Error: Failed to set ICE candidate; RTCPeerConnection is closed.], so I'm not sure it's accurate in this case.

Also, one of the conditions for displaying the log is !iceCandidateObj.address, but when I logged iceCandidateObj and ran the demo code below, iceCandidateObj.address was null regardless of whether addIceCandidate threw an error.

Demo code: (repository is here for reproduction: https://github.com/dguenther/simple-peer-issue-demo)

require('segfault-handler').registerHandler('segfault.log')
const SimplePeer = require('simple-peer')
const wrtc = require('wrtc')

const LOOP_TIME_MS = 70

function getRandomInt(min, max) {
  min = Math.ceil(min);
  max = Math.floor(max);
  return Math.floor(Math.random() * (max - min + 1)) + min;
}

let iteration = 0

const initiators = []
const recipients = []

async function eventLoop() {
  console.log(`Iteration ${++iteration}`)
  
  while (initiators.length > 20) {
    const conn = initiators.splice(getRandomInt(0, initiators.length - 1), 1)[0]
    conn.destroy()
  }
  
  while (recipients.length > 20) {
    const conn = recipients.splice(getRandomInt(0, initiators.length - 1), 1)[0]
    conn.destroy()
  }
  
  for (let i = 0; i < 4; i++) {
    const recip = new SimplePeer({ initiator: false, wrtc })
    const init = new SimplePeer({ initiator: true, wrtc })

    recip.on('signal', (signal) => {
      if (!init.destroyed) init.signal(signal)
    })
    init.on('signal', (signal) => {
      if (!recip.destroyed) recip.signal(signal)
    })
    
    initiators.push(init)
    recipients.push(recip)
  }
  
  setTimeout(eventLoop, LOOP_TIME_MS)
}

eventLoop()

What did you expect to happen?

I'd prefer for the log to not go directly to console, since users are reporting it as a bug when they see it in our CLI, and I think we'd have to use something like filter-console to filter the logs out.

Are you willing to submit a pull request to fix this bug?

Yep 👍 I could use some advice on what the best change would be here though. It looks like this was originally added to deal with mDNS issues in a few PRS: #517 #521

dguenther avatar May 12 '21 22:05 dguenther