NodeJS Promise then not executed

Question

I need some help. It's my first try with promises. Here is my code for the promise:

const deleteUniversRefInTarget = (universName, targetName) => {
  console.log('Appel de deleteUniversRefInTarget')
  const promis = new Promise((resolve, reject) => {
    Target.findOneAndUpdate({ univers: universName, name: targetName },
      (err, target) => {
        console.log('Entrée dans la promesse')
        if (err) {
          reject(err)
        } else {
          if (target === null) {
            reject(TypeError(`Invalid univers'n name ${universName}`))
          } else {  
            if (target.univers.length === 1) {
              resolve('deleteTarget')
            } else {
              target.univers.splice(target.univers.indexOf(universName), 1)
              resolve('dereferencedUnivers')
            }
          }
        }
      })
  })
  return promis
}

I call this promise here :

exports.deleteATarget = (req, res) => {
  deleteUniversRefInTarget(req.params.universName, req.params.targetName)
    .then((response) => {
      console.log('Fin du traitement de la promesse')
      if (response === 'deleteTarget') {
        Target.findOneAndDelete({ name: req.params.targetName, univers: req.params.universName },
          (err, target) => {
            if (err) {
              res.send(err)
            }
            res.json({ message: `Target ${target.name} isn't used in any univers, so we deleted it` })
          })
      } else {
        res.json({ message: `Target ${req.params.targetName} no longer used in ${req.params.universName} univers` })
      }
    })
    .catch((error) => {
      res.send(error)
    })
}

In the console, I can see : Appel de deleteUniversRefInTarget But not Fin du traitement de la promesse

So ... do you know what I'm doing bad ?

Does `Target !== undefined` and `Target.findOneAndUpdate` exists? If not, there is your problem, else try to surround your `Target.findOneAndUpdate` call with a try/catch to know what is the error. — Fefux, Aug 29 '18 at 14:11
Hi Fefux, and thank you for your help. Yes Target is defined, (its a mongoose model for a mongoDB database) and findOneAndUpdate exists too. I'll call with a try/catch and post the answer here. Thank you. — Guzim Glorantha, Aug 29 '18 at 14:20
So I changed my code to use try catch : `exports.deleteATarget = (req, res) => { try { deleteUniversRefInTarget(req.params.universName, req.params.targetName) .then((response) => { console.log('Fin du traitement de la promesse') ... }) .catch((error) => { res.send(error) }) } catch (error) { res.send(error) } }` But with no effect. — Guzim Glorantha, Aug 29 '18 at 14:22
Try to put your try/catch around the call `Target.findOneAndUpdate`, Promise in JS doesn't reject on unhandled exception — Fefux, Aug 29 '18 at 15:34

score 0 · Answer 1 · answered Sep 02 '18 at 13:07

I'm not sure I understood everything, but here is my new code about this anti-pattern : ```

const deleteTargetOrDerefUniversInTarget = (universName, targetName) => {
  const promis = new Promise((resolve, reject) => {
    Target.findOne({ name: targetName, univers: universName })
      .then((target) => {
        if (target === null) {
          reject(TypeError(`Invalid univers'n name ${universName} or target's name ${targetName}`))
        } else if (target.univers.length === 1) {
          resolve({ action: 'deleteTarget', target })
        } else {
          resolve({ action: 'dereferencedUnivers', target })
        }
      })
      .catch((err) => {
        reject(err)
      })
  })
  return promis
}

exports.deleteATarget = (req, res) => {
  deleteTargetOrDerefUniversInTarget(req.params.universName, req.params.targetName)
    .then((response) => {
      if (response.action === 'deleteTarget') {
        Target.findOneAndDelete({ name: response.target.name, univers: req.params.universName })
          .then((target) => {
            res.json({ message: `Target ${target.name} isn't used in any univers, so we deleted it` })
          })
          .catch((err) => {
            res.status(err.status).json(err)
          })
      } else {
        response.target.univers.splice(response.target.univers.indexOf(req.params.universName), 1)
        response.target.save()
        res.json({ message: `Target ${response.target.name} no longer used in ${req.params.universName} univers` })
      }
    })
    .catch((error) => {
      res.send(error)
    })
}

``` In this new code, no more exec call. The first promise just send back an action to perform that the caller manage.

score -1 · Answer 2 · answered Aug 29 '18 at 17:45

-1

Ok, sounds much better when I transform my mongoose query (findOneAndUpdate) to a promise : `

const deleteUniversRefInTarget = (universName, targetName) => {
  console.log('Appel de deleteUniversRefInTarget')
  const promis = new Promise((resolve, reject) => {
    Target.findOneAndUpdate({ univers: universName, name: targetName })
      .exec()
      .then((target) =>{
        console.log('Entrée dans la promesse')
        if (target === null) {
          reject(TypeError(`Invalid univers'n name ${universName}`))
        } else {  
          if (target.univers.length === 1) {
            resolve('deleteTarget')
          } else {
            target.univers.splice(target.univers.indexOf(universName), 1)
            resolve('dereferencedUnivers')
          }
        }
      })
      .catch((err) => {
        reject(err)
      })
  })
  return promis
}

`

And the difference is mainly the .exec() part. I think we can say it's solve ... even if I'm not sure it's the correct way to do ot.

answered Aug 29 '18 at 17:45

Guzim Glorantha

47
7

Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Aug 29 '18 at 19:01
I'm not sure I understand your comment Bergi. Do you mean I'm using this anti-pattern or is it just a general remark not connected to my code ? – Guzim Glorantha Aug 30 '18 at 06:15
Yes, you are using it. You're creating a promise with `.exec()` inside the `new Promise` constructor. – Bergi Aug 30 '18 at 09:53
Ok, as I'm a newbie, it doesn't help a lot as I don't know what to do with this information. What is the correct way to avoid the anti-pattern ? Thanks for your answer. – Guzim Glorantha Aug 30 '18 at 11:31
Have you read the linked question? All the answers there are about exactly that – Bergi Aug 30 '18 at 13:04

NodeJS Promise then not executed

2 Answers2