Loopback-datasource-juggler: FindOrCreate does not return errors on connector error

[jamesjjk]

Bug or feature request

• [x] Bug

Description of feature (or steps to reproduce if bug)

There is a bug in dao.js that does not produce an error when it should on a connector issue. This yields issues for mongodb in particular.

Reference as to fix:
https://github.com/strongloop/loopback-connector-mongodb/issues/296

Errors are overwritten by Model.notifyObserversOf should insert conditional and return just after findOrCreate callback (please :)

function _findOrCreate(query, data, currentInstance) {
    var modelName = self.modelName;
    function findOrCreateCallback(err, data, created) {
      console.log(!!err); //INSERT HERE: if (err) { return cb(err); }
      var context = {
        Model: Model,
        data: data,
        isNewInstance: created,
        hookState: hookState,
        options: options,
      };
      Model.notifyObserversOf('loaded', context, function(err) {
        if (err) return cb(err);

        var obj, Model = self.lookupModel(data);

Additional information (Node.js version, LoopBack version, etc)

This Error is present in:

• 2.53.0 (one I'm using)
  But also in your 3.0.0

[jamesjjk]

Added PR https://github.com/strongloop/loopback-datasource-juggler/pull/1226

[dhmlau]

Thanks @jamesjjk for creating the PR. Please sign the CLA mentioned in the PR #1226.

[jamesjjk]

CLA signed. sorry super busy!

[rmg]

@jamesjjk I couldn't find your name in the CLA db, but given how busy you seem to be and how trivial the fix is I just went ahead and "fixed this myself" in e9baea77e028b63fce2d0ce3ee405a5eebf8132e.

[jamesjjk]

@rmg Defo signed the CLA, but thanks! I'm happy to PR other fixes. I have a critical fix for the MongoDB atomic upsert issue - mongodb is currently not atomic and may require a client to retry an upsert event multiple times (for another ticket) https://jira.mongodb.org/browse/SERVER-14322 let me know.

[dhmlau]

@jamesjjk , thanks for letting us know. Please open a separate ticket for the requirements above. In this case, you'd know about the progress. Thanks.

[jamesjjk]

@dhmlau Was this fixed in the 2.xx branch?

[jamesjjk]

@dhmlau Fix is not in 2.5.4 not in LTS this isn't ideal

[dhmlau]

@jamesjjk , it's strange. The PR https://github.com/strongloop/loopback-datasource-juggler/pull/1226 is for 2.x branch.
And if you look at https://github.com/strongloop/loopback-datasource-juggler/blob/2.x/lib/dao.js#L1050, your change is here.

I've created a LB2 application, and I can also see the change there too.

[dhmlau]

Since you've opened #1397, let's continue there. Thanks.

[jamesjjk]

@dhmlau @ssh24 Its a shame that this did not land in LTS this fix is critical if you are using mongodb with atomic commits. FYI