JS: Recursively calling a promise function

Question

I am making a node.js application that can create thumbnails for images. To avoid freezing the application while generating thumbnails I decided to use an asynchronous library for creating thumbnails. However depending on the image multiple thumbnail sizes might be needed.

var thumbnailSizes = [100];
if (image.type == 'coolImage') thumbnailSizes.push(500);
generateThumbnails(image.filename, thumbnailSizes).then(function() {
    // Do cool things with the saved thumbnails (This is never reached)
});

function generateThumbnails(filename, thumbnailSizes) {
    return new Promise(resolve => {
        var path = filename.substring(0, filename.lastIndexOf('\\'));
        console.log('start');
        console.log('length = ' + thumbnailSizes.length);
        thumb({
            prefix: thumbnailSizes[0] + '_';
            source: filename,
            destination: path,
            width: thumbnailSizes[0]
        }).then(function () {
            if (thumbnailSizes.length > 1) {
                console.log('next');
                generateThumbnails(filename, thumbnailSizes.splice(0, 1));
            } else {
                console.log('finished');
                resolve('true');
            }
        }).catch(function (e) {
            console.log('error');
        });
        console.log('end');
    });
}

This code successfully creates the first thumbnail but not the second. This is what my console looks like after the code stops running.

> Console Output
start
length = 2
end
next
start
length = 1
end

The code calls generateThumbnails() for a second time successfully but does not call the thumb function again, skipping to the end and never resolving. How can I make this work?

Answer 1

I don't see the need for recursion here.

async function generateThumbnails(filename, thumbnailSizes) {
  var path = filename.substring(0, filename.lastIndexOf('\\'));

  return await Promise.all(thumbnailSizes.map(size => thumb({
    prefix: `${size}_`,
    source: filename,
    destination: path,
    width: size
  })));
}

Or if you need to create the thumbnails one by one:

async function* generateThumbnails(filename, thumbnailSizes) {
  var path = filename.substring(0, filename.lastIndexOf('\\'));
  for(const size of thumbnailSizes) {
    yield await thumb({
      prefix: `${size}_`,
      source: filename,
      destination: path,
      width: size
    });
  }
}

Which is consumable with a for await loop in the calling function:

for await(const thumbnail of generateThumbnails(file, sizes) {
  // handle single size
}

Also, I wouldn't use .substring() to make path manipulation, I'm sure the Node path module has a function or seven that can help you reliably extract the interesting part from the path.

Answer 2

You are only calling resolve in the else block of your condition in the callback, not in the if block. Resolving the promise that the recursive call returns won't have any effect on the promise returned by the outer call. Also you never reject the promise in case of an error.

Anyway, you should avoid the Promise constructor antipattern, so that you don't need any resolve calls at all but can simply return from the then callbacks to chain promises:

function generateThumbnails(filename, thumbnailSizes) {
    console.log('length = ' + thumbnailSizes.length);
    if (thumbnailSizes.length == 0) {
        console.log('finished');
        return 'true'; // are you sure?
    } else {
        var path = filename.substring(0, filename.lastIndexOf('\\'));
        return thumb({
//      ^^^^^^
            prefix: thumbnailSizes[0] + '_';
            source: filename,
            destination: path,
            width: thumbnailSizes[0]
        }).then(function () {
            console.log('next');
            return generateThumbnails(filename, thumbnailSizes.slice(1));
//          ^^^^^^
        })
    }
}

…
var thumbnailSizes = [100];
if (image.type == 'coolImage') thumbnailSizes.push(500);
console.log('start');
generateThumbnails(image.filename, thumbnailSizes).then(function() {
    console.log('end');
    // Do cool things with the saved thumbnails (This is never reached)
}, function(err) {
    console.log('error', err);
});

Also I fixed your recursion - the base case should be the empty array.

Answer 3

it seems you are resolving your promise with another promise, which might cause the promise chain to be broken, you can try changing:

resolve(generateThumbnails(filename, thumbnailSizes.splice(0, 1)));

for

return generateThumbnails(filename, thumbnailSizes.splice(0, 1))

However my suggestion would be (if you are using the latest ES versions) to use async/await then you don't need a recursive call and you code will be more readable:

// your function definition
//async () => {

var thumbnailSizes = [100];
if (image.type == 'coolImage') thumbnailSizes.push(500);

for(const size of thumbnailSizes) { // do not use a foreach
 const thumbnail = await generateThumbnail(image.fineName, size);
 // Do more fun stuff with your thumbnail
}

});

function generateThumbnail(filename, size) {
        var path = filename.substring(0, filename.lastIndexOf('\\'));

        return thumb({
            prefix: size + '_';
            source: filename,
            destination: path,
            width: size
        })
}