CODEWITHSUNDEEP
javascriptnode.jspromise
Toby Boulton
Toby Boulton

Reputation: 190

Promise resolving too early

I'm having an issue where my Promise.all is resolving too early. For a test I want to console.log the length of the array which is getting pushed from within the promise map but it is returning 0 sadly. I'm sure it's something simple...

fromPath(job.data.path, { density: 100, format: "png" }).bulk(-1, true).then(output => {
  Promise.all(output.map(page => 
    jimp.read(Buffer.from(page.base64, 'base64')).then(img => 
      {
        img.invert().getBase64Async(jimp.AUTO).then(data => imageArray.push(data.replace('data:image/png;base64,', ''))).catch(err => console.log(err))
      }
      ).catch(err => console.log(err))
      )).catch(err => console.log(err))
}
  // This returns early
).then(console.log(imageArray.length)).then(done()).catch(err => console.log(err));

Any help would be greatly appreciated.

Upvotes: 0

Views: 471

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074285

There are a lot of issues there. Mainly they fall into these categories:

  • Not returning the results of promise chains from fulfillment handlers, which means the chain the fulfillment handler is in won't be linked up to the promise chain you created within it
  • Calling functions and passing their return value into then, rather than passing a function into then

See inline comments in this minimal, least-changes reworking of that code:

fromPath(job.data.path, { density: 100, format: "png" }).bulk(-1, true)
.then(output => {
    // Return the result of `Promise.all`
    return Promise.all(
        output.map(
            page => jimp.read(Buffer.from(page.base64, 'base64'))
                .then(img => {
                    // Return the ersult of this promise chain
                    return img.invert().getBase64Async(jimp.AUTO)
                    .then(data => imageArray.push(data.replace('data:image/png;base64,', '')))
                    // This error handler does nothing useful, since we now return
                    // the promise chain
                    // .catch(err => console.log(err))
                })
                // Recommend **not** handling errors at this level, but using
                // `Promise.allSettled` instead
                .catch(err => console.log(err))
        )
    )
    // This will only be reached if a *synchronous* error occurs calling (for instance)
    // `getBase64Async`, since you've already caught errors above
    .catch(err => console.log(err))
})
// Use a function here, don't call `console.log` directly
.then(() => console.log(imageArray.length))
//    ^^^^^^
// Pass `done` directly to `then`, don't call it and pass its return value
// (e.g., remove the `()`)
.then(done) // Or `.then(() => done)` if you want `done` called with no arguments
.catch(err => console.log(err));

FWIW, though, if I had to not use async functions I'd probably do it like this:

fromPath(job.data.path, { density: 100, format: "png" }).bulk(-1, true)
.then(output => Promise.allSettled(
    output.map(
        page => jimp.read(Buffer.from(page.base64, 'base64'))
            .then(img => img.invert().getBase64Async(jimp.AUTO))
            .then(data => {
                imageArray.push(data.replace('data:image/png;base64,', ''));
            })
    )
))
.then(results => {
    // Show errors
    const errors = results.filter(({status}) => status === "rejected");
    for (const {reason} of errors) {
        console.error(reason);
    }
    // Done
    done();
});

Upvotes: 1

Related Questions