How to flatten a Promise within a Promise?

Question

I have the following 2 functions, each returns a Promise:

const getToken = () => {
  return new Promise((resolve, reject) => {
    fs.readFile('token.txt', (err, data) => {
      if (err) { return reject(err) }
      if (!tokenIsExpired(data.token)) {
        return resolve(data.token)
      } else {
        return requestNewToken()
      }
    })
  })
}

const requestNewToken = () => {
  return new Promise((resolve, reject) => {
    restClient.get(url, (data, res) => {
      fs.writeFile('tokenfile.txt', data.token, err => {
        resolve(data.token)
      })
    })
  })
}

function1()
  .then(value => {
    console.log('taco')
  })
  .catch(err => {
    console.log(err)
  })

So function1 runs, and (depending on some condition), it sometimes returns function2, which is returning another Promise. In this code, when function2 is called, the console.log('taco') never runs. Why is this? I thought that if you return a Promise from within a Promise, the resolved value of the nested Promise is what is resolved at the top level.

In order for me to get this to work, I have to do this:

const getToken = () => {
  return new Promise((resolve, reject) => {
    if (!tokenIsExpired()) {
      return resolve(getToken())
    } else {
      return requestNewToken ()
        .then(value => {
          resolve(value)
        })
    }
  })
}

That works, but it seems like I'm doing something wrong. It seems like there should be a more elegant way to handle/structure this.

Answer 1

You're right that promises auto-unwrap, but in this case you're returning from inside a promise constructor, which is ignored, you need to invoke either resolve or reject instead of using return. I think this might be the solution you're looking for:

const function1 = () => {
  return new Promise((resolve, reject) => {
    if (someCondition) {
      resolve('foobar')
    } else {
      resolve(function2());
    }
  })
}

Inside a promise constructor, you need to call resolve or reject, which are equivalent to using return or throw from inside a then callback.

If you find this distinction confusing (I do), you should avoid the promise constructor entirely by just beginning a new chain with Promise.resolve, like this:

const function1 = () => {
  return Promise.resolve().then(() => {
    if (someCondition) {
      return 'foobar';
    } else {
      return function2();
    }
  })
}

const function2 = () => {
  return new Promise((resolve, reject) => {
    resolve('hello world')
  })
}
someCondition = false;
function1()
  .then(value => {
    console.log(value)
  })

With your updated code, I recommend using a library to wrap APIs, rather than accessing the promise constructor yourself. For example, using bluebird's promisify:

const bluebird = require('bluebird');
const readFile = bluebird.promisify(fs.readFile);
const writeFile = bluebird.promisify(fs.writeFile);
const getUrl = bluebird.promisify(restClient.get, {multiArgs:true});

const getToken = () => {
  return readFile('token.txt')
    .then((data) => {
      if(!tokenIsExpired(data.token)) {
        return data.token;
      } else {
        return requestNewToken();
      }
    });
};

const requestNewToken = () => {
    return getUrl(url)
      .then(([data, res]) => {
        return writeFile('tokenFile.txt', data.token)
          .then(() => data.token);
      });
};

I've remained faithful to your source code, but I'll note there may be a bug to do with writing data.token, and later trying to read the token property in that file.

Why use a library instead of the Promise constructor?

• It allows you to write code which deals only with promises, which is (hopefully) easier to understand
• You can be confident that callback APIs are correctly converted without losing errors. For example, if your tokenIsExpired function throws an error, using your promise constructor code, it would be lost. You would need to wrap all of your inner callback code in try {} catch(e) {reject(e)}, which is a hassle and easily forgotten.