Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Specify that UDP DNS errors are emitted to the socket's 'error' listeners #6354

Closed
wants to merge 1 commit into from
Closed

Conversation

mcollina
Copy link
Member

See #2900 and #4846

@Nodejs-Jenkins
Copy link

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit mcollina/node@e3455e6 has the following error(s):

  • First line of commit message must be no longer than 50 characters
  • Commit message must indicate the subsystem this commit changes

The following commiters were not found in the CLA:

  • Matteo Collina

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

Specify that UDP DNS errors are emitted and cannot be tracked for every
different `send` call.

See #2900
and #4846
** A Note about DNS resolving **

If you reuse the same socket for concurrent message exchanges with different hosts,
do the DNS-resolving yourself as DNS errors are emitted on the socket `error` event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably more appropriate as a dgram.Socket#send() note?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a note to dgram.Socket#send(), but it is after the note about datagram size. I think the one about datagram size is more important, so I put this one after that.

@bnoordhuis
Copy link
Member

I've decided against landing the patch for now. I've reopened #4846 because I think we need to look at the API again; I've been playing around with it and I've got to admit the current behavior is pretty bad.

@bnoordhuis bnoordhuis closed this Oct 15, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants