Chaining/Nesting promises

Question

I'm having nightmare trying to chain aws js sdk. Currently I have the following (omitted any parameters to remove chaff, the calls work in isolation so its safe to assume they are correct)

function deploy() {
  return sts.assumeRole().promise()
    .then(() => {
      return new Promise((resolve, reject) => {
        return new Promise((resolve, reject) => {
          AWS.config.update({
            credentials: {
              accessKeyId: data.Credentials.AccessKeyId,
              secretAccessKey: data.Credentials.SecretAccessKey,
              sessionToken: data.Credentials.SessionToken
            }
          });
        resolve();
        })
        .then(() => {
          fsPromise.readFile(packageLocation).then(() => {
            return s3.upload().promise();
          });
        });
    }).then(() => {
      return ebs.createApplicationVersion().promise();
    }).then(() => {
      return ebs.createEnvironment().promise();
    });
};

If I run each then one by one in order it works, however when i run them together the createApplicationVersion call fails as the upload hasn't worked due to the fact it is attempting to upload before assumeRole has finished. I assume...

I am obviously doing something wrong, but It isn't clear to me what.

*"due to the fact it is attempting to upload before `assumeRole` has finished"* No, we can be sure that `assumeRole` is done before anything else above happens. — T.J. Crowder, Jul 15 '19 at 15:59
Does `AWS.config.update()` do its work asynchronously? Does it return a promise? (Or does it have this `.promise()` function on its return that you're using elsewhere?) Because nothing is waiting for that `update` to complete, if it's asynchronous. — T.J. Crowder, Jul 15 '19 at 15:59
@T.J.Crowder AWS.config.update() is setting some local variables, in the full length version of the code I have that wrapped in another promise, updated question — Tom Riley, Jul 15 '19 at 16:03
Don't worry, if it's just setting variables and not doing anything asynchronous, we don't need the details of it. And if it's doing synchronous work, there's no reason to wrap a promise around it. — T.J. Crowder, Jul 15 '19 at 16:05

T.J. Crowder · Accepted Answer · 2019-07-15T16:18:08.477

The use of promises there is much more complicated than necessary. Remember that then (and catch and finally) return new promises that are settled based on what their handlers return. new Promise within a then (or catch or finally) handler is almost never necessary; just return whatever you would resolve that promise with (a value or another promise), and the promise returned by then et. al. will be resolved to it.

That chain should look like this, if I assume AWS.config.update() is synchronous:

function deploy() {
  return sts.assumeRole().promise()
    .then(() => {
        AWS.config.update();
    })
    .then(() => fsPromise.readFile(packageLocation))
    .then(() => s3.upload().promise())
    .then(() => ebs.createApplicationVersion().promise())
    .then(() => ebs.createEnvironment().promise());
}

Each step in that chain will wait for the previous step to complete.

If you don't want deploy to fulfill its promise with the fulfillment value of ebs.createEnvironment().promise(), then add a final then handler:

    .then(() => ebs.createEnvironment().promise())
    .then(() => { });
}

(I'll assume you want to do that for the following examples.)

Or like this if AWS.config.update() is asynchronous and returns a promise:

function deploy() {
  return sts.assumeRole().promise()
    .then(() => AWS.config.update())
    .then(() => fsPromise.readFile(packageLocation))
    .then(() => s3.upload().promise())
    .then(() => ebs.createApplicationVersion().promise())
    .then(() => ebs.createEnvironment().promise())
    .then(() => { });
}

Or, of course, in any relatively recent version of Node.js (I'm guessing from fsPromise and just general context that this is Node.js), you could use an async function:

async function deploy() {
  await sts.assumeRole().promise();
  await AWS.config.update(); // Or without `await` if it is synchronous and doesn't return a promise
  await fsPromise.readFile(packageLocation);
  await s3.upload().promise();
  await ebs.createApplicationVersion().promise();
  await ebs.createEnvironment().promise();
}

Side note about this code, and code like it you posted in a comment:

return new Promise((resolve, reject) => {
  AWS.config.update({
    credentials: {
      accessKeyId: data.Credentials.AccessKeyId,
      secretAccessKey: data.Credentials.SecretAccessKey,
      sessionToken: data.Credentials.SessionToken
    }
  });
  resolve();
})

There's no reason to use new Promise there. It doesn't convert the call to AWS.config.update into an asynchronous call or anything like that. If for some reason you needed a promise there (you don't in this case, AKAICT), you'd use Promise.resolve:

AWS.config.update(/*...*/);
return Promise.resolve();

But again, no need for that here. Remember that promises only provide a way to observe the result of an asynchronous process, they don't make a process asynchronous. (The only thing that they actually make asynchronous is observing the result.)

Awesome answer thanks, one question, I need to pass the response from assumeRole into AWS.config.update, thats why I had it nested within the assume role promise? — Tom Riley, Jul 15 '19 at 16:15
@TomRiley - Glad that helped. No problem passing the result into `update`, just replace that line with ```.then(result => { AWS.config.update(/*...you can use `result` here...*/); })``` — T.J. Crowder, Jul 15 '19 at 16:19
@TomRiley - For the `async` function version: ```const result = await sts.assumeRole().promise();``` then ```AWS.config.update(/*...you can use `result` here...*/);``` — T.J. Crowder, Jul 15 '19 at 16:22

Vitaliy Isikov · Answer 2 · 2019-07-15T16:05:19.293

1

You don't need to nest your promises the way that you did. Taking a stab in the dark, try this:

function deploy() {
  return sts.assumeRole().promise()
    .then(() => { AWS.config.update() })
    .then(() => fsPromise.readFile(packageLocation))
    .then(() => s3.upload().promise())
    .then(() => ebs.createApplicationVersion().promise())
    .then(() => ebs.createEnvironment().promise())
};

edited Jul 15 '19 at 16:05

answered Jul 15 '19 at 16:03

Vitaliy Isikov

3,647
9
38
46

1

There's no reason for the `new Promise` part there. – T.J. Crowder Jul 15 '19 at 16:04

Chaining/Nesting promises

2 Answers2