Refactor a promise cascade with passed results

Question

I'm creating a first draft for a node CLI to run docker stacks and containers.

Every call is made using docker commands, with asynchronous calls and results. So we decided to use promises.

Our main problem now is the famous then cascade, especially when we have to pass values and start spinners/messages along the way.

Here is our main ugly method. I'm trying to figure out how this could be refactored to be more... readable!

upgradeServiceImage({env, stack, service}) {
    if (!env || !stack || !service) {
      throw Error(`You must specify env, stack and service to upgrade a service`);
    }

    let payloadSpinner = ora({
      text: `Getting upgrade payload. Please wait...`
    }).start();

    this.getServiceUpgradePayload({env, stack, service})
      .then((response) => {
        payloadSpinner.succeed(`Upgrade payload retrieved successfuly for service ${stack} > ${service}`);

        let upgradeSpinner = ora({
          text: `Upgrading the service. Please wait...`
        }).start();

        this.upgradeImage(response)
          .then((res) => {
            upgradeSpinner.succeed(`Image upgrade request for service "${service}" was made.`);

            this.checkUpgrade({env, stack, service}).then((res) => {
              let finishUpgradeSpinner = ora({
                text: `Finishing upgrade of service by deleting old container. Please wait...`
              }).start();

              this.finishUpgrade(response).then(() => {
                finishUpgradeSpinner.succeed(`Upgrade is now complete!`);
              })
              .catch((err) => {
                finishUpgradeSpinner.fail(`Finishing upgrade failed. Please see in UI. Err: ${err}`);
              })
            })
            .catch((err) => {
              upgradeSpinner.fail(`${err}`);
            })

          })
          .catch((err) => {
            upgradeSpinner.fail(`Image upgrade failed with error: ${err}`);
          });
      })
      .catch((err) => {
        payloadSpinner.fail(err);
      });
  };

It works perfectly but it's barely readable and modular.

I tried some fixes with Promise.all but I got problems to recover my spinners (loading messages) properly and I wasn't able to pass objects from one promise to another (example: response).

Answer 1

Given code like this:

let payloadSpinner = ora({
  text: 
}).start();

this.getServiceUpgradePayload({env, stack, service})
  .then((response) => {
    payloadSpinner.succeed(`Upgrade payload retrieved successfuly for service ${stack} > ${service}`);
    // start the second task
  })
  .catch((err) => {
    payloadSpinner.fail(err);
  });

you can rewrite it with a helper that manages the spinner for you:

function withSpinner({ task, textOnStart, textOnSuccess, textOnError }) {
  const spinner = ora({
    text: textOnStart,
  })
  return task.then(value => {
    spinner.succeed(textOnSuccess(value));
    return result;
  }, reason => {
    spinner.fail(reason);
    return Promise.reject(reason);
  });
}

upgradeServiceImage({env, stack, service}) {
  if (!env || !stack || !service) {
    throw Error(`You must specify env, stack and service to upgrade a service`);
  }

  withSpinner({
    task: this.getServiceUpgradePayload({env, stack, service}),
    textOnStart: `Getting upgrade payload. Please wait...`,
    textOnSuccess: result => `Upgrade payload retrieved successfuly for service ${stack} > ${service}`,
    textOnError: error => error,
  }).then(result => {
    // start the second task
  })

Then you can re-use the helper to create spinners for next steps.

Note that the promise returned by withSpinner is rejected, so if the first task fails, the second task won't execute.

Here's a working demo:

/* some scaffolding for the purpose of the example */

let spinnerId = 0;

function ora({ text }) {
  let myId = spinnerId++;
  console.log("Spinner", myId, "start", text);
  return {
    succeed(msg) {
      console.log("Spinner", myId, "succeed", msg);
    },
    fail(msg) {
      console.log("Spinner", myId, "fail", msg);
    }
  };
}

function delay(ms, result) {
  return new Promise(resolve => setTimeout(() => resolve(result), ms));
}
function delayFail(ms, reason) {
  return new Promise((resolve, reject) => setTimeout(() => reject(reason), ms));
}

/* scaffolding ends, actual code begins */

function withSpinner({ task, textOnStart, textOnSuccess, textOnError }) {
  const spinner = ora({
    text: textOnStart,
  })
  return task.then(value => {
    spinner.succeed(textOnSuccess(value));
    return value;
  }, reason => {
    spinner.fail(reason);
    return Promise.reject(reason);
  });
}

function upgradeServiceImage() {
  return withSpinner({
    task: delay(500, "THE UPGRADE PAYLOAD"),
    textOnStart: `Getting upgrade payload. Please wait...`,
    textOnSuccess: result => `Upgrade payload retrieved successfuly: ${result}`,
    textOnError: error => error,
  }).then(result => {
    return withSpinner({
      task: delay(800, "upgradeImage"),
      textOnStart: `Upgrading the service. Please wait...`,
      textOnSuccess: result => `Image upgrade request for service was made.`,
      textOnError: error => error,
    });
  }).then(result => {
    return withSpinner({
      task: delayFail(700, "some kind of error"),
      textOnStart: `Checking upgrade`,
      textOnSuccess: result => `Checking upgrade finished`,
      textOnError: error => `CHecking upgrade failed because ${error}`,
    });
  }).then(result => {
    console.log("this won't run anymore because previous step failed");
  }).catch(error => {
    // additionally log the error if you want
    console.error("catch", error);
  });
};

upgradeServiceImage();

Update: This is how the same code would look like using async / await. Only the upgradeServiceImage function has been modified. This approach is more 'flat' and readable. It also makes it straightforward to use previous results in consecutive tasks.

/* some scaffolding for the purpose of the example */

let spinnerId = 0;

function ora({ text }) {
  let myId = spinnerId++;
  console.log("Spinner", myId, "start", text);
  return {
    succeed(msg) {
      console.log("Spinner", myId, "succeed", msg);
    },
    fail(msg) {
      console.log("Spinner", myId, "fail", msg);
    }
  };
}

function delay(ms, result) {
  return new Promise(resolve => setTimeout(() => resolve(result), ms));
}
function delayFail(ms, reason) {
  return new Promise((resolve, reject) => setTimeout(() => reject(reason), ms));
}

/* scaffolding ends, actual code begins */

function withSpinner({ task, textOnStart, textOnSuccess, textOnError }) {
  const spinner = ora({
    text: textOnStart,
  })
  return task.then(value => {
    spinner.succeed(textOnSuccess(value));
    return value;
  }, reason => {
    spinner.fail(reason);
    return Promise.reject(reason);
  });
}

async function upgradeServiceImage() {
  try {
    const upgradeResult = await withSpinner({
      task: delay(500, "THE UPGRADE PAYLOAD"),
      textOnStart: `Getting upgrade payload. Please wait...`,
      textOnSuccess: result => `Upgrade payload retrieved successfuly: ${result}`,
      textOnError: error => error,
    });
    const upgradeImageResult = await withSpinner({
      task: delay(800, "upgradeImage"),
      textOnStart: `Upgrading the service. Please wait...`,
      textOnSuccess: result => `Image upgrade request for service was made.`,
      textOnError: error => error,
    });
    const anotherResult = await withSpinner({
      task: delayFail(700, "some kind of error"),
      textOnStart: `Checking upgrade`,
      textOnSuccess: result => `Checking upgrade finished`,
      textOnError: error => `CHecking upgrade failed because ${error}`,
    });
    console.log("this won't run anymore because previous step failed");
  } catch (error) {
    // additionally log the error if you want
    console.error("catch", error);
  };
};

upgradeServiceImage();