5

I'm looking for advice on how to chain promises for a "find or create" feature using mongodb/mongoose.

I've currently tried:

userSchema.statics.findByFacebookIdOrCreate = function(facebookId, name, email) {
  var self = this;
  return this.findOne({
    facebookId: facebookId
  }).exec().then(function(user) {
    if (!user) {
      return self.model.create({
        facebookId: facebookId,
        name: name,
        email: email
      }).exec().then(function(user) {
        return user;
      });
    }
    return user;
  });
};

And I call it from my (node/express) API endpoint:

User.model.findByFacebookIdOrCreate(fbRes.id, fbRes.name, fbRes.email)
  .then(function(user) {
    return res.sendStatus(200).send(createTokenForUser(user));
  }, function(err) {
    return res.sendStatus(500).send({
      error: err
    });
  });

Problems are though:

  1. Even though user is null from the findOne query, the create is never called
  2. I'm not sure I'm using the right promise style/most efficient coding style
  3. Am I handling errors correctly eg just at the top level or do I need to do it at every level

Can anyone see what I'm doing wrong, and how I could do it better?

Thanks.

UPDATE

The cause of the problem was that

self.model.create(...)

should have been (no model reference)

self.create(...)

However, I now need to know what I'm doing wrong with the error handling - I could see that an error was occurring, but I couldn't see the cause.

I still have some errors occurring which I know because I get a status of 500

return res.sendStatus(500).send({ error: err });

but the actual error message/detail is empty.

hong4rc
  • 3,999
  • 4
  • 21
  • 40
prule
  • 2,536
  • 31
  • 32
  • 1. Try `return self.model.create(...` 2. When chaining promises, always return either a new promise or the final value. 3. Having a single error function at the end is just fine. – Ivancho Nov 23 '14 at 22:38
  • Thanks, I did originally have that return but it didn't help. Only took it out to see the effect. – prule Nov 23 '14 at 22:40
  • I've edited the question to put the return back in. Any other ideas? – prule Nov 23 '14 at 22:42
  • Well, that's strange. I thought it is invoked but you don't get the result of it because of the missing `return`. – Ivancho Nov 23 '14 at 22:45
  • 1
    I found the main cause of the problem: self.model.create(... should have been self.create(... – prule Nov 23 '14 at 23:33
  • Now I need to find out what I'm doing wrong with the error handling - I should have seen an error that indicated the problem, but even when I had error handling on every 'then()' I still never witnessed the cause being reported... – prule Nov 23 '14 at 23:35
  • @prule and you doesn't need `.exec()`, because `.create()` returns a promise, not a query. – Leonid Beschastny Nov 23 '14 at 23:35
  • Your error handling looks fine... But is the latest express smart enough to accept an object as an argument to `.send()`? Shouldn't it be `.json({error: 'error'})` instead? – Leonid Beschastny Nov 23 '14 at 23:39
  • Leonid - removing the exec() for create() helped! I was getting a strange error although the code had actually inserted okay. Thanks! – prule Nov 24 '14 at 00:10
  • @LeonidBeschastny thanks again - logger.info(err) produced nothing - but logger.info(err.message) gave me what I needed. – prule Nov 24 '14 at 00:12
  • I can't find an easy way to jsonify the whole error object. So for now I've just guessed (successfully) there is a message property on it and return/log just that. I'll post the final code later. – prule Nov 24 '14 at 00:17

2 Answers2

2

The problem could be that:

  1. create method returns a promise and it doens't have method exec
  2. If you want to use then() in your custom method you'll have to return a promise, but you're returning a mongoose document: return user;

This will always returns a promise, it allows you to use then() after your method (You will have to add mpromise module):

userSchema.statics.findByFacebookIdOrCreate = function (facebookId, name, email) {
  var self = this;
  var Promise = require('mpromise');
  var promise = new Promise;
  this.findOne({facebookId: facebookId }).exec()
    .then(function (user) {
        if(user) {
            promise.fulfill(user);
            return;
        }

        self.model.create({ facebookId: facebookId, name: name, email: email })
            .then(function (user) {
                promise.fulfill(user);
                return;
            });
    });
  return promise;
};

Hope this helps you

  • That looks a lot like the [deferred antipattern](http://stackoverflow.com/q/23803743/1048572) – Bergi Mar 26 '15 at 17:36
0

Your question (and the subsequent answer from @EduardoRodríguez) helped me solve a similar problem, so cheers! However, using latest mongoose version + ES6 destructuring and arrow functions, I was able to get it down to something akin to the following:

userSchema.statics.findByFacebookIdOrCreate = function (facebookId, name, email) {
  var User = this;
  return User.findOne({facebookId})
    .then(user => user || User.create({facebookId, name, email}));
};

...and then:

User.findByFacebookIdOrCreate(fbRes.id, fbRes.name, fbRes.email)
  .then(user => res.sendStatus(200).send(createTokenForUser(user))
  .catch(error => res.sendStatus(500).send({error});

Note: this does involve using native ES6 promises by configuring mongoose with:

mongoose.Promise = global.Promise;

I'm still quite new to this, but hopefully this helps others going forward (it works for me).

Daniel Loiterton
  • 5,073
  • 5
  • 33
  • 46