node.js - Combining 2 Promises

Question

I have to create a file upload which stores files in an amazon S3 bucket, and then writes info into a database.

  router.post('/report', upload.single('file'), function(req, res, next) {
    reportService
      .uploadReport(req.body, req.file)
      .then(data => {
        res.send(data);
      })
      .catch(err => {
        res.send(err);
      });
  });

This piece of code handles the call. It triggers the following code.

function uploadReport(report, file) {
  var objectParams = { Bucket: bucketName, Key: file.filename, Body: '' };
  var fs = require('fs');
  var fileStream = fs.createReadStream(file.path);
  fileStream.on('error', function(err) {
    console.log('File Error', err);
  });
  objectParams.Body = fileStream;
  return new Promise((resolve, reject) => {
    s3.putObject(objectParams)
    .promise()
    .then(
      data => {
        resolve(this.addReport(report, data.Location)
          .then(response => resolve(response))
          .catch(err => {
            reject(err);
          })
        );
      },
      err => {
        reject(err);
      }
    );
  });
}

This piece of code uploads the file to my s3 bucket, after the upload is finished, it calls the save report function which writes into the database.

function addReport(report, url) {
  return new Promise((resolve, reject) => {
    reportModel
      .addReport(report, url)
      .then(report => {
        resolve(report);
      })
      .catch(err => {
        reject(err);
      });
  });
}

I know my 2 promises work separately, but combining them into 1 promise doesn't trigger any function.

How can I correctly combine them into 1 promise?

You should read [What is the explicit promise construction antipattern and how do I avoid it?](https://stackoverflow.com/q/23803743/691711). — zero298, Jul 31 '18 at 15:32
Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! `reportModel.addReport` already returns a promise, so the whole `addReport` function could be simplified to `function addReport(report, url) { return reportModel.addReport(report, url); }` or probably dropped altogether — Bergi, Jul 31 '18 at 15:49

zero298 · Accepted Answer · 2018-07-31T15:51:39.047

1

It looks like you are violating the What is the explicit promise construction antipattern and how do I avoid it?. You are resolving a Promise with a Promise. Instead, you want to just return the Promise:

return s3
    .putObject(objectParams)
    .promise()
    .then(data => this.addReport(report, data.location));

edited Jul 31 '18 at 15:51

answered Jul 31 '18 at 15:37

zero298

25,467
10
75
100

1

Wouldn't `s3.putObject(objectParams).promise().then(data => this.addReport(report, data.Location));` be a bit more concise and avoid the Promise constructor entirely? – rossipedia Jul 31 '18 at 15:46
1

@rossipedia Actually since all the `.catch`s are doing is `.reject`ing, I don't think you need them at all. I think you should just pass up the uncaught Promise and let the caller attach the `.catch`s. – zero298 Jul 31 '18 at 15:53

score 0 · Answer 2 · answered Jul 31 '18 at 15:55

0

You will have to return the addReport promise and then resolve/reject the original promise inside the then callback.

answered Jul 31 '18 at 15:55

mr.G

19
3

node.js - Combining 2 Promises

2 Answers2