How to avoid nesting promises when handling errors from multiple APIs

user_15675817 0
Viewed 58

I’m working on an application where I need to make requests to two apis. I’m using cognito to handle authentication, and then a lambda that communicates with a database. But, I don’t think that my problem is specific to either of those implementations. It could arise with any two apis.

I’m trying to write the process of signing up a new user. I need to create a new user in cognito so that the new user is able to login, and then I need to create a corresponding user in the database that will store the non-authentication related data for the user. If one of the api requests encounters an error, then I need to delete the item that I created in the other api.

My current implementation is basically this:

const signUpNewUser = (userInfo) => {
  API.post("user", "/user", userInfo)
    .then((response) => {
      return COGNITO.post("user", "/user", response.newUserID);
    })
    .then((res) => {
      //BOTH REQUESTS OCCURED WITH NO ERRORS
    })
    .catch((error) => {
      if (error.origin === "COGNITO_ERROR") {
        //IF DB CHANGES CONFIRMED BUT COGNITO FAILED, DELETE CREATED GUEST IN DB
        return API.delete("guest", "/guest", userInfo);
      } else if (error.origin === "DATABASE_ERROR") {
        //IF DB CHANGES FAILED THEN COGNITO HAS NOT RUN YET, SO DON'T NEED TO DELETE IN THIS CASE
      }
    });
};

This follows the pattern I see demonstrated on the internet. However, I’m having trouble distinguishing cognito errors from database errors. In the above code I sort them by error.origin but they don’t actually have a property that reliably indicates their origin. This problem must be common when working with multiple api’s that you don’t control, but I’m having trouble finding a good solution.

It feels I need to nest promises in this situation. I could nest a catch after API.Post and COGNITO.post, and use that catch to throw a new error that has an origin property. Then that would bubble up and get caught by the final catch that handles all errors. Like this:

const signUpNewUser2 = (userInfo) => {
  API.post("user", "/user", userInfo)
    .catch((err) => {
      let parsedError = err;
      parsedError.origin = "DATABASE_ERROR";
      throw parsedError;
    })
    .then((response) => {
      let newGuestID = response.id;
      return COGNITO.post("user", "/user", newGuestID)
        .then((res) => {
          return res;
        })
        .catch((err) => {
          let parsedError = err;
          parsedError.origin = "COGNITO_ERROR";
          throw parsedError;
        });
    })
    .then((res) => {
      //BOTH REQUESTS OCCURED WITH NO ERRORS
    })
    .catch((error) => {
      if (error.origin === "COGNITO_ERROR") {
        //IF DB CHANGES CONFIRMED BUT COGNITO FAILED, DELETE CREATED GUEST IN DB
        return API.delete("guest", "/guest", guestInfo);
      } else if (error.origin === "DATABASE_ERROR") {
        //IF DB CHANGES FAILED THEN COGNITO HAS NOT RUN YET, SO DON'T NEED TO DELETE IN THIS CASE
      }
    });
};

But everything I've read says you should avoid nesting promises.

Alternatively, I could put API.post and COGNITO.post in separate functions with internal .then .catch statements, and then have those functions return a promise or throw an error with an added property to indicate origin. But I've seen people say that just hides the problem and makes code harder to follow.

The standard patter I see is that you have one catch, towards the end of a .then chain, that knows how to handle multiple kinds of errors. But if you don’t control the APIs you are working with, how can you confidently sort those errors? Is there something basic about the nature of errors in js that I'm missing?

promise rest try-catch javascript
1 Answers

Because you want to make the API calls in serial, this should be pretty easy to manage. All you need to do is do COGNITO.post in a .then after the first API call - no need to insert another .catch in between.

const signUpNewUser2 = (userInfo) => {
    API.post("user", "/user", userInfo)
        .then((response) => {
            let newGuestID = response.id;
            return COGNITO.post("user", "/user", newGuestID)
                .then(handleBothSuccess)
                .catch((err) => {
                    // COGNITO failed
                    return API.delete("guest", "/guest", guestInfo);
                });
        })
        .then((res) => {
            //BOTH REQUESTS OCCURED WITH NO ERRORS
        })
        .catch((error) => {
            // Some error other than COGNITO failing occurred
        });
};

There's nothing wrong with nesting Promises when the control flow you need to implement requires it - or with declaring the .then or .catch functions in separate standalone variables first, which avoids visual nesting.

Alternatively, consider async/await, which may be clearer to follow.

const signUpNewUser2 = async (userInfo) => {
    let newGuestId;
    try {
        newGuestId = await API.post("user", "/user", userInfo);
    } catch (e) {
        // API failed, do something here if you want...
        return;
    }
    let cognitoResponse;
    try {
        cognitoResponse = await COGNITO.post("user", "/user", newGuestID);
    } catch (e) {
        // COGNITO failed
        // If deleting throws, the error will percolate to the caller
        return API.delete("guest", "/guest", guestInfo);
    }
    //BOTH REQUESTS OCCURED WITH NO ERRORS
};
user_95152070
Related