Correct using multiple promises

Hi,
I have service which work with db a return promise and I use it in other services. My typical situation is: load user from db, then load setting for user.
So first I have to SELECT user from db wait on it and then use the user id to SELECT his setting. And I’m not sure whats the correct way for using multiple functions with promise. I have function initSetting and this function returns promise and I little bit confused where everywhere should I put return Promise.resolve . Could you please review my example and give me help for better way: thank you

initSetting(): Promise {
    return this.userService.loadCurrentUser() // I have to SELECT current user from db
        .then((data) => {
            // when SELECT done I check user was selected          
            if(data.res.rows.length > 0) {
                this.currentLoggedUserId = data.res.rows.item(0).user_id; // get user id from SELECT result
                // SELECT setting by user id from table setting
                return this.loadSetting()
                    .then((data) => {
                        if(data.res.rows.length > 0) {
                            this.setting.period_start = data.res.rows.item(0).period_start;
                        }

                        return Promise.resolve(this.setting);
                    }, (error) => {
                        console.log("DEBUG: error in loadSetting", error);
                        return Promise.resolve(this.setting);
                    });
            } else {
                console.log("DEBUG: no user selected");
                return Promise.resolve(this.setting);
            }
        }, (error) => {
            console.log("DEBUG: error in select user from db", error);
            return Promise.resolve(this.setting);
        });
}

Just a search for promise best practices is all you need, but here’s an article for you: https://60devs.com/best-practices-for-using-promises-in-js.html

I see at least 4 anti patterns here, so, it’s good that you noticed you had an issue. You should read that article, but basically you want something like this (disclaimer I didn’t test this, just quickly wrote it out, may not run as written):

return this.userService.loadCurrentUser()
  .then(data => {
    if (data.res.rows.length === 0) {
      return Promise.reject("No user found")
    }

    const currentLoggedUserId = data.res.rows.item(0).user_id;
    return this.loadSetting(currentLoggedUserId);
  })
  .then(data => {
    if(data.res.rows.length === 0) {
      return Promise.reject("No settings found");
    }

    this.setting.period_start = data.res.rows.item(0).period_start;
    return this.setting;
  })
  .catch(console.log);

Again, just made that up, but the main issues were:

  1. Never nest promises, only chain them, simply return any value from one to chain to the next
  2. Promise wrapping, don’t do extra promise wrapping, so just do return this.setting, never Promise.resolve(this.setting).
  3. Use .catch instead of comma separated success and error callbacks. It’s more readable and can also catch any error in the chain.
  4. Don’t create “global variables” to hold some async value. I create the currentLoggedUserId locally and call the next promise with it. If you actually need currentLoggedUserId on your class for something else, then fine, but if you’re doing it just to hold a value for async calls that’s bad.

I would consider breaking out each step of the promise into private functions to help readability, I didn’t do it because I thought it might be confusing, but you could have something like:

this.userService.loadCurrentUser()
  .then(this.loadSettings)
  .then(this.setPeriodStart)
  .catch(console.log);
4 Likes

Thank you for your time, very helpful for me.

I have last question about Promise.
Everything is working correct but I’m not sure about returning value. My function in written in TS and it’s:

initSetting(): Promise<any> {
      return this.userService.loadCurrentUser()
        .then((data) => {
            if(data.res.rows.length === 0) {
                throw new Error('No user found');
            }
            const currentLoggedUserId = data.res.rows.item(0).user_id;
            return this.loadSetting(currentLoggedUserId);
        })
        .then((data) => {
            if(data.res.rows.length === 0) {
                throw new Error('No settings found');
            }
             
            this.setting.period_start = data.res.rows.item(0).period_start;
            return this.setting;
        })
        .catch(function(error) {
                
        });
}

and this is working but as you can see I declared that function initSetting is returning Promise but when everything is ok it is returning this.setting and that’s object.
So it is any “magic” and it’s wrapped by Promise automaticaly or how is it?
Thank you

It absolutely returns a promise, because this returns a promise, and you are returning it:

When that promise resolves, it will resolve with whatever value is returned at the end of the promise chain. If it truly just returned settings you wouldn’t have to use .then when you call this function.

1 Like