AWS S3 / Javascript callback issue

Question

So, I'm having a problem with JavaScript asynchronous execution when making an API call to AWS S3.

I have a sequence of nested callbacks that are working fine up until a specific S3 call that my code is not waiting for. Here's my code:

getThumbUrls(contentIndex, function(data) {
  console.log('Returning from getThumbUrls');
  // let's just display thumbUrls[0] for now...
  console.log('The thumbUrls are ' + data[0]);
});

getThumbUrls() looks like this:

function getThumbUrls(contentIndex, callback) {
  console.log('Entering getThumbUrls');

  var thumbUrls = [];

  JSON.parse(contentIndex).forEach(videoKey => {
    // get the thumbnail: bucket-name/thumbnails/<first-key>
    console.log('videoKey = ' + videoKey);

    getThumbFileName(videoKey, function(thumbFileName) {
      console.log('Returning from getThumbFileName');
      console.log('Returned thumb filename is ' + thumbFileName);

      thumbUrls.push(CLOUDFRONT_URL + videoKey + '/thumbnails/' + thumbFileName);

    });

  });

  callback(thumbUrls);
}

And getThumbFileName() looks like this:

function getThumbFileName(videoKey, callback) {
  console.log('Entering getThumbFileName...');

  const s3 = new AWS.S3({
    apiVersion: '2006-03-01',
    params: {
      Bucket: 'my-bucket-name'
    }
  });

  // Get the name of the file.
  params = {
    Bucket: 'my-bucket-name',
    Delimiter: '/',
    Prefix: videoKey + '/' + THUMBS_FOLDER,
    MaxKeys: 1
  };

  var urlKey;
  //console.log('listObjects params = ' + JSON.stringify(params, null, 4));
  s3.listObjectsV2(params, (err, data) => {
    if (err) {
      console.log(err, err.stack);
      callback(err);
      return;
    }

    var thumbsKey = data.Contents;
    // MaxKeys was 1 bc first thumbnail key is good enough for now. Therefore, only one iteration.
    thumbsKey.forEach(function (keys) {
      console.log('thumbKey = ' + keys.Key);
      urlKey = keys.Key;
    });

  });

  callback(urlKey);
  //callback('20161111-TheWind.jpg');
}

Obviously, what's happening is that execution doesn't wait for the s3.listObjectsV2 call to finish. I've verified that the entire flow works properly when all getThumbFileName() does is callback with the filename.

Would someone kindly show me how to force execution to wait for s3.listObjectsV2 to complete before calling back with undefined?

using callbacks with forEach (and any type of iteration) is kinda complicated, why don't you use a promise approach? promises are meant to make our lives easier =) — guijob
– guijob, Commented Jan 16, 2019 at 3:27
I'm almost certain you are correct @guijob... that callbacks are complicating my life here. :) But I'm uncertain as to how to introduce promises here. Would you mind giving me a tip??? :D — motivus
– motivus, Commented Jan 16, 2019 at 15:35
I am currently reading about promises here... seeing as my approach indulges the "callback hell" anti-pattern. haha.. whoops. — motivus
– motivus, Commented Jan 16, 2019 at 18:43
I wrote a response to try help you out how starting using promises into your code — guijob
– guijob, Commented Jan 16, 2019 at 21:59

guijob · Accepted Answer · 2019-01-16 21:55:43Z

2

As discussed, you should avoid callbacks approach when dealing with asynchronous operations over iterations, due their difficulty.

(You can skip this section if you don't want to know motivation behind promises approach).

Just to mention, in a callback approach, you must have to wait for all callbacks to complete in your getThumbUrls(), using a if which will check if all callbacks has been called, then just call callback(thumbUrls); with all responses pushed into your thumbUrls array:

function getThumbUrls(contentIndex, callback) {
  const thumbUrls = [];

  // counter which will increment by one for every callback
  let counter = 0;
  JSON.parse(contentIndex).forEach(videoKey => {
    getThumbFileName(videoKey, function (thumbFileName) {
      thumbUrls.push(CLOUDFRONT_URL + videoKey + '/thumbnails/' + thumbFileName);

      // for each callback response you must add 1 to a counter and then
      counter++;
      // check if all callbacks already has been called
      if (counter === JSON.parse(contentIndex).length) {
        // right here, thumbsUrls are filled with all responses
        callback(thumbUrls);
      }
    });
  });
}

So, you can make use of Promises, and a Promise.all will be enough for you to handle all responses from api. You can study over internet and check your code below, which is using a promise approach. I've added some comments to help you understanding what is happening.

// when using promises, no callbacks is needed
getThumbUrls(contentIndex)
  .then(function (data) {
    console.log('Returning from getThumbUrls');
    // let's just display thumbUrls[0] for now...
    console.log('The thumbUrls are ' + data[0]);

  })

// when using promises, no callbacks is needed
function getThumbUrls(contentIndex) {
  console.log('Entering getThumbUrls');

  // not needed anymore, Promise.all will return all values
  // var thumbUrls = [];

  // Promise.all receives an array of promises and returns to next .then() all results
  // changing forEach to map to return promises to my Promise.all
  return Promise.all(JSON.parse(contentIndex).map(videoKey => {
    console.log('videoKey = ' + videoKey);

    // returning a promise
    return getThumbFileName(videoKey)
      .then(function (thumbFileName) {
        console.log('Returning from getThumbFileName');
        console.log('Returned thumb filename is ' + thumbFileName);

        return CLOUDFRONT_URL + videoKey + '/thumbnails/' + thumbFileName;
      });
  }))
}

// when using promises, no callbacks is needed
function getThumbFileName(videoKey) {
  console.log('Entering getThumbFileName...');

  const s3 = new AWS.S3({
    apiVersion: '2006-03-01',
    params: {
      Bucket: 'my-bucket-name'
    }
  });

  // Get the name of the file.
  params = {
    Bucket: 'my-bucket-name',
    Delimiter: '/',
    Prefix: videoKey + '/' + THUMBS_FOLDER,
    MaxKeys: 1
  };

  // urlKey not need anymore
  // var urlKey;

  // most of AWS functions has a .promise() method which returns a promise instead calling callback funcions
  return s3.listObjectsV2(params).promise()
    .then(function (data) {
      var thumbsKey = data.Contents;
      //if you want to return only first one thumbsKey:
      return thumbsKey[0];
    })
    .catch(function (err) {
      console.log(err, err.stack);
      callback(err);
      return;
    })
}

Hope this helps you out in your study.

answered Jan 16, 2019 at 21:55

guijob

4,4883 gold badges23 silver badges41 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

motivus Over a year ago

It helps me out tremendously. Not only in understanding where and why I was going wrong in my "callback hell" approach, but also helps my developing understanding of using promises. Thanks!! Your callback tip worked beautifully. Next, I'm going to reimplement using promises. I'll report back on how it goes. May take me a day.

motivus Over a year ago

Well, I'm VERY excited by what I've learned about Promises; even more so because I started with the terrible nested callback approach. Your solution using promises works perfectly; only issue is that s3.listObjectsV2 needs to return thumbsKey[0].Key. Also, my getThumbFileName returns more than just the filename: it returns the prefix plus the thumbnail filename. So, that's been modified as well. You're a super teacher @guijob. Thank you! I'm going to keep writing promises now :)

guijob Over a year ago

Cool! That's part of js learning curve, once you get confident using Promises then you can learn about async/await keywords, which uses Promises and will make your async code readability much better

Michael - sqlbot · Accepted Answer · 2019-01-16 03:48:27Z

1

Would someone kindly show me how to force execution to wait

That's the wrong question. You are not trying to get execution to "wait," or, at least, you shouldn't be. You just need to call the callback in the right place -- inside the callback from s3.listObjectsV2(), not outside.

function getThumbFileName(videoKey, callback) {
  ...
  s3.listObjectsV2(params, (err, data) => {
    if (err) {
      ...
    }

    var thumbsKey = data.Contents;
    // MaxKeys was 1 bc first thumbnail key is good enough for now. Therefore, only one iteration.
    thumbsKey.forEach(function (keys) {
      console.log('thumbKey = ' + keys.Key);
      urlKey = keys.Key;
    });

    callback(urlKey); // right
  });

  // wrong // callback(urlKey);

}

The way you wrote it, the callback fires after s3.getObjectsV2() begins to run -- not after it finishes (calls its own callback).

answered Jan 16, 2019 at 3:48

Michael - sqlbot

180k28 gold badges388 silver badges458 bronze badges

6 Comments

motivus Over a year ago

Thanks @michael-sqlbot. Unfortunately, the execution remains out of order... though I'm sure you're correct that my callback was in the wrong location given what I want to have happen. My issue is that I'm getting conceptually confused now as to how to have orderly returns from these callbacks.

Michael - sqlbot Over a year ago

Part of what you may be missing is that you don't want to have "returns" at all. That isn't how this is all supposed to work. What a function with a callback actually returns ... is discarded. Sprinkling some additional console.log('...'); in your code should help clarify what happens, when.

motivus Over a year ago

You are right, of course. But I meant to refer to not what it returns, but when it returns. Currently, the call to s3.listObjectsV2 is not completing before the enclosing callback. I've actually removed a ton of console.logs as I verified that everything was working prior to including that s3.listObjectsV2 call. So, I feel I just need to not return until it finishes. Does that make sense?

Michael - sqlbot Over a year ago

I see what you are saying, but "not return until" is not your objective. getThumbUrls should not actually be calling its callback... it actually needs to pass that callback to the next function, which calls it when done. This is why it's called "callback hell." For-Each is particularly messy because when you're working async with callbacks, there is no "not return until" -- that's not an option.

motivus Over a year ago

Okay. Clearly, I need to switch to promise / then / then / catch. I am investigating. Thank you so much for your advice!

|

Collectives™ on Stack Overflow

AWS S3 / Javascript callback issue

2 Answers 2

3 Comments

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related