0

The two pieces of code below throw a type error:

TypeError: Cannot read property 'then' of undefined.

I feel like I'm missing something fundamental. Remarkable is that the log of 'result' in the second piece of code is done after the error is thrown. Which leads me to believe I might be doing something wrong involving asynch. However I cannot get my head around it, even after reading the suggested questions.

Any help would be greatly appreciated!

router.route('/user/:id')

    .put(auth.authRest, function(req, res) {
      userManager.updateUser(req.params.id, req.body)
        .then(function(response) { // line where error is thrown
          res.send({data:response});
        });
    });

and from userManager:

this.updateUser = function(id, data) {
    User.findOne({ _id: id }).exec(function(err, user){
      if(err) {
        console.log(err);
      } else {
        for(let prop in data) {
          user[prop] = data[prop];
        }

        var result = user.save().catch(function(err){
          console.log(err);
        });

        console.log(result); // this log is done below the error, it does contain a promise
        return result;
      } 
    }).catch(function(err){
      console.log(err);
    });

  };
NG.
  • 459
  • 1
  • 6
  • 20
  • 2
    You do not return a `Promise` from `this.updateUser`. Does `exec` return a Promise? – t.niese Feb 28 '17 at 09:49
  • hmm, when logging result it tells me it contains a promise, how come? – NG. Feb 28 '17 at 09:51
  • 1
    The `return result;` belongs to the callback you pass to `exec` and not to the the function you assign to `this.updateUser`. – t.niese Feb 28 '17 at 09:54

2 Answers2

2

If you want to use Promises you need to return a Promise from this.updateUser, the return result belongs to the callback you pass to exec and not to the function you assigned to this.updateUser.

this.updateUser = function(id, data) {
  return User.findOne({
    _id: id
  }).exec().then(function(user) {
    for (let prop in data) {
      user[prop] = data[prop];
    }

    var result = user.save().catch(function(err) {
      console.log(err);
    });

    console.log(result); // this log is done below the error, it does contain a promise
    return result;
  }).catch(function(err) {
    console.log(err);
  });

};

Depending on how you want to do the error handling you could shrink it down to:

this.updateUser = function(id, data) {
  return User.findOne({
    _id: id
  }).exec().then(function(user) {
    for (let prop in data) {
      user[prop] = data[prop];
    }

    return user.save();
  }).catch(function(err) {
    console.log(err);
  });
};
t.niese
  • 39,256
  • 9
  • 74
  • 101
-1

'updateUser' method should return a promise, so that .then call in the first method would work.

Try something like below ( used node package 'q')

this.updateUser = function(id, data) {
    var deferred = Q.defer()
    User.findOne({ _id: id }).exec(function(err, user){
      if(err) {
        console.log(err);
        deferred.reject(err)
      } else {
        for(let prop in data) {
          user[prop] = data[prop];
        }

        var result = user.save().catch(function(err){
          console.log(err);
        });

        console.log(result); // this log is done below the error, it does contain a promise
        deferred.resolve(resolve)
        //return result;
      } 
    }).catch(function(err){
        deferred.reject(err)
      console.log(err);
    });

    return deferred.promise

  };
Muralidharan
  • 452
  • 3
  • 6
  • 1
    http://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it – Ben Fortune Feb 28 '17 at 09:54
  • @BenFortune: The issue is not about which promise pattern to use.. It's about the thing that causes the issue which in this case is ideally that returning promise is missing- "TypeError: Cannot read property 'then' of undefined". – Muralidharan Feb 28 '17 at 09:58
  • You are true about that you answer solves the Problem. But using a pattern every Promise library vendor advises/insist that you should not use anymore, is something that you should avoid. – t.niese Feb 28 '17 at 10:08