dgram socket.send() throws exceptions instead of invoking callback on DNS lookup failure

[othiym23]

For context, see LockerProject/Locker#916.

Looking at lines 146-148 of dns.js, it seems like should the c-ares call fail, the lookup code will throw an exception that isn't trapped and passed off to the callback in dgram.js. This means that right now, to completely handle some expected failure modes, it's necessary to wrap the call (which already has a callback ready to handle errors) in an exception-handling block.

[othiym23]

This is being seen in v0.6.10.

[othiym23]

Actually, I spoke too soon -- because the exception is getting thrown on the next tick, there's no way to trap it except at a global level. Is there a supported pattern for dealing with this, or is this a straight-up bug?

[othiym23]

Further investigation reveals that even if you're using the dgram socket as a send-only connection, you still need to have an error handler defined on the socket listener. Even if there's an error handler on socket.send() (as in this example), if there's not an error listener on the socket, you'll get an unhandled exception propagated to the root. This begs the question of why socket.send() accepts an error handler.

[Southern]

Looks like someone forgot to change it over from throw ... to callback(...). Since callback seems to be always defined, not sure why there was even a throw there to begin with.

[bnoordhuis]

Looking at lines 146-148 of dns.js, it seems like should the c-ares call fail, the lookup code will throw an exception that isn't trapped and passed off to the callback in dgram.js.

That call to cares.getaddrinfo() cannot fail (unless the system runs out of memory but in that case we can't go forward anyway). The throw is a sanity check, it could have been an assert if we used those.

Further investigation reveals that even if you're using the dgram socket as a send-only connection, you still need to have an error handler defined on the socket listener. Even if there's an error handler on socket.send() (as in this example), if there's not an error listener on the socket, you'll get an unhandled exception propagated to the root. This begs the question of why socket.send() accepts an error handler.

It's not an error handler, it's a 'datagram sent' callback. The err argument is null if everything went right.

Are you seeing actual bugs? If so, can you post a test case?

[othiym23]

Test case can be seen in the example linked above, based on code from the dgram documentation:

1. Create a socket that's going to be used only to send datagrams.
2. Call socket.send() with an invalid hostname and a callback.
3. The resolution failure will get caught by the callback, but will also get thrown as an exception because there isn't a socket.on('error') handler registered.

To be clear, this might not be a bug, but this behavior differs from what I would have expected given the documentation.

[bnoordhuis]

Right, the exception object is passed to the callback and emitted as an 'error' event. It's kind of silly but that's how v0.2 and v0.4 used to do it. We can change that in v0.8: pass to the callback if there is one, otherwise emit it.

[Southern]

Don't know that I would want to stop it from emitting and using a callback. They could be using the event for generic logging, and the callback to handle and generate specific error types based on the dns.lookup they're performing.

[othiym23]

These points make sense, but it would be very useful if there were a single-call method that would manage setting up and tearing down the connection for just shooting off single packets into the network. I'm using UDP for statsd instrumentation, and it doesn't require the full overhead of a persistent socket connection. I can guarantee that the connection will never receive any packets.

[mcollina]

I'm hitting this issue in https://github.com/mcollina/node-coap, again in node v0.10.20.
I'm going around it, but I thinks is a bit of a long standing issue.

To reproduce:

var dgram  = require('dgram')
  , client = dgram.createSocket('udp4')

client.send(new Buffer(1), 0, 1, 1234, 'aaaa.eee', function(err) {
  if (err)
    console.log('error received')

  client.close()
})

This unfortunately result in:

$ node error.js
error received

events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: getaddrinfo ENOTFOUND
    at errnoException (dns.js:37:11)
    at Object.onanswer [as oncomplete] (dns.js:124:16)

[bnoordhuis]

@mcollina That's because the socket doesn't have an 'error' event listener.

It's somewhat inelegant but right now you should always have an 'error' listener. There has been some discussion about changing that but that was eventually decided against, see #4846 for rationale.

[mcollina]

Adding an 'error' listener does not solve the issue: I don't know where the error came from.

This very fact make the whole 'send' method much less useful, as I need to do DNS-resolving myself (to track down the error correctly), as CoAP was designed to reuse the same socket for multiple exchanges.

No real issue, but the doc should come with a huge warning on that, something like:
"If you reuse the same socket for multiple message exchanges, do the DNS-resolving yourself as DNS errors are emitted on the socket".