1

I've tried but failed in grasping clearly how javascript promises and await work! I somehow managed to cobble together a function that performs what I need in my node.js micro service, but I'm not sure if I'm doing it the right (optimal) way. Also, I achieved what I wanted using promise without await, but also I haven't done any extensive testing of my code to see if it is indeed running exactly the way I think it is. Here is my code that I currently have and works, but I'm not sure if I'm missing using await for proper functioning:

  const QryAllBooks = {
        type: new GraphQLList(BookType),
        args: {},
        resolve(){
              return new Promise((resolve, reject) => {
                 let sql = singleLineString`
                      select distinct t.bookid,t.bookname,t.country
                      from books_tbl t
                      where t.ship_status = 'Not Shipped'
                  `;
                 pool.query(sql, (err, results) => {
                   if(err){
                      reject(err);
                   }
                   resolve(results);

                const str = JSON.stringify(results);
                const json = JSON.parse(str);

                const promises = [];
                for (let p = 0; p < results.length; p++){
                   const book_id = json[p].bookid;
                   const query = `mutation updateShipping
                                  {updateShipping
                                   (id: ${book_id}, input:{
                                      status: "Shipped"
                                   })
                                   { bookid
                                     bookname }}`
                    promises.push(apolloFetch({ query }));
               }

              //I need an await function so that previous apolloFetch  
              //goes in sequence of bookid, one after the other

              Promise.all( promises ).then(( result) => {
                      errorLogger(27, 'Error', result);
                      })
                     .catch(( e ) => {
                         errorLogger( 29, 'Error', e );
                     )};
                      });
                });
            }
          };

       module.exports = {
              QryAllBooks,
              BookType
       };
Roger Dodger
  • 927
  • 2
  • 16
  • 37
  • 2
    Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it) - you should not be doing anything after the call to `resolve` inside the promise executor. Put all that stuff in a `then` callback on the `new Promise`! – Bergi Feb 24 '19 at 17:33
  • 1
    _"but I'm not sure if I'm missing using await for proper functioning:"_ The code is not missing `await` because `resolve` is not an `async` function. The code is missing `.catch()` chained to `.then()` to handle error. – guest271314 Feb 24 '19 at 17:35
  • Bergi/guest271314 - my error again - in trying to type everything I missed the catch block which I have in my live, working function. I'm having trouble grasping the concept of using await along with the promise, so that all the bookids are executed in sequence, instead of letting the promise handle that part. How do I add await to ensure that they're (bookid's) all executed sequentially? – Roger Dodger Feb 24 '19 at 17:55

1 Answers1

1

Avoid the Promise constructor antipattern - you should not be doing anything after the call to resolve inside the promise executor. Put all that stuff in a then callback on the new Promise:

resolve() {
    return new Promise((resolve, reject) => {
        let sql = singleLineString`
            select distinct t.bookid,t.bookname,t.country
            from books_tbl t
            where t.ship_status = 'Not Shipped'
        `;
        pool.query(sql, (err, results) => {
            if(err) reject(err);
            else resolve(results);
        });
    }).then(results => {
        const str = JSON.stringify(results);
        const json = JSON.parse(str);

        const promises = [];
        for (let p = 0; p < results.length; p++){
            const book_id = json[p].bookid;
            const query = `mutation updateShipping {
                updateShipping(id: ${book_id}, input:{
                    status: "Shipped"
                }) { bookid
                     bookname }
                }`;
            promises.push(apolloFetch({ query }));
        }
        return Promise.all(promises);
    }).then(result => {
        errorLogger(27, 'Result', result);
        return result;
    }, err => {
        errorLogger(29, 'Error', err);
        throw err;
    )};
}

You can now replace those then calls with await syntax. And also exchange the Promise.all for a sequential awaiting in the loop:

async resolve() {
   try {
        const results = await new Promise((resolve, reject) => {
//                      ^^^^^
            let sql = singleLineString`
                select distinct t.bookid,t.bookname,t.country
                from books_tbl t
                where t.ship_status = 'Not Shipped'
            `;
            pool.query(sql, (err, results) => {
                if(err) reject(err);
                else resolve(results);
            });
        });
        const promises = results.map(res => {
            const book_id = res.bookid;
            const query = `mutation updateShipping {
                updateShipping(id: ${book_id}, input:{
                    status: "Shipped"
                }) { bookid
                     bookname }
                }`;
            return apolloFetch({ query });
        });
        const result = await Promise.all(promises);
//                     ^^^^^
        errorLogger(27, 'Result', result);
        return result;
    } catch(err) {
        errorLogger(29, 'Error', err);
        throw err;
    }
}

async resolve() {
    const results = await new Promise((resolve, reject) => {
//                  ^^^^^
        let sql = singleLineString`
            select distinct t.bookid,t.bookname,t.country
            from books_tbl t
            where t.ship_status = 'Not Shipped'
        `;
        pool.query(sql, (err, results) => {
            if(err) reject(err);
            else resolve(results);
        });
    });
    const fetches = [];
    for (let p = 0; p < results.length; p++){
        const book_id = results[p].bookid;
        const query = `mutation updateShipping {
            updateShipping(id: ${book_id}, input:{
                status: "Shipped"
            }) { bookid
                 bookname }
            }`;
        fetches.push(await apolloFetch({ query }));
//                   ^^^^^
    }
    return fetches;
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Bergi - I'm not following - I need to wrap all of the functionality into QryAllBooks; also - does the third block of your code replace the 2nd block? I'm confused. – Roger Dodger Feb 24 '19 at 21:08
  • All the three snippets do essentially the same thing. (OK, I omitted the weird logging from the latter two) – Bergi Feb 24 '19 at 21:14
  • Bergi - ok I got it now - but what about await? Wouldn't I need it in order to ensure that the calls are sequentially made? I mean - how would I fit in await in this code? (in your modified code) – Roger Dodger Feb 24 '19 at 21:46
  • The latter two snippets already use `await`. (Btw, `then` also ensures sequencing) – Bergi Feb 24 '19 at 22:24
  • My mistake - apologies! Will try this out and update shortly! – Roger Dodger Feb 24 '19 at 22:32
  • Bergi - I tried your code and the mutation works but when I try to output the values in the graphiql interface in the browser, book_id and name are returning null. Looks like the way graphql works isn't compatible with that code. Also, where do I add my error checking after the await call in order to log errors? – Roger Dodger Feb 25 '19 at 02:28
  • I don't know GraphQL, sorry if I maimed the query syntax when indenting. You can do the error checking by wrapping the whole function body in a `try`/`catch` – Bergi Feb 25 '19 at 10:11
  • Bergi - when you say "wrap the whole function body", could you modify your code and show what you mean? I can't use that code without making it work with graphql. Still trying to debug but graphql is a b when trying to debug. – Roger Dodger Feb 25 '19 at 20:59
  • Edited. It's the same for the third snippet – Bergi Feb 25 '19 at 21:06
  • Bergi - thanks for that. It's not a complete solution for my graphql issue but I accepted it as answer. – Roger Dodger Feb 25 '19 at 21:25