Mai Huy Hoàng
Mai Huy Hoàng

Reputation: 3

Promise.then() but functions run async

I'm new to Javascript and doing a crawler, I've created 4 Promise as these

var openConfig = new Promise((resolve, reject) => {
  fs.readFile('./config.json', (err, data) => {
    if (err) throw err;
    config = JSON.parse(data);
    client = new MsTranslator({
      client_id: config.translatorId,
      client_secret: config.translatorSecret
    }, true)
    resolve();
  })
})

var openFile = new Promise((resolve, reject) => {
  console.log('Opening file...')
  fs.readFile('./writing/writing.json', (err, data) => {
    if (err) throw err;
    writing = JSON.parse(data);
    console.log('Done parsing file');
    resolve();
  })
})

var ask = new Promise((resolve, reject) => {
  var rl = readline.createInterface({
    input: process.stdin,
    output: process.stdout
  })
  rl.question('Which lesson do you want to add? ', (ans) => {
    lessonId = ans;
    rl.close();
    resolve();
  })
})

var createLesson = new Promise((resolve, reject) => {
  console.log('Now processing lesson ' + lessonId);
})

then call the first Promise

openConfig
  .then(() => {
    return openFile;
  })
  .then(() => {
    return ask;
  })
  .then(() => {
    return createLesson;
  })

but as I run, the console show

Opening file...
Which lesson do you want to add? Now processing lesson undefined
Done parsing file

which I understood as my promises are wrong and my function run async. Can you help me to fix this?

Thank you.

Upvotes: 0

Views: 128

Answers (2)

Bergi
Bergi

Reputation: 665344

Promises are not "called". In your then chain, you only sequentially await them - but the tasks were already started when you created the promises. If you want to sequence the actions, put them in functions.

Btw, your code contains multiple typical mistakes. Don't use global variables, and always promisify at the lowest possible level:

function openFile(path) {
  return new Promise((resolve, reject) => {
    fs.readFile('./config.json', (err, data) => {
      if (err) reject(err); // never `throw err` in non-promise callbacks!
      else resolve(data);
    });
  });
}
function openJSON(path) {
  return openFile(path).then(JSON.parse);
}
function openConfig(path) {
  return openJSON(path).then(config =>
    new MsTranslator({
      client_id: config.translatorId,
      client_secret: config.translatorSecret
    }, true)
  )
}
function ask(question) {
  return new Promise((resolve, reject) => {
    var rl = readline.createInterface({
      input: process.stdin,
      output: process.stdout
    })
    rl.question(question, ans => {
      rl.close();
      resolve(ans); // always resolve to *something*
    });
  });
}

readConfig('./config.json')
.then(client => {
  console.log('Opening file...')
  return openJSON('./writing/writing.json');
})
.then(writing => {
  console.log('Done parsing file');
  return ask('Which lesson do you want to add? ');
})
.then(lessonId => {
  console.log('Now processing lesson ' + lessonId);
});

Upvotes: 2

tmslnz
tmslnz

Reputation: 1813

Instead of assigning new Promises to vars (these run as soon as created), you should wrap them into functions, which in turn return a new Promise

To help you understand here's a simplified example:

function p1 (data) {
    return new Promise(function (resolve, reject) {
        resolve(Object.assign(data, {a:1}));
    });
}

function p2 (data) {
    return new Promise(function (resolve, reject) {
        resolve(Object.assign(data, {b:2}));
    });
}

function p3 (data) {
    return new Promise(function (resolve, reject) {
        resolve(Object.assign(data, {c:3}));
    });
}

p1({z:0})
.then(p2)
.then(p3)
.then((data)=>console.log(data))

This results in { z: 0, a: 1, b: 2, c: 3 }

See here if you wish to experiment a bit with the above: https://repl.it/DwNB/0


On a separate note, if you are using promises, you should also handle errors in the chain in a final .catch() instead of synchronously throwing midway. That's what the reject callback is for!

Upvotes: 0

Related Questions