Simple-peer: Meta information about error context passed to _destroy and .emit('error', x)

[joshontheweb]

It would be helpful to have some meta information about the source function in which the error happened passed along with error events. Since the errors all get channeled into the one _destroy function before being emitted, it can be hard to categorize the errors programmatically as to which stage in the signaling process the error originated in. That is, unless I'm missing something.

For instance, I would like to be able to log errors categorized by the context of the failure like so:

peer.on('error', function (err) {
  var errorContext = 'createOffer'
  reportError(peer._pc, errorContext, err)
})

As it stands now, it seems difficult to discern the error context for reporting without doing something like triggering every possible error and trying to build a hardcoded index of what could come from what source. I also imagine that this approach wouldn't be portable between browsers since some of the browser's native error messages are passed directly through.

What I propose would be something like this using createOffer as an example:

Peer.prototype._createOffer = function () {
  var self = this
  if (self.destroyed) return

  self._pc.createOffer(function (offer) {
    if (self.destroyed) return
    offer.sdp = self.sdpTransform(offer.sdp)
    self._pc.setLocalDescription(offer, onSuccess, onError)

    function onSuccess () {
      if (self.destroyed) return
      if (self.trickle || self._iceComplete) sendOffer()
      else self.once('_iceComplete', sendOffer) // wait for candidates
    }

    function onError (err) {
      self._destroy(err, null, 'setLocalDescription') // <-- string representing failure context
    }

    function sendOffer () {
      var signal = self._pc.localDescription || offer
      self._debug('signal')
      self.emit('signal', {
        type: signal.type,
        sdp: signal.sdp
      })
    }
  }, function (err) { self._destroy(err, null, 'createOffer') }, self.offerConstraints) // <-- here again
}


Peer.prototype._destroy = function (err, onclose, errContext) { // <-- take errContext as third optional arg
  var self = this
  ...
  if (err) self.emit('error', err, errContext) // <-- pass errContext through to the error event
  self.emit('close')
}

By adding the errContext as a third optional argument to _destroy() it should keep things backwards compatible while enabling more descriptive and granular debugging and error tracking.

I'm not married to this particular solution and welcome alternative ideas on this if there is a better way.

Also I'm happy to put in the leg work and make a pull request as long as there is agreement from the maintainers that this is a welcome addition. Thanks :)

[t-mullen]

I agree debugging can be difficult. If the errors thrown by WebRTC had a stack trace, this wouldn’t be an issue.

A solution could be wrapping the errors in a new error object when they’re caught. Then we could use the existing destroy method and see the context with err.stack.

[feross]

I think the right solution here is to use error codes. This is what Node.js does.

So, we would attach a 'code' property to the browser error objects with different string values, like 'ERR_SET_LOCAL_DESCRIPTION', etc.

And anywhere we create our own errors we could use a helper function like this to do it instead:

function makeError (message, code) {
  var err = new Error(message)
  err.code = code
  return err
}

Then, we'd need to add a section to the readme to document these string values, and consider any change to them to be a major version breaking change.

[t-mullen]

Even better! So in the example above, we'd do:

self._destroy(makeError(err, 'ERR_SET_LOCAL_DESCRIPTION'), null)

[joshontheweb]

Sounds good to me. I like this a bit better as it would avoid having to parse out function names from the stack traces. I'll take a stab at getting some of this implemented soon.

[feross]

@RationalCoding Well, we recently changed the destroy() API, so I believe it's just:

self.destroy(makeError(err, 'ERR_SET_LOCAL_DESCRIPTION'))

[t-mullen]

Closed by #243