Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

achingbrain
Copy link
Contributor

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.

@ronkorving
Copy link
Collaborator

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

@agnat
Copy link
Owner

agnat commented Aug 20, 2015

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants