How to handle recursive promise rejections?

Question

I have a conceptual question concerning async await code : this is a snippet I have in my code:

async function uploadFileBackup(fileId, versionNumber, filePath, sync) {
//...some irrelevant code
 const toReturn = new Promise((accept, reject) => {
    s3.upload(params, async function (err, data) {
      if (err) {
        reject(err)
      } else {
        //here is the relevant code
        file.backupStatus = 'success'
        await file.save()
        accept(data)
      }
    })
  })

  if (sync)
    return toReturn
//...some other irrelevant code
}

Now if the line

await file.save()

(that is happening on a promise success) fails, then my app throws a Unhandled promise rejection and this exactly is my issue.

Also if the passed accept method throws an exception, the promise will have an unhandled rejection.

I could naturally surround the code in the else statement with a try catch block, but that won't help, worse, it will mislead the developer since he will think the error comes from the promise code s3.upload, while that part will have succeeded and the accept code will be the one that failed : which has definitely different consequences :

• a s3 upload failure means the file didn't upload
• but a file.save() failure means only the file status in our db was not updated, but the physical file is in the s3 system

those two errors cannot be handled in the same try catch block, but catching what happens in the accept method is also not clear (about what happened behind the scene). Finally, adding try catch blocks in callback-promise bridges complexifies the code and readability

So what are the best practices in this case

Answer 1

You need to avoid the Promise constructor antipattern! And one must not pass an async function as a normal callback to a function that ignores the return value, and therefore doesn't handle errors in the returned promise.

You should instead promisify s3.upload alone

function upload(params) {
  return new Promise((resolve, reject) => {
    s3.upload(params, (err, data) => {
      if (err) reject(err)
      else resolve(data)
    })
  })
}

and then use it like

async function uploadFileBackup(fileId, versionNumber, filePath, sync) {
  … // some irrelevant code
  if (sync) {
    const result = await upload(params)
    file.backupStatus = 'success'
    await file.save()
    return result
  }
  … // some other irrelevant code
}

Also you should avoid creating toReturn as a promise that is only conditionally awaited (if (sync)) and otherwise forgotten about without handling errors.

If you now want to handle errors from await upload or await file.save you can wrap them in try/catch or do whatever is appropriate. If you don't handle them, they will reject the promise returned by uploadFileBackup and need to be handled by its caller somewhere, but they will not lead to unhandled rejections from stray promises.