Promises confusion in NodeJS

Question

I'm having a lot of trouble wrapping my head around the asynchronous nature of code execution on NodeJS. I have a simple function to fetch the output of ip a on a Linux machine and parse out the IP Subnet manually. I'd simply like to console.log() the IP Subnet after that's done.

I understand that NodeJS mostly runs asynchronously so I can't expect the logic to be finished before I console.log() the variable. I understand to concept of Callbacks to combat this problem, but I'd prefer to have access to the variable outside the logic loop. I turned to Promises, which seems like a good solution for this, but I think I'm missing something and they're not working the way I expected. Here's my code below:

let subnetString = '';

function getIPLinux() {
  return new Promise((resolve) => {
    const ipOutput = spawn( 'ip', ['a'] );

    ipOutput.stdout.on('data', (data) => {
        String(data).split('\n').forEach( (line) => {

            if ( line.includes('inet') && line.indexOf('inet6') < 0 && line.indexOf('127.0.0.1') < 0 ) {
                const ipSubnet = line.split(' ')[5];
                const ipStringArray = ipSubnet.split('.');
                subnetString = ipStringArray[0] + '.' + ipStringArray[1] + '.' + ipStringArray[2] + '.*';
                console.log('Found subnet at end of if loop: ' + subnetString);
            }

        })
    })

    console.log('Found subnet at end of promise: ' + subnetString);
    resolve();
  })
}

getIPLinux().then( () => {
  console.log('Found subnet after then: ' + subnetString);
});

My output is as follows:

Found subnet at end of promise: 
Found subnet after then: 
Found subnet at end of if loop: 192.168.1.*

Only the last line logged is correct. I'm having trouble wrapping my mind around this non-blocking code execution. I'm open to other methodologies too if I'm coming at this the wrong way.

Have a look at [this](https://stackoverflow.com/q/52747777/1048572) or [that](https://stackoverflow.com/q/46289682/1048572) to get an idea on how to promisify the data stream event emitter — Bergi, Feb 24 '19 at 15:09

score 2 · Accepted Answer · answered Feb 24 '19 at 15:06

2

spawn() is also async. You're using an event callback for the stdout which is good but you're resolving the promise immediately below that without waiting for the input to finish. Try

ipOutput.on('close', () => {
  console.log('Found subnet at end of promise: ' + subnetString);
  resolve(subnetString);
});

At the end of your promise.

https://nodejs.org/api/child_process.html#child_process_event_close

answered Feb 24 '19 at 15:06

Jeff Snider

135
3

That worked! That made a little light bulb go off for me too about the order of execution. I ended up simplifying it a bit further and just put the `resolve();` after `console.log('Found subnet at end of if loop: ' + subnetString);` – Jason Burgett Feb 24 '19 at 15:14
For this that seems fine and unlikely to be a problem. Keep in mind though that there's no guarantee that the **data** event will only occur once and that it will contain what you want. For large output it could be called multiple times. Even if what you want is in the first call of **data**, it could run again after your promise resolves and if you're editing a variable outside the promise like you have in your question, it could change in unpredictable ways. The program you `spawn()` is still running and possibly producing output until you get that **close** event. – Jeff Snider Feb 24 '19 at 15:39
You're right about that. I switched back to using the ipOutput.on('close', ...) as you originally suggested. – Jason Burgett Feb 24 '19 at 16:26

score 1 · Answer 2 · answered Feb 24 '19 at 15:12

You need to resolve your Promise from inside the .on('data', (data) => { // here }) callback
Instead of writing subnetString as a global variable, pass it inside the Promise. Like this resolve(subnetString)
Don't forget to catch if there's any errors. For example if no valid lines have been found
Since you're returning only one subnetString you can use Array.find instead of doing a forEach loop. It take a "validation" callback and returns the item or null if not found

const isValidLine = l => l.includes('inet') && !l.includes('inet6') && !l.includes('127.0.0.1');

const extractSubnet = l => {
  const ipStringArray = l.split(' ')[5].split('.');
  return ipStringArray[0] + '.' + ipStringArray[1] + '.' + ipStringArray[2] + '.*';
}

function getIPLinux() {
  return new Promise((resolve, reject) => {
    const ipOutput = spawn('ip', ['a']);

    ipOutput.stdout.on('data', data => {
      const line = data.split('\n').find(isValidLine);

      // Resolve or Reject your promise here
      if (!line) {
        reject('Line not found');
      } else {
        const subnetString = extractSubnet(line);
        resolve(subnetString);
      }
    });
  });
}

getIPLinux().then(subnetString => {
  console.log('Found subnet after then: ' + subnetString);
});

score 0 · Answer 3 · answered Feb 24 '19 at 15:07

This is a problem similar to this one that wasn't correctly addressed with promises.

resolve is synchronously called on promise construction, a promise resolves earlier than data arrives. A promise is supposed to resolve with data but it doesn't.

Considering that only single data should be processed, it should be:

function getIPLinux() {
  return new Promise((resolve) => {
    const ipOutput = spawn( 'ip', ['a'] );

   const handler = (data) => {
        String(data).split('\n').forEach((line) => {
            if (line.includes('inet') && line.indexOf('inet6') < 0 && line.indexOf('127.0.0.1') < 0) {
                const ipSubnet = line.split(' ')[5];
                const ipStringArray = ipSubnet.split('.');
                const subnetString = ipStringArray[0] + '.' + ipStringArray[1] + '.' + ipStringArray[2] + '.*';
                ipOutput.stdout.off('data', handler);
                resolve(subnetString);
            }
        })
    })

    ipOutput.stdout.on('data', handler);
  })
}

Promises confusion in NodeJS

3 Answers3