Async Promise chaining with a loop in Node.js & MongoDB

Question

I recently imported my Database from Sql Server to MongoDB, converting to a full MERN stack. I have a titles_table, a skills_table, and a title_skills_table. The title_skills_table is a 'join' table that holds the _ids of the skills and title tables as some skills can fall under multiple titles.

I'm trying to call all title_skills associated with a particular title, which I've been able to do. What I can't figure out is how to use that title_skills array response to find all the skills by skill_id in the skills_table.

I've tried async waterfall, async chaining, among other things, but it's either just returning an empty error message or breaking the server.

I can do the two calls individually and get a response from each, it's joining them in a looping chain that seems to kill me.

Here's what I currently have:

const router = require('express').Router();
const Skill = require('../models/Skill');
const TitleSkills = require('../models/TitleSkills');

//Get A Skill by Skill Id
router.get("/skill/:id", async (req, res) => {
  try {
    const skill = await Skill.findOne({skill_id: req.params.id}).populate('SkillCategory')
    res.json(skill);
  } catch (err) {
    res.json({ message: err });
  }
});

//Get TitleSkills by TitleId *by itself
// router.get("/title/:id", async (req, res) => {
//   try {
//     const titleSkills = await TitleSkills.find({title_id: req.params.id})
//     res.json(titleSkills)
//   } catch (err) {
//     res.json({ message: err });
//   }
// });

//Get a skill by skill id and through it in an array
const getSkillsByskillId = (id) => new Promise((resolve, reject) => {
  const skillsArr = []
  router.get(`/skill/${id}`)
    .then((result) => resolve(skillsArr.push(result.data)))
    return(skillsArr)
    .catch(error => reject(error))
});

//Get all TitleSkills by TitleId and loop each TitleSkill through Skills Table
router.get("/title/:id", async (req, res) => {
  try {
    const titleSkills = await TitleSkills.find({title_id: req.params.id})
      .then(titleSkills.forEach((ts) => {
       getSkillsByskillId(ts.skill_id)
      }))
    res.json(titleSkills)
  } catch (err) {
    res.json({ message: err });
  }
});

module.exports = router;

#saveme

danh · Accepted Answer · 2021-01-15 17:29:21Z

1

This is equivalent to the OP function of the same name...

const getSkillsByskillId = async (id) => {
  const result = await Skill.findOne({skill_id: req.params.id}).populate('SkillCategory')
  return result.data;  // removed OP's extra wrap of the array
}

Call it from the route, by mapping over it, collecting promises, and running them together with Promise.all()...

router.get("/title/:id", async (req, res) => {
  try {
    const skills = await TitleSkills.find({title_id: req.params.id})
    let promises = skills.map(ts => getSkillsByskillId(ts.skill_id));
    let titleSkills = await Promise.all(promises);
    res.json(titleSkills)
  } catch (err) {
    res.json({ message: err });
  }
});

edited Jan 15, 2021 at 17:29

answered Jan 15, 2021 at 0:49

danh

62.7k10 gold badges100 silver badges139 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Evan G Over a year ago

danh... so close!!! I fiddled with it a bit and got it to work. Two things: 1) I used the mongoose findOne() method in getSkillsByskillId function instead of the get() method: const result = await Skill.findOne({skill_id: id}).populate('SkillCategory'). the get() method was returning the nulls. 1b) Using the findOne() method only requires me to return result No array necessary 2) I had to change let titleSkills = await Promise.all(promises); because titleSkills is already declared for the find() method.

danh Over a year ago

@EvanG - glad it is working. I've edited in the correct endpoint and syntax. The problem in the OP was conceptual regarding the async operations on an array. I probably should have made the answer more conceptual, rather than leave the impression that one could just copy paste and run.

terrymorse · Accepted Answer · 2021-01-15 22:55:05Z

1

This promise chain is ill-formed:

//Get a skill by skill id and through it in an array
const getSkillsByskillId = (id) => new Promise((resolve, reject) => {
  const skillsArr = [];
  router.get(`/skill/${id}`)
    .then((result) => resolve(skillsArr.push(result.data)))
  return (skillsArr)
    .catch(error => reject(error)); // <- never executed
});

That return (skillsArr) executes immediately after router.get is called, returning an empty array.

This modified code returns a promise that resolves to skillsArr (there's no need to include a catch block that doesn't do anything besides reject):

//Get a skill by skill id and return it in an array
const getSkillsByskillId = (id) => {
  const skillsArr = [];
  return router.get(`/skill/${id}`)
    .then((result) => {
      skillsArr.push(result.data);
      return skillsArr;
    });
};

Also, this call to getSkillsByskillId ignores the resolve:

    getSkillsByskillId(ts.skill_id)
   }))

This probably ought to be something like this:

getSkillsByskillId(ts.skill_id)
  .then(skillsArr => doSomething() );

Finally, this block is combining await and Promise chain unnecessarily.

const titleSkills = await TitleSkills.find({title_id: req.params.id})
  .then(titleSkills.forEach((ts) => {
    getSkillsByskillId(ts.skill_id);
  }))

This is equivalent:

const titleSkills = await TitleSkills.find({title_id: req.params.id});
for (let ts of titleSkills) {
  const skillsArr = await getSkillsByskillId(ts.skill_id);
  // do something with skillsArr
});

edited Jan 15, 2021 at 22:55

answered Jan 14, 2021 at 18:25

terrymorse

7,1561 gold badge23 silver badges31 bronze badges

6 Comments

Evan G Over a year ago

hey terrymorse, thanks for answering. I've tried your suggestions several different ways and I still can't seem to get it to work. Mostly it's giving me an error in the console: ` const skillsArr = await getSkillsByskillId(ts.skill_id); ^^^^^ SyntaxError: await is only valid in async function `

terrymorse Over a year ago

@EvanG Oh, I see my problem. You can only use await directly inside an async function. I've fixed it by replacing the .forEach() with a simple for (...) block.

danh Over a year ago

This answer still needlessly instantiates a promise in getSkillsBySkillId. get returns a promise, so there's no need to create one.

danh Over a year ago

It also suggests unnecessarily that the async operations run on the results of the find be performed in sequence, with an await in a the for loop.

terrymorse Over a year ago

@danh So getSkillsBySkillId could just do return router.get().then().catch() ? I see your point about the sequential finds, but it could be that the sequence order is significant.

|

Evan G · Accepted Answer · 2021-01-15 02:29:33Z

0

//Get A skill by skill_id
const getSkillsByskillId = async (id) => {
  const result = await Skill.findOne({skill_id: id}).populate('SkillCategory')
  return result;
}


//Get all TitleSkills by TitleId and then Map each TitleSkill to Skills_Table
router.get("/title/:id", async (req, res) => {
  try {
    const titleSkills = await TitleSkills.find({title_id: req.params.id})
    let promises = titleSkills.map(ts => getSkillsByskillId(ts.skill_id));
    let skills = await Promise.all(promises);
    res.json(skills)
  } catch (err) {
    res.json({ message: err });
  }
});

edited Jan 15, 2021 at 2:29

answered Jan 15, 2021 at 2:14

Evan G

134 bronze badges

Collectives™ on Stack Overflow

Async Promise chaining with a loop in Node.js & MongoDB

3 Answers 3

2 Comments

6 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

6 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related