diff --git a/doc/api/dns.markdown b/doc/api/dns.markdown index 13dee0a6cb4375..03e204aae3befe 100644 --- a/doc/api/dns.markdown +++ b/doc/api/dns.markdown @@ -168,6 +168,23 @@ corresponding lookup methods. On error, `err` is an [`Error`][] object, where `err.code` is one of the error codes listed [here](#dns_error_codes). +Returns an `EventEmitter` object that will emit a single `'complete'` event +on the next next tick of the Node.js event loop (using `setImmediate()`) after +the resolve operation completes, or an `'error'` event if an error occurs +while *creating* the resolve request. + +For instance, the `dns.setServers()` method will throw an error if called +while an outstanding `dns.resolve()` operation is still being processed. The +`'complete'` event, then, can be used to determine when the resolve is +completely finished so that the `dns.setServers()` call can be invoked: + +```js +dns.resolve('example.com', 'A', (err, addr) => { /** ... **/ }) + .on('complete', () => { + dns.setServers(newServerArray); + }); +``` + ## dns.resolve4(hostname, callback) Uses the DNS protocol to resolve a IPv4 addresses (`A` records) for the @@ -283,8 +300,8 @@ If a port specified on the address it will be removed. An error will be thrown if an invalid address is provided. -The `dns.setServers()` method must not be called while a DNS query is in -progress. +An error will be thrown if the `dns.setServers()` method is called while a +DNS query is in progress. ## Error codes diff --git a/lib/dns.js b/lib/dns.js index 8d1541718abf75..2b5370a348ac2f 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -5,6 +5,7 @@ const util = require('util'); const cares = process.binding('cares_wrap'); const uv = process.binding('uv'); const internalNet = require('internal/net'); +const EventEmitter = require('events'); const GetAddrInfoReqWrap = cares.GetAddrInfoReqWrap; const GetNameInfoReqWrap = cares.GetNameInfoReqWrap; @@ -13,6 +14,7 @@ const QueryReqWrap = cares.QueryReqWrap; const isIP = cares.isIP; const isLegalPort = internalNet.isLegalPort; +var _resolvesOutstanding = 0; function errnoException(err, syscall, hostname) { // FIXME(bnoordhuis) Remove this backwards compatibility nonsense and pass @@ -212,33 +214,62 @@ exports.lookupService = function(host, port, callback) { function onresolve(err, result) { - if (err) - this.callback(errnoException(err, this.bindingName, this.hostname)); - else - this.callback(null, result); + const emitter = this.emitter; + if (err) { + process.nextTick(() => { + emitter.emit('error', + errnoException(err, this.bindingName, this.hostname)); + }); + } else { + process.nextTick(() => { + emitter.emit('resolve', result); + }); + } + // Decrement the outstanding resolve counter. + _resolvesOutstanding--; + // This has to be done on setImmediate in order to give c-ares + // time to cleanup after the resolve completes. + setImmediate(() => emitter.emit('complete')); } +class Resolver extends EventEmitter { + constructor(binding, bindingName, name, callback) { + super(); + _resolvesOutstanding++; + if (typeof name !== 'string') { + throw new Error('"name" argument must be a string'); + } + // Do this on nextTick in order to give users time to + // attach the on 'complete' listener. + process.nextTick(() => { + var req = new QueryReqWrap(); + req.bindingName = bindingName; + req.hostname = name; + req.oncomplete = onresolve; + req.emitter = this; + // For backwards compatibility reasons, if the callback is + // passed, set it as both the resolve and error listener, + // so that errors are handled by the callback. + if (typeof callback === 'function') { + req.emitter.on('resolve', (res) => { + callback(null, res); + }); + req.emitter.on('error', callback); + } + var err = binding(req, name); + if (err) { + _resolvesOutstanding--; + req.emitter.emit('error', errnoException(err, bindingName)); + } + }); + } +} function resolver(bindingName) { var binding = cares[bindingName]; return function query(name, callback) { - if (typeof name !== 'string') { - throw new Error('"name" argument must be a string'); - } else if (typeof callback !== 'function') { - throw new Error('"callback" argument must be a function'); - } - - callback = makeAsync(callback); - var req = new QueryReqWrap(); - req.bindingName = bindingName; - req.callback = callback; - req.hostname = name; - req.oncomplete = onresolve; - var err = binding(req, name); - if (err) throw errnoException(err, bindingName); - callback.immediately = true; - return req; + return new Resolver(binding, bindingName, name, callback); }; } @@ -283,6 +314,8 @@ exports.getServers = function() { exports.setServers = function(servers) { + if (_resolvesOutstanding > 0) + throw new Error('Cannot call setServers() while resolving.'); // cache the original servers because in the event of an error setting the // servers cares won't have any servers available for resolution const orig = cares.getServers(); diff --git a/test/internet/test-dns-setservers-during-resolve.js b/test/internet/test-dns-setservers-during-resolve.js new file mode 100644 index 00000000000000..bfcbade5918992 --- /dev/null +++ b/test/internet/test-dns-setservers-during-resolve.js @@ -0,0 +1,15 @@ +'use strict'; + +const common = require('../common'); +const dns = require('dns'); +const assert = require('assert'); + +var m = dns.resolve('www.example.com', 'A', (err, addresses) => { + assert.throws(() => { + dns.setServers([]); + }, /Cannot call setServers\(\) while resolving./); +}); + +m.on('complete', common.mustCall(() => { + dns.setServers([]); +})); diff --git a/test/internet/test-dns.js b/test/internet/test-dns.js index aec66f5882ff5f..f05a5bd2d4ce7c 100644 --- a/test/internet/test-dns.js +++ b/test/internet/test-dns.js @@ -44,20 +44,13 @@ function checkWrap(req) { TEST(function test_reverse_bogus(done) { - var error; - try { - dns.reverse('bogus ip', function() { - assert.ok(false); - }); - } catch (e) { - error = e; - } - - assert.ok(error instanceof Error); - assert.strictEqual(error.errno, 'EINVAL'); + dns.reverse('bogus ip', (err) => { + assert(err); + assert.strictEqual(err.errno, 'EINVAL'); + done(); + }); - done(); }); diff --git a/test/parallel/test-dns-regress-7070.js b/test/parallel/test-dns-regress-7070.js index e696327d4d5f66..7bcb01f45f0a5d 100644 --- a/test/parallel/test-dns-regress-7070.js +++ b/test/parallel/test-dns-regress-7070.js @@ -5,4 +5,6 @@ var dns = require('dns'); // Should not raise assertion error. Issue #7070 assert.throws(function() { dns.resolveNs([]); }); // bad name -assert.throws(function() { dns.resolveNs(''); }); // bad callback +// The following is no longer valid since dns.resolve returns an +// EventEmitter. The results can be handled by setting event listeners. +//assert.throws(function() { dns.resolveNs(''); }); // bad callback