Function work incorrectly when using promise in node js

Question

I have routes like that:

router.get('/:projectid/, (req, res) => {
    testCase.getTestCaseDetail(req.params.projectid, req.params.testcaseid, req.params.snapshotId).then(testcaseData => {
      res.render('testCaseService', {
        title: 'Page',
        testcase: testcaseData,
        layout: 'project_layout',
      });
    });
  });

In the handler function, I have getTestCaseDetail function:

function getTestCaseDetail(projectId, id, snapshotId) {
  let testCaseId = parseInt(id);
  return new Promise(((resolve, reject) => {
    return testCaseSchema.aggregate([
      { $match: { 'projectId': projectId, 'testCaseId': testCaseId } },
      {
        $lookup: {
          from: snapshotInfoSchema.collection.collectionName,
          localField: testCaseObj.SERVICE_ID,
          foreignField: 'artifacts.id',
          as: 'services',
        },
      },
      { $unwind: '$services' },
      {
        $match: {
          'services.snapshot.id': snapshotId,
        }
      }
    ]).then(testCaseResult => {
      resolve(addTestCasesV2(testCaseResult, snapshotId));
    })
      .catch(err => {
        reject(err);
      })
  }));
}

and addTestCasesV2 function

const addTestCasesV2 = function (testcases, snapshotId) {
  const result = [];
  let serviceTypeMapping;
  let serviceName;
  let testCase = {
    id: '',
    testCaseId: '',
    name: '',
    serviceName: '',
    serviceType: '',
    modifiedAt: '',
    testScripts: '',
    snapshotId: '',
    services: '',
    inputs: [],
    outputs: [],
  };
  let promiseInputResults, promiseOutputResults;
  const testcasesList = lodash.map(testcases, (tc) => {
    const artifacts = lodash.map(tc.services.artifacts, (art) => {
      if (art.id === tc.service_id) {
        serviceTypeMapping = art.processType.serviceTypeName;
        serviceName = art.name;
        if (!commonUtil.isUndefined(art.processParameters)) {
          if (!commonUtil.isUndefined(art.processParameters.input)) {
            promiseInputResults = lodash.map(art.processParameters.input, (ip) => {
              let classId = commonUtil.getArtifactId(ip.classId);
              return objectType.getObjectTypeByClassId(snapshotId, classId)
            });
          }

          if (!commonUtil.isUndefined(art.processParameters.output)) {
            promiseOutputResults = lodash.map(art.processParameters.output, (ip) => {
              let classId = commonUtil.getArtifactId(ip.classId);
              return objectType.getObjectTypeByClassId(snapshotId, classId)
            });
          }
        }
        testCase.id = tc.testCaseId;
        testCase.testCaseId = tc.testCaseId;
        testCase.name = tc.name;
        testCase.serviceName = serviceName;
        testCase.serviceType = serviceTypeMapping;
        testCase.modifiedAt = tc.modifiedAt;
        testCase.testScripts = tc.testScripts;
        testCase.snapshotId = snapshotId;
        testCase.services = tc.services;

        Promise.all(promiseInputResults).then(inputItems => {
          return testCase.inputs = inputItems;
        });

        Promise.all(promiseOutputResults).then(outputItems => {
          return testCase.outputs = outputItems;
        });

      }
    });
  });
  return testCase;
};

The inputs/outputs is an list of item, like that: inputs:[ { name: "test1", type: "String" }, { name: "test2", type: "number" }, ]

I have a problem with promise lifecycle, this is the current flow 1. Routes 2. function getTestCaseDetail 3. resolve(addTestCasesV2(testCaseResult, snapshotId)); 4. addTestCasesV2 ==> return testCase but without go to 2 promise.all functions 5. resolve(addTestCasesV2(testCaseResult, snapshotId)); 6. Routes 7. go back 2 promise.all functions 8. end at return testCase.outputs = outputItems;

Please see the image to more detail flow (the white number is current flow, the orange number is my expect flow)

Please advice me. Many thanks.

What does `objectType.getObjectTypeByClassId()` return? Does it return a value or a promise? Is it synchronous or asynchronous? — jfriend00, Jun 27 '18 at 14:46
And, `getTestCaseDetail()` contains a promise anti-pattern. There is no need for wrapping it in a `new Promise()` in that function. Just do `return testCaseSchema.aggregate(...).then(...)`. Return the promise you already have. — jfriend00, Jun 27 '18 at 14:47
Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, Jun 27 '18 at 16:07

dasfdsa · Accepted Answer · 2018-06-27T16:37:10.277

Your code doesn't seem correct. If testcases is an array with more than one item, your lodash.map callback will be called testcases.length time. Each time overwriting testCase.id assigned in previous callback.

Anyways, I have corrected bits of your code to make it in run order that you wanted. I have logged ==step== at various places for your help.

First Function:

function getTestCaseDetail(projectId, id, snapshotId) {
    let testCaseId = parseInt(id);
    return new Promise(((resolve, reject) => {
        return testCaseSchema.aggregate([
            { $match: { 'projectId': projectId, 'testCaseId': testCaseId } },
            {
                $lookup: {
                    from: snapshotInfoSchema.collection.collectionName,
                    localField: testCaseObj.SERVICE_ID,
                    foreignField: 'artifacts.id',
                    as: 'services',
                },
            },
            { $unwind: '$services' },
            {
                $match: {
                    'services.snapshot.id': snapshotId,
                }
            }
        ]).then(testCaseResult => {
            console.log('=======STEP 1=======');
            resolve(addTestCasesV2(testCaseResult, snapshotId));//=======STEP 2=======
            console.log('=======STEP 5=======')

        })
            .catch(err => {
                reject(err);
            })
    }));
}

Second function

const addTestCasesV2 = function (testcases, snapshotId) {
    console.log('=======STEP 2=======')
    const result = [];
    let serviceTypeMapping;
    let serviceName;
    let testCase = {
        id: '',
        testCaseId: '',
        name: '',
        serviceName: '',
        serviceType: '',
        modifiedAt: '',
        testScripts: '',
        snapshotId: '',
        services: '',
        inputs: [],
        outputs: [],
    };
    let promiseInputResults, promiseOutputResults;

    return Promise.resolve()
        .then(()=>{
            console.log('=======STEP 3=======');
            const testcasesList = lodash.map(testcases, (tc) => {
                const artifacts = lodash.map(tc.services.artifacts, (art) => {
                    if (art.id === tc.service_id) {
                        serviceTypeMapping = art.processType.serviceTypeName;
                        serviceName = art.name;
                        if (!commonUtil.isUndefined(art.processParameters)) {
                            if (!commonUtil.isUndefined(art.processParameters.input)) {
                                promiseInputResults = lodash.map(art.processParameters.input, (ip) => {
                                    let classId = commonUtil.getArtifactId(ip.classId);
                                    return objectType.getObjectTypeByClassId(snapshotId, classId)
                                });
                            }

                            if (!commonUtil.isUndefined(art.processParameters.output)) {
                                promiseOutputResults = lodash.map(art.processParameters.output, (ip) => {
                                    let classId = commonUtil.getArtifactId(ip.classId);
                                    return objectType.getObjectTypeByClassId(snapshotId, classId)
                                });
                            }
                        }
                        testCase.id = tc.testCaseId;
                        testCase.testCaseId = tc.testCaseId;
                        testCase.name = tc.name;
                        testCase.serviceName = serviceName;
                        testCase.serviceType = serviceTypeMapping;
                        testCase.modifiedAt = tc.modifiedAt;
                        testCase.testScripts = tc.testScripts;
                        testCase.snapshotId = snapshotId;
                        testCase.services = tc.services;


                        /*=======FOLLOWING IS NOT REQUIRED=======*/
                        // Promise.all([promiseOutputResults]).then(outputItems => {
                        //     return testCase.outputs = outputItems;
                        // });

                    }
                });
            });
            return Promise.all([promiseInputResults,promiseOutputResults]);
        })
        .then(inputItems => {//array of resolved values
            console.log('=======STEP 4=======');
            testCase.inputs = inputItems[0];
            testCase.outputs = inputItems[1];
            return testCase;
        })
};

Now you can use following to extract testcase from first function:

getTestCaseDetail(myProjectId, id, mySnapshotId)
    .then(testCase=>console.log(testCase))

JSfiddle for your understanding.

Thanks for your advice, I have some questions: 1. This block code " .then(inputItems =>...." should put after Promise.all, Is that right? 2. After replace, I try to run, but the result is undefined in console.log(testCase) — Phạm Quốc Bảo, Jun 27 '18 at 15:41
@PhạmQuốcBảo 1. Promise.all([p1,p2) is a function which returns a new promise. This promise resolves to an [v1,v2]. So you chain to this promise using then(), like any other promise. See this answer [here](https://stackoverflow.com/questions/51011826/value-of-promise-all-after-it-is-rejected-shows-promisestatus-resolved/51013221#51013221) for little bit more detail. — dasfdsa, Jun 27 '18 at 15:49
@PhạmQuốcBảo 2.Can you console.log() at step 4 to check what value you are getting from as inputItems ? — dasfdsa, Jun 27 '18 at 15:53
Here is the log number and error: ===STEP 1==== =======STEP 2======= ===STEP 5==== =======STEP 3======= =======STEP 4======= internal/process/warning.js:18 (node:8508) UnhandledPromiseRejectionWarning: TypeError: Cannot read property '0' of undefined — Phạm Quốc Bảo, Jun 27 '18 at 15:57
The error happens because we get item[0] of undefined: inputItems= undefined — Phạm Quốc Bảo, Jun 27 '18 at 15:58
oops...typo. replace Promise.all(...) by Promise.all([promiseInputResults,promiseOutputResults]). I forgot to put promises into array. — dasfdsa, Jun 27 '18 at 15:59
@PhạmQuốcBảo Second function edited to put `return Promise.all()` in right place. Can you try now? — dasfdsa, Jun 27 '18 at 16:10
yeah, it works properly. Thanks for your support, I appreciate that. many thanks. — Phạm Quốc Bảo, Jun 27 '18 at 16:20
but I have a small issue, the inputItems[0] is array promise not array object. it's different with Promise.all('single array') — Phạm Quốc Bảo, Jun 27 '18 at 16:23
`inputItems[0]` and `inputItems[1]` will be the resolved values of `promiseInputResults`, `promiseOutputResults` respectively. — dasfdsa, Jun 27 '18 at 16:26
but Promise.all(promiseInputResults).then(inputItems => { return testCase.inputs = inputItems; }); ==> it returns an array object — Phạm Quốc Bảo, Jun 27 '18 at 16:27
Dont get you. Can you edit in your question to tell what is the resolved value of promiseInputResults and promiseOutputResults . Also what exact value are you getting as inputItems? — dasfdsa, Jun 27 '18 at 16:30
Thanks, @dasfdsa. I try to run with these codes below: return Promise.all(promiseInputResults).then(inputItems => { testCase.inputs = inputItems; }) .then(() => { return Promise.all(promiseOutputResults).then(outputItems => { testCase.outputs = outputItems; return testCase; }); }) ==> I can get list object instead list of promise, Is it correct? — Phạm Quốc Bảo, Jun 27 '18 at 16:41
@PhạmQuốcBảo no. Promise.all() expects array of promise. If there is just one promise, you dont need Promise.all. `promiseInputResults.then(inputItems => { testCase.inputs = inputItems; return promiseOutputResults }).then(outputItems => { testCase.outputs = outputItems; return testCase; }); ` — dasfdsa, Jun 27 '18 at 16:49
@PhạmQuốcBảo My advice is to understand the Promises first. It's a simple topic. Also, if you do so, you won't have to waste so much time debugging and you would be able to code confidently. — dasfdsa, Jun 27 '18 at 16:51

Function work incorrectly when using promise in node js

1 Answers1

Linked