0

Simple example I think. I want to use Express to return a users IP and hostname upon request. But resolving that hostname from the IP is giving me a little bit of trouble.

router.get('/', (req, res) => {
    logRequest(req, res);
});

async function logRequest(req, res) {
    res.send({
        ip: req.ip,
        hostname: await getRemoteHostName(req.ip)
    });
}

async function getRemoteHostName(ip) {
    await require('dns').reverse(ip, (err, domains) => {
        if (err) { handleError(err); return; }
        return domains.map(s => s.toLowerCase());
    });
}

All I'm getting back is my IP {ip: 192.168.10.100} when I expect that I should be getting back my hostname too. If I console.log my hostname it prints to console but the hostname is not sent back by express in the response. What am I doing wrong? I feel like express is sending the response back before dns.reverse can finish the lookup but I want that lookup to finish and then return the response.

gh0st
  • 1,653
  • 3
  • 27
  • 59
  • 1
    It's not necessary to understand async/await to make this work. You need to understand promises. `dns.reverse` doesn't return a promise so it cannot be awaited. Promisify it. – Estus Flask Nov 01 '18 at 15:32
  • remove `await` before my `require('dns')...` and wrap it in a `new Promise()` is what you're saying? – gh0st Nov 01 '18 at 15:33
  • That's correct. For last Node versions there's already promisified version of it, as the answer mentions. – Estus Flask Nov 01 '18 at 15:41

2 Answers2

1

You're mixing callback and async/await

await should be combined with a Promise

In your case dns.reverse is not a promise and return nothing.

First, you need to use dns promise

Then you need to update getRemoteHostName to return hostnames and handle errors

const { Resolver } = require('dns').promises;
const resolver = new Resolver();

async function getRemoteHostName(ip) {
  try {
    const hostnames = await resolver.reverse(ip)
    // your logic here
    return hostnames
  } catch(error) {
    // handle error here
  }
}
Jordane
  • 624
  • 1
  • 7
  • 23
  • So in newer versions of node you don't need `... = require('dns').promises;`, just `const { Resolver } = require('dns');` – gh0st Nov 01 '18 at 15:45
  • The linked docs ... are `11.0` ... what's newer than that? The docs show to use `.promises` – Cody G Nov 01 '18 at 15:46
  • Crap, sorry. I'm on v8.11. So I need to wrap my call in a promsie. Correct – gh0st Nov 01 '18 at 15:49
  • 1
    Yeah I'm not sure what exactly you need to do for 8.x. Looks like you might be able to use util.promsify for some stuff but probably safer to just convert it yourself using a `function(err,data){return new Promise(...)}` type of thing – Cody G Nov 01 '18 at 15:53
  • @CodyG. could you double check my answer I posted below please and see if that makes sense to you? And if I'm missing anything? – gh0st Nov 01 '18 at 16:11
0

With node v8.11.3 I needed to promisify my getRemoteHostName() method.

function getRemoteHostName(ip) {
  return new Promise((resolve, reject) => {
    require('dns').reverse(ip, (err, domains) => {
      if (err) { reject(err); }
      resolve(domains.map(d => d.toLowerCase()));
    });
  }).catch(error => {
    console.log(`error: ${error}`);
  });
}

After that I wrapped my logRequest() with an async/await like so

async function logRequest(req, res) {
  const client = {
    ip: req.ip,
    hostname: await getRemoteHostName(req.ip)
  };

  res.send(client);
}

At which point I could make a request to Express with

router.get('/', (req, res) => {
    logRequest(req, res);
});

And get my hostname as expected.

gh0st
  • 1,653
  • 3
  • 27
  • 59
  • 1
    get rid of the `.catch()` you're swallowing the errors, and I personally do `if(err){reject(err)}else{resolve(...)}` I think you can run into asynchronous issues if reject is then passed to an asynchronous error handling function ... resolve would be called too, which would seem like it would throw some other type of weird error that promises can only be resolved or rejected once but I actually don't know what would happen. – Cody G Nov 01 '18 at 16:55
  • 1
    good question resource on it though, which I agree with https://stackoverflow.com/questions/32536049/do-i-need-to-return-after-early-resolve-reject ... basically I don't like the lack of using `else` or a `return` because you can indeed run code unintended. – Cody G Nov 01 '18 at 17:04