Proper way to run asynchronous function in a Promise

Question

I am making a test app using systeminformation. I'm trying to make it so that each then waits for the previous function to finish. The problem I'm having is that the functions I am running inside are also promises, so the next then runs before the function finishes.

const si = require('systeminformation');

var cpuObj;

function initCPU() {

  return new Promise(resolve => {

    si.cpu()
      .then(data => cpuObj = data)
      .catch(err => console.log(err))
    .then(() => {
      setTimeout(() => console.log("timer"), 3000);
    })
    .then(() => {
      si.cpuTemperature().then(data => console.log(data));
    })
    .then(() => {
      console.log("here");
    });
  });
}

function test() {
  console.log(cpuObj);
}

initCPU().then(() => {
  test();
});

Output:

here
{ main: -1, cores: [], max: -1 }
timer

Expected Output:

{ main: -1, cores: [], max: -1 }
timer
here

Don't fall for the [Promise constructor antipattern](https://stackoverflow.com/q/23803743/1541563)! — Patrick Roberts, Apr 22 '19 at 16:15

Patrick Roberts · Answer 1 · 2019-04-23T16:06:00.813

A few points that need to be addressed:

setTimeout() does not return a promise, so you need to promisify and return it.
Flatten your chain by returning the promises from within each of the continuations rather than attempting to chain continuations within other continuations (i.e. then() inside of then()).
Do not wrap the continuation chain with a promise constructor, as the chain itself is already a promise, just return it directly instead. This is considered an antipattern.
Do not use globals, because it makes the initCPU() no longer re-entrant safe. Multiple calls to initCPU() before the promise returned by the first call resolves will result in unexpected behavior otherwise. Instead, use the appropriate scope to pass values along, which in this case is the function itself.
Allow errors to propagate to the caller and let the caller decide how to handle the error. Do not handle errors from within initCPU() unless you expect to use a fallback and continue to provide meaningful data to the caller.

const si = require('systeminformation');
const delay = ms => new Promise(resolve => { setTimeout(resolve, ms); });

function initCPU() {
  // use local scope, not global
  let cpuObj;

  // return this promise chain directly
  return si.cpu()
    .then(data => {
      cpuObj = data;
      // return the promise to the chain
      return delay(3000);
    })
    // let caller handle errors
    // .catch(err => console.log(err))
    // flatten your chain
    .then(() => {
      console.log('timer');
      // return the promise to the chain
      return si.cpuTemperature();
    })
    // flatten your chain
    .then(data => {
      console.log(data);
      console.log('here');
      // pass data to caller
      return cpuObj;
    });
}

function test(cpuObj) {
  // received from last continuation of initCPU()
  console.log(cpuObj);
}

initCPU()
  .then(test)
  // handle error from caller
  .catch(err => {
    console.log(err);
  });

If you just want to query the cpu object immediately, and query cpuTemperature after 3 seconds, I'd do something like this using Promise.all():

// default to 3 seconds, allow it to be configurable
function initCPU(ms = 3000) {
  return Promise.all([
    si.cpu(),
    delay(ms).then(() => si.cpuTemperature())
  ]).then(([cpu, cpuTemperature]) => ({
    cpu,
    cpuTemperature
  }));
}

function test (obj) {
  console.log(obj.cpu);
  console.log(obj.cpuTemperature);
}

initCPU()
  .then(test)
  .catch(err => {
    console.log(err);
  });

Proper way to run asynchronous function in a Promise

1 Answers1