Blowing up Mongoose with undefined value in $push

[idibidiart]

@daffl 's own code (from prematurely closed issue) with undefined in place of string for value

My question was: should Feathers Mongoose check for existence of value being pushed onto array by $push or is this something that Mongoose must fix?

const feathers = require('feathers');
const errorHandler = require('feathers-errors/handler');
const rest = require('feathers-rest');
const bodyParser = require('body-parser');
const mongoose = require('mongoose');
const service = require('feathers-mongoose');
const Schema = mongoose.Schema;
const MessageSchema = new Schema({
  text: {
    type: String,
    required: true
  },
  items: [ { type: String } ]
});
const Model = mongoose.model('Message', MessageSchema);

// Tell mongoose to use native promises
// See http://mongoosejs.com/docs/promises.html
mongoose.Promise = global.Promise;

// Connect to your MongoDB instance(s)
mongoose.connect('mongodb://localhost:27017/feathers');

// Create a feathers instance.
const app = feathers()
  // Enable REST services
  .configure(rest())
  // Turn on JSON parser for REST services
  .use(bodyParser.json())
  // Turn on URL-encoded parser for REST services
  .use(bodyParser.urlencoded({extended: true}))
  // Connect to the db, create and register a Feathers service.
  .use('/messages', service({
    Model,
    lean: true
  }))
  .use(errorHandler());

// Start the server.
const port = 3030;
app.listen(port, () => {
    console.log(`Feathers server listening on port ${port}`);
});

// Create a dummy Message
app.service('messages').create({
  text: 'Message created on server',
  items: [ 'a' ]
}).then(function(message) {
  return app.service('messages').update(message._id, {
    $push: { items: undefined }
  });
}).then(message => console.log('Created and pushed message', message));

Output:

Feathers server listening on port 3030
/Users/marcfawzi/c0de/temp1/node_modules/mongoose/lib/services/updateValidators.js:38
          if (castedDoc[keys[i]][_keys[ii]].$each) {
                                           ^

TypeError: Cannot read property '$each' of undefined
    at module.exports (/Users/marcfawzi/c0de/temp1/node_modules/mongoose/lib/services/updateValidators.js:38:44)
    at model.Query.Query._findAndModify (/Users/marcfawzi/c0de/temp1/node_modules/mongoose/lib/query.js:1943:18)
    at model.Query.Query._findOneAndUpdate (/Users/marcfawzi/c0de/temp1/node_modules/mongoose/lib/query.js:1777:8)
    at /Users/marcfawzi/c0de/temp1/node_modules/kareem/index.js:253:8
    at /Users/marcfawzi/c0de/temp1/node_modules/kareem/index.js:18:7
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)

[daffl]

$push is Mongoose/MongoDB specific and not part of the Feathers querying syntax. Feathers database adapters should not do anything other than normalizing the shared querying syntax.

So it should be either fixed in Mongoose or can be verified with a simple hook like this:

app.service('myservice').before({
  update(hook) {
    const push = hook.data.$push;
    if(push) {
      Object.keys(push).forEach(key => {
        if(push[key] === undefined) {
          throw new Error('You can not push undefined');
        }
      });
    }
  }
});

[idibidiart]

trying to wrap my mind around the rationale to make a distinction between Feathers query syntax and the ODM/ORM query syntax in the context of a Feathers wrapper around the ODM/ORM. I've always had wrappers jump thru hoops to make sure the thing being wrapped doesn't blow up with bad input even if it's the responsibility of the thing being wrapped to check for valid input. In other words, trying to gain some wisdom.

EDIT:

Maybe just enforcing a strict separation of responsibility? setting the expectation for Mongoose to fix its own issues? not covering things up? that sort of thing?

[daffl]

Anything we add to cover anything up and make it behave differently than the ORM itself is

• Additional work for us to maintain and document
• Going to cause confusion since using the ORM does not behave as expected.

The bottom line is that Feathers database adapters are just really thin wrappers that provide a common querying syntax. The issue you are describing is definitely a Mongoose bug and Feathers even provides the ability to fairly easily work around it with the hook I posted.

[idibidiart]

I think you're also very correct about this from an abstract point of view. The 'before' hooks is a far better place for the fix than in the adapter. Great. Thanks. Will report to Mongoose. I'm working on reproducing another bug I found with Mongoose.