CODEWITHSUNDEEP
javascriptnode.jses6-promise
Efosa
Efosa

Reputation: 137

Promise.all is returning undefined

When I asynchronously map an array, Promise.all was supposed to make the function wait until all the promises are resolved. However, Promise.all shows as undefined. Here is my code. Please can someone tell me what I am doing wrong? Thanks.

router.get("/vehicle_reports/interior-pictures", auth, async (req, res) => {

  const fileKeysObj = await Report.findOne({ orderId: req.params.jobId }, {
    "vehicleInterior": 1
  })
  const fileKeysArray = fileKeysObj.interior
  console.log("fileKeysArray: ", fileKeysArray);

  //Retrieve the files from S3
  const files = fileKeysArray.map(async (item) => {
    const params = {
      Bucket: process.env.AWS_BUCKET_NAME,
      Key: item.fileKey
    }
    await s3.getObject(params, async (err, data) => {
      if (err) {
        throw new Error()
      }
      else {
        const base64Data = base64Arraybuffer.encode(data.Body)
        const contentType = data.ContentType
        const fileName = item.fileName
        return { base64Data, contentType, fileName }
      }
    })
  })
  console.log( files) //Pending promise
  await Promise.all(files) 
  console.log( files) //Undefined
  res.send(files) //Sends empty array
})

Upvotes: 1

Views: 3652

Answers (2)

slebetman
slebetman

Reputation: 113866

I wish people would stop hyping async/await. The await keyword was designed to work with Promises. And not all asynchronous functions return promises. Lots of APIs (such as S3) use callbacks instead. Also, architectures where you can expect multiple/infinite data return such as servers listening to incoming connection or reading a stream are not good fit for Promises which are basically single-shot. For those EventEmitters are more appropriate.

The async keyword does not convert a function to a Promise. It does return a promise but does not have the ability to convert callback based functions to Promises that can be used with await. For that you need to use the original Promise constructor. Therefore, the correct way to get an array of promises is as follows:

const files = fileKeysArray.map((item) => { /* note: async keyword is useless here */
  const params = {
    Bucket: process.env.AWS_BUCKET_NAME,
    Key: item.fileKey
  }


  // We are returning a Promise, so no need to force it to further
  // return a promise by marking this function as "async" above:

  return new Promise((perfectly_fine, oops_something_went_wrong) => {
    s3.getObject(params, async (err, data) => {
      if (err) {
        // Normally people would name this function "reject"
        // but I'm illustrating that the name is really up to you
        // and is not part of the syntax:

        oops_something_went_wrong(err)
      }
      else {
        const base64Data = base64Arraybuffer.encode(data.Body)
        const contentType = data.ContentType
        const fileName = item.fileName

        // This is how you "return" to a promise:

        perfectly_fine({ base64Data, contentType, fileName })
      }
    })
  })
})

Now you can await the result. But you are using await wrong. The correct way to await is as follows:

let resulting_files = await Promise.all(files);
console.log(resulting_files);

You can also choose to not use await. Instead you can use .then():

Promise.all(files).then(resulting_files => {
  // use resulting_files here:

  console.log(resulting_files);
});

Upvotes: 1

frosty
frosty

Reputation: 21762

Replace the inside of the map call to look like this. 

const params = {
  Bucket: process.env.AWS_BUCKET_NAME,
  Key: item.fileKey
}
return new Promise((res, rej) => {
    s3.getObject(params, async (err, data) => {
      if (err) {
        rej('FAILED');
      } else {
        const base64Data = base64Arraybuffer.encode(data.Body)
        const contentType = data.ContentType
        const fileName = item.fileName
        res( { base64Data, contentType, fileName });
      }
    })

});

Upvotes: 0

Related Questions