0

Undefined values in array after promises:

I'm using Q to manage Promises with Node.js and I'm using easyImage to process images.

This block of code work fine, it load, save, cut images from tmp folder and paste it in the user folder. The only problem I have is to save the final data to the DB. I get undefined values within array...

exports.postAccountImages = function(req, res, next) {
  User.findById(req.user.id, function(err, user) {
    var path = __dirname + '/../public/images/u/' + user._id + '/';
    var max = 800;
    fs.exists(path, function(exists) {
      if (!exists) fs.mkdirSync(path);
    });

    var promises = [];
    for (var key in req.files) {
      if (req.files.hasOwnProperty(key)) {
        (function(file) {
          q().then(function() {
            return promises.push(easyimg.info(file.path).then(function(image) {
              easyimg.resize({
                src: file.path,
                dst: path + file.name,
                width: (image.width >= max) ? max : image.width,
                height: (image.height >= max) ? max : image.height,
                quality: 80
              }).then(function(image) {
                fs.remove('./tmp-uploads/' + image.name);
                return {
                  src: image.name,
                  main: false
                };
              });
            }));
          });
        })(req.files[key]);
      }
    }

    q.all(promises).then(function(result) {
      console.log(result); // [undefined, undefined, undefined, ...]
      // Here I should push result to the DB
    });
  });
};
Ayeye Brazo
  • 3,316
  • 7
  • 34
  • 67
  • 2
    You need to do some learning about both promises and asynchronous callbacks. This answer might help you: http://stackoverflow.com/questions/14220321/how-to-return-the-response-from-an-ajax-call. In general, you must "use" the async result IN the async callback, not anywhere else. – jfriend00 Dec 03 '14 at 22:27
  • I'm sure I have to learn about promises but from the link you posted I don't understand exactly what should I do with it. It explain something with jQuery... I'm using Node with Express... the question is: How can I get the return? – Ayeye Brazo Dec 03 '14 at 22:31
  • can I have an example based on my problem please? – Ayeye Brazo Dec 03 '14 at 22:52
  • You would have to show what you're trying to do with the returned image size. That code needs to go inside the callback. – jfriend00 Dec 03 '14 at 23:02
  • I added the missing/final part. It should process more that one image per time... multiple image upload. That's why there is a `count`, all this is inside a `for` as well. – Ayeye Brazo Dec 03 '14 at 23:12
  • If this is inside a `for` loop to process multiple images, you will need to include that code too as you can't finish your response until all the images are done processing which leads to a different way of coding it so you can gather all the async results and then process the response. Dealing with multiple async results requires a different type of coding from the top level down. It can't be just one independent function. That function has to be used differently. Likewise, you can't call `next()` multiple times either. – jfriend00 Dec 03 '14 at 23:15
  • Full code has been attached. Thanks – Ayeye Brazo Dec 03 '14 at 23:25
  • I see several different issues. I'm working on a rewrite. – jfriend00 Dec 04 '14 at 00:24
  • Why `q().then()` - that seems like there's no reason to have that at all. Just do `promises.push()` outside of that like in my answer. I also modified my answer to add a return statement on the first line after the `.push()` which is needed to get the result back. You will need to add that too. – jfriend00 Dec 04 '14 at 22:05

1 Answers1

1

Here's a general idea for how to do this (untested):

var promises = [];
for (var key in req.files) {
    if (req.files.hasOwnProperty(key)) {
        (function(file) {
            promises.push(easyimg.info(file.path).then(function(image) {
                return easyimg.resize({
                    src: file.path,
                    dst: path + file.name,
                    width: Math.max(image.width, 800),
                    height: Math.max(image,height, 800),
                    quality: 80
                }).then(function(image) {
                    fs.remove('./tmp-uploads/' + image.name, function(err) {
                        if (err) {
                            // logging error, but not stopping execution
                            // since this is a non-fatal error
                            console.log("err removing temp upload: ", err);
                        }
                    });
                    return {src: file.name, main: false};
                });
            }));
        })(req.files[key]);
    }
}
// now wait for all promises to finish
// assumes you want to call next() no matter what when all image processing is done
Promise.all(promises).then(function(results) {
    // all results are in the results array here
    // do whatever processing of the results array you want to do here
    res.sendStatus(200);
    next();
}, function() {
    // set some status to send when there's an error
    res.sendStatus(xxx);
    next();
});

I corrected several problems and made some improvements:

  1. Your variable file was being overwritten whenever processing more than one file because you were trying to use the same variable in multiple async callbacks. I put it into a closure so it's saved separately for each image that is being processed. This is probably the main issue that kept it from working with more than one image.

  2. You weren't calling next() at all the right times (sometimes calling it too many times).

  3. Your error handling had multiple issues because you can't just return out of an async method to stop processing.

  4. I decided that if you can't remove the temporary file that processing should continue rather than abort since that isn't a fatal issue.

  5. This code uses Promise.all() to get a callback when all your operations are done rather than using a manual counter. This also makes error handling much simpler.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Thanks, it helped me a lot managing the whole process and I see I need to learn promises, I'm still learning and I never faced on it. I edited my original Question, Everything works fine, I used Q to manage your Promise. But at the moment as you can read I still have a final issue the q.all return an array with no data inside... that data is the final data I should push to the DB. If you could explain this as well I'd really appreciate. Thanks. – Ayeye Brazo Dec 04 '14 at 12:26
  • @AyeyeBrazo - I added a necessary return statement on the line after the `.push()`. That is necessary to get the return value back appropriately. – jfriend00 Dec 04 '14 at 22:06
  • Promise is undefined. – Ayeye Brazo Dec 05 '14 at 13:51
  • Using `Q()` rather than `Promise` worked, I'm still using the old current version of Node and it doesn't support `Promise` yet. – Ayeye Brazo Dec 05 '14 at 13:59