node_mdns icon indicating copy to clipboard operation
node_mdns copied to clipboard

Do not emit error when callback is passed to Advertisement.create

Open achingbrain opened this issue 10 years ago • 2 comments

If dns_sd.DNSServiceRegister results in an error and you've passed a callback to Advertisement.create the error is passed to the callback as well as being emitted on the advert itself.

E.g.:

var advert = mdns.createAdvertisement(mdns.tcp('my-service'), port, function (error) {
  // will be called with error
})
advert.on('error', function (error) {
  // will also be called with the same error
})

Since the error is emitted on the advert you have to listen for the error event even if you pass a callback otherwise your process will crash with an uncaught exception.

This pull request changes Advertisement to not emit the error if a callback was passed and adds tests for the new behaviour.

achingbrain avatar Apr 15 '15 09:04 achingbrain

I'm happy with the solution, but why the sinon injection here?

ronkorving avatar Aug 20 '15 03:08 ronkorving

Apologies for being unresponsive. It must have slipped... my bad.

Man, this (my) code is old... and it still has error handling bugs. >:-{ ... So, nice catch. The fix looks good to me too, but like @ronkorving I wonder if we really need a new dependency. Could you refactor the test to make do without it?

agnat avatar Aug 20 '15 14:08 agnat