Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript

Async / await seems not to be functioning as expected.

Of course we all realize that is not the case. I am just not getting it through my skull.

I’m not sure why but this whole async/await issue is eluding me. I have included a github gist because the file it too big to paste in here:

https://gist.github.com/dhawkinson/b40f6936caafe48d7be41551119f9817

Right now I am only concerned with 3 functions in this file:

const getJsonKeys = async () => {
     // code goes here
}
const putEmptyJsonKeys = async () => {
     // code goes here
}
const getJsonData = async () => {
     // code goes here
}

These are at lines 40, 59, 77 respectively.

Here is what is happening:

At line 350 I call getJsonKeys(). The code at line 351 is temporary (it is only there to prove to me that I actually do get jsonKeysList — eventually, after the Timeout. At line 352 I call putEmptyJsonKeys(). The code at line 353 is also temprary until I prove I am actually performing the PUT (I am not, yet). At line 356 the for loop fails because jsonKeysList is undefined when the line executes so the length of jsonKeys cannot be determined.

Unless I am wrong, it is a timing issue brought on because line 356 executes before the Promise.resolve at line 51 has happened. But, I don’t understand that because that whole function is an async/await block, that isn’t supposed to move on until after the Promise.resolve happens, right? And I know it happens, eventually because of the console.log at line 351, which I have copied below.

Fetched [ '5sM5YLnnNMN_1540338527220.json',
  '5sM5YLnnNMN_1540389571029.json',
  '6tN6ZMooONO_1540389269289.json' ]

What am I not getting here? Why is the async/await not resolving in time?

4 Answers

Brendan:

That was absolutely brilliant and so helpful. Here is what I ended up with and it is successful, and doubly so because it raises the scope level of jsonKeysList, the values in the array of which drive the main processing of the app, so it is really nice to have them at a higher (more global) level.

Thanks for the catch on getJsonKeys(). It does not need to pass params in and out and should not have any arguments -- rookie mistake brought on by an incomplete understanding (it was a monkey-see-monkey-do mistake).

The promise-resolve mistake was the only way I could figure out how to expose the data wrapped up in the promise. Your way is much better, more direct and more readable, as well as putting higher in the scope chain. Thank you.

I didn't realize that try () catch could stand alone like that. I have always seen it in a function block and just assumed it was sort of like .then() .catch().

I know about .forEach(), I simply forgot about it. Another rookie mistake.

The res.render() suggestion was a very clarifying suggestion and I probably will try and put some kind of progress indicator on there. Thanks again.

Brendan Whiting
seal-mask
.a{fill-rule:evenodd;}techdegree seal-36
Brendan Whiting
Front End Web Development Techdegree Graduate 84,738 Points

I haven't dug deep enough into the code to fully know what's going on, but I have a few observations:

  • Try and write your functions as pure functions. This is a function that does not cause side effects. It takes in input and returns output, and doesn't affect any variables outside of its scope. It's much easier to test, debug, and reason about your code this way. Right now it looks like you have functions that have side effects of changing global variables, but don't return anything. Returning values is also where a lot of the magic of promises comes in. If you define a function as being an async function, whatever you return from it will be a promise, whether you explicitly return a promise, or you return a simple value, it will wrap that value in a promise. This allows you to compose asynchronous functions together in a way that you make sure everything happens in the order you want it to.
  • It also looks like you're assigning a value to a variable at the same time as passing it in as an argument. This is nonstandard and makes it hard to understand and debug. For example: return Promise.resolve(jsonKeysList = res.keys); This is assigning a new value to the global variable jsonKeysList, and also passing that value into Promise.resolve. (But since you don't return this promise, nothing is listening for the promise to be resolved, the only change is the global variable being changed).
  • If you're going to use the async/await strategy (which I recommend) the way to catch errors is by wrapping code with try/catch blocks just like you can with synchronous code. You're mixing and matching traditional promises and async/await right now a bit, probably better to pick one style and stick with it.

So what I'm suggesting is something like this:

const getJsonKeys = (keysId) => {

    const options = {
        method: 'GET',
        uri: baseURL + keysID,
        headers: { 'User-Agent': 'Request-Promise' },
        json: true // Automatically parses the JSON string in the response
    };
    return rpn(options);
}

Then later on something can wait for the value that's returned from getJsonKeys, which will be a promise:

const doAnotherOperation = async () => {
    const keys = await getJsonKeys();
  // do something with the keys
}

Since I unwrapped the promise with await, it will be the json object, not a promise.

Brendan: I hate to say it but I simply do not understand what you are driving at. I tried your suggestion (I think) and I am further behind than when I started.

const getJsonKeys = async () => {

    const options = {
        method: 'GET',
        uri: baseURL + keysID,
        headers: { 'User-Agent': 'Request-Promise' },
        json: true // Automatically parses the JSON string in the response
    };
    return rpn(options)
};

And below that

async () => {
     jsonKeysList = await getJsonKeys();
     console.log('Fetched', jsonKeysList);
}

What did I do wrong? The last console.log doesn't even execute. I am really struggling with this whole promise resolution thing. What I want is a variable called jsonKeyslist that looks like this:

{
    "keys":
     [
     "5sM5YLnnNMN_1540338527220.json",
     "5sM5YLnnNMN_1540389571029.json",
     "6tN6ZMooONO_1540389269289.json"
     ]
}

That list represents the three json file names that I will go after in my loop.

Brendan Whiting
seal-mask
.a{fill-rule:evenodd;}techdegree seal-36
Brendan Whiting
Front End Web Development Techdegree Graduate 84,738 Points

Try something like this

const nonAnonymousFunction = async () => {
     jsonKeysList = await getJsonKeys();
     console.log('Fetched', jsonKeysList);
}

nonAnonymousFunction();

Thanks: That was really dumb. I know the difference between defining a function and calling a function but I still get caught by it.

Brendan:

I adapted your answer after spending the day doing extensive reading on promises (my head is awhirl but I think I'm getting closer). It isn't so much that I don't "get" promises (although that is part of it). I always get stuck with the resolution of the promise, in other words -- exposing the data. Here is how I solved it, and it works for now (though I may have trouble downstream).

//  GET the list of keys to JSON data files (returns a promise)
const getJsonKeys = async (req, res, next) => {

    const options = {
        method: 'GET',
        uri: baseURL + keysID,
        headers: { 'User-Agent': 'Request-Promise' },
        json: true // Automatically parses the JSON string in the response
    };
    const keyList = await rpn(options)
        .then(function (res) {
            return res.keys
        })
        .catch(function(err){
            console.log(err);
        });
    res = keyList
    return res;
};
//
// lots of intervening code
//
module.exports = (rtProcess) => {
    rtProcess.get('/process', function (req, res, next) {
        res.render('process');

        const resolveKeys = Promise.resolve(getJsonKeys());
        resolveKeys.then((jsonKeysList) => {
            console.log('Retrieve Keys');
            const writeEmptyKey = Promise.resolve(postEmptyJsonKeys()); //  reset the file keys List for next run
            writeEmptyKey.then((emptyKeysList) => {
                console.log('Wrote', emptyKeysList);
            });

            //  loop on the keys -- get and parse JSON, then write SQL
            for ( let i =0; i < jsonKeysList.length; i++ ) {
                courseID = jsonKeysList[i];
                console.log(courseID);
                //
                //   downstream code
                //
            }
            //
            //  more downstream code
            //  
        }); //  end of resolveKeys
    }); // end of process route
};  //  end of module exports

The thing I don't like about this solution is I can't figure out to raise the scope of jsonKeysList. I would rather close the resolve block right after the console.log('Retrieve Keys') and have jsonKeysList available for downstream processing, outside the block. But, since I could not get that to work, I punted and enclosed the downstream code inside the block for the sake of scope. I am hoping it doesn't rise up and bite me downstream.

The Good News is, I did isolate jsonKeysList as a standalone array. I can loop on it for the json data I need.

As you can see, I am still a rookie.

Brendan Whiting
seal-mask
.a{fill-rule:evenodd;}techdegree seal-36
Brendan Whiting
Front End Web Development Techdegree Graduate 84,738 Points

A few observations:

  • When you define the function getJsonKeys, you say it has three parameters: (req, res, next). But later on when you call it, you don’t pass in any arguments: getJsonKeys(). Do you want this function to always make the same request for the same information, thus not need any arguments? Or do you want to pass in some argument to this function to tell it to fetch specific data?

  • Promise.resolve(getJsonKeys()) is actually redundant. getJsonKeys() returns a promise that resolves to the JSON. So wrapping it in Promise.resolve is an unnecessary step.

  • My understanding of the response.render method is that it’s the last thing you do, probably should put it at the end.

  • I still think a worthy goal would be to go all-in on async/await, rather than mixing and matching with the older .then() style. It’s not that big a deal. But if you wanted to do that, here’s how you could refactor:

const getJsonKeys = async () => {
    const options = {
        method: 'GET',
        uri: baseURL + keysID,
        headers: { 'User-Agent': 'Request-Promise' },
        json: true // Automatically parses the JSON string in the response
    };
    return (await rpn(options)).keys;
    /*
        If you wrap an expression you want to resolve in parens, 
        then after it resolves you’ll get the resolved JSON, 
        and you can chain onto it.
     */
    // 
};

module.exports = (rtProcess) => {
    // I’m making this function also an arrow function and also async, to be consistent
    rtProcess.get('/process', async (req, res, next) => {
        const jsonKeysList = await getJsonKeys(); 
        /*
            Refactored this with await, and without the redundant Promise.resolve()
         */
        try {
            await postEmptyJsonKeys();
        } catch(err) {
            console.log(err);
            /*
                Maybe if this method fails you would want to render
                something else back to the user rather than going forward
             */
        }

        /*
            Now because we've used async await, this part of the method is only
            going to be called after everthing else. In your current code, other
            parts of the code weren't waiting for the promises to resolve.
         */

        jsonKeysList.forEach(courseId => {
            /*
                The forEach method is great. 
                It's worth knowing about and I think nicer than for loops
             */
        });

        res.render('process', { /* pass some values to the template here? */ });
        /*
            I'm waiting until the very end to call render and return something to
            the user, which can include some of the work you've done on each key
         */
    });
};