Waiting for an function to complete before executing the next step - node js

Question

I am new to node js and currently learning about promises and await/async. I tried the below code but couldn't figure out how to make the code wait till the function hostping is finished. I have also tried promises but couldn't make it wait.

var ping = require('ping');
var hoststatus
var hosts = ['google.com'];

async function test()
{
  var connected = await hostping(hosts);
  console.log('connected--------------', connected)
  connected.then(function(hoststatus) {
    console.log('hoststatus--------------', hoststatus)
    if (hoststatus == 1) {
      console.log('faaaaaaaail-------------')
    } else {
      console.log('passssssssssssssssss-----------')
    }
  });
}

async function hostping(hosts) {
  console.log('hosts-----------', hosts)
  await hosts.forEach(async function(host) {
    await ping.sys.probe(host, async function(isAlive) {
      var msg = isAlive ? 'host ' + host + ' is alive' : 'host ' + host + ' is dead';
      console.log(msg);
      if (isAlive == 'false') {
        hoststatus = 1
      }
    });
  });

  return hoststatus;
}

test()

@Eugen Sunic - thanks for the comment. I tried but still get the same issue. `var ping = require('ping'); var hoststatus var hosts = ['google.com']; const promise1 = new Promise(function(resolve, reject) { hosts.forEach(function(host){ ping.sys.probe(host, function(isAlive){ var msg = isAlive ? 'host ' + host + ' is alive' : 'host ' + host + ' is dead'; console.log(msg); }); }); resolve ('host check is completed') }); Promise.all([promise1]).then(function(values) { console.log(values); });` — karthik110885, Feb 10 '20 at 12:45
since `Array#forEach` returns `undefined` you can't await it - use a vanilla for loop instead - also, I doubt `ping.sys.probe` returns a Promise, since it takes a callback - so you can't wait that either ... and since the callback (`function(isAlive)`) never uses `await`, why make it `async` - seems you've read about async/await and used it everywhere where it's not useful - and one last thing ... why is `hoststatus` global? — Jaromanda X, Feb 10 '20 at 12:48

Terry Lennox · Answer 1 · 2020-02-10T13:04:39.760

This should do what you wish, we use a for .. of loop to iterate the hosts in order.

The ping library also has a promise wrapper that allows you to skip using callbacks.

You could also use Promise.all to do all the pings and probes at once, but I don't believe this is what you wish to do.

I have included a hostpingVer2 that uses Promise.all if you want to do all pings at once.

const ping = require('ping');
const hosts = ['google.com', 'amazon.com', 'itdoesntexist'];

async function test() {
    console.log('hosts: ', hosts)
    const results = await hostping(hosts);
    console.log('ping results: ', results);
}

async function hostping(hosts) {
    const results = [];
    for(let host of hosts) {
        let probeResult = await ping.promise.probe(host);
        results.push( { host, hoststatus: probeResult.alive ? 0: 1, alive: probeResult.alive } );
    }
    return results;
}

async function hostpingVer2(hosts) {
    const probeResults = await Promise.all(hosts.map(host => ping.promise.probe(host)));
    return probeResults.map(probeResult => { 
        return { host: probeResult.host, hoststatus: probeResult.alive ? 0: 1, alive: probeResult.alive } 
    });
}

test();

You are very welcome indeed! Thank you for your question! – Terry Lennox Feb 10 '20 at 13:05 — Terry Lennox, Feb 10 '20 at 13:05

Ashish Modi · Answer 2 · 2020-02-10T12:57:32.723

0

You are using await inside forEach which is not supposed to work as forEach doens't support promises. Consider changing it to for..of or promise.all.

Also, your probe function is a callback style which doesn't return promise. You need to wrap it in a promise to await it.

function probe(host) {
  return new Promise((resolve, reject) => {
    ping.sys.probe(host, function (isAlive) {
      const msg = isAlive ? `host ${host} is alive` : `host ${host} is dead`;
      console.log(msg);
      resolve(isAlive);
    });
  });
}

async function hostping(hosts) {
  console.log("hosts-----------", hosts);
  for(const host of hosts) {
    // eslint-disable-next-line no-loop-func
    // eslint-disable-next-line no-await-in-loop
    const isAlive = await probe(host);
    if (isAlive === "false") {
      hoststatus = 1;
    }
  }
  return hoststatus;
}

edited Feb 10 '20 at 12:57

answered Feb 10 '20 at 12:49

Ashish Modi

7,529
2
20
35

you can use await inside a forEach with an `async` callback - it just doesn't work like you think – Jaromanda X Feb 10 '20 at 12:50
can you show me an example or documentation which says async/promises etc works inside forEach? – Ashish Modi Feb 10 '20 at 12:53
define "works" ... of course it "works", it just doesn't "work" as you would expect ... saying `forEach doesn't support promises` is the wrong way of looking at it, since you can quite easily use promises in a forEach loop - of course, most instances of using promises in a forEach loop would work better in a map – Jaromanda X Feb 10 '20 at 12:54
`ping.sys.probe(host, async function (isAlive) {` oh dear ... why is there an async there? – Jaromanda X Feb 10 '20 at 12:55
`var ping = require('ping'); var hoststatus var hosts = ['google.com']; async function test(){ var connected = await hostping(hosts); console.log('connected--------------', connected) } async function hostping(hosts) { console.log('hosts-----------', hosts) for (const host of hosts) { await ping.sys.probe(host, async function(isAlive) { var msg = isAlive ? 'host ' + host + ' is alive' : 'host ' + host + ' is dead'; console.log(msg); if (isAlive == 'false') { hoststatus = 1 } }); } return hoststatus; } test()` – karthik110885 Feb 10 '20 at 12:56
If it doesn't work the way you expect it to work. I would say it doesn't work. It's all about terminology. I dont see anything wrong in that. – Ashish Modi Feb 10 '20 at 12:56
but you can use promises in a forEach loop - but you're better off using a map - e.g. document.querySelectorAll has a forEach method but not a map – Jaromanda X Feb 10 '20 at 12:57
@AshishModi - i tried this but still getting the same error – karthik110885 Feb 10 '20 at 12:57
@karthik110885 I removed async which was a typo. pls try the updated answer. – Ashish Modi Feb 10 '20 at 12:58
@karthik110885 - use the other answer - it is correct – Jaromanda X Feb 10 '20 at 12:58
@JaromandaX - i am new to node js and still not sure on how to use async properly. Would be really helpful for me if you can point with an example. – karthik110885 Feb 10 '20 at 12:58
async/await is syntax sugar for promises ... you await something that is/returns a promise - it's all about knowing what a function returns (.forEach returns `undefined` so there's no point awaiting it) – Jaromanda X Feb 10 '20 at 12:59
@JaromandaX - see this https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach hope it explains. – Ashish Modi Feb 10 '20 at 13:01
@AshishModi - thank you very much for helping with the issue. – karthik110885 Feb 10 '20 at 13:05
@JaromandaX - i think i understand now. So, i have to understand if a function returns something to use await. thank you very much – karthik110885 Feb 10 '20 at 13:06
@karthik110885 - a function needs to return not just something ... a Promise ... only a Promise makes sense with await (though you can await a non promise, but that makes no sense) – Jaromanda X Feb 10 '20 at 13:32

ksankar · Accepted Answer · 2020-02-11T04:00:13.543

To do what you want, you should use the promise wrapper of ping and also for..of because forEach doesn't work with async.

The hostping function has to return a promise that test will wait for.
Use for of inside to loop over the hosts
use the promise wrapper of ping and wait for probe result
After all hosts are checked resolve the overall status

Below is modification of your code to get the flow working as you expected:

const ping = require('ping');
const hosts = ['google.com', 'yahoo.com'];

async function test()
{
  const status = await hostping(hosts);

    if (status) {
        console.log('All hosts are alive');
    } else {
        console.log('Some/all hosts are dead');
    }

}

function hostping(hosts) {
    let status = true;
    return new Promise(async function(resolve, reject) {
      for (const host of hosts) {
        const res = await ping.promise.probe(host);
        var msg = `host ${host} is ${res.alive ? 'alive' : 'dead'}`;
        console.log(msg);
        if (!res.alive) { status = false; }
      }
        console.log('all host checks are done, resolving status');  
        resolve(status);
    });
}

test()

If you're interested in understand more about async, these are good posts to read: https://www.coreycleary.me/why-does-async-await-in-a-foreach-not-actually-await/

https://itnext.io/javascript-promises-and-async-await-as-fast-as-possible-d7c8c8ff0abc

hi, thanks for the help. But this always returns the status as false even if all the servers are alive `host google.com is alive host 192.168.xx.xxx is alive all host checks are done, resolving status status----------- false Some/all hosts are dead` — karthik110885, Feb 10 '20 at 13:37
changing if(res.alive) to if (!res.alive) { status = false; } works like a charm — karthik110885, Feb 10 '20 at 13:54
async Promise constructor executor - usually means there's a far better way to write the code — Jaromanda X, Feb 11 '20 at 22:03

Waiting for an function to complete before executing the next step - node js

3 Answers3