findOneAndUpdate error handling middleware invoked with wrong arguments

[davebaol]

Looks like error is passed as the 1st and the 2nd argument of findOneAndUpdate error handling middleware.
The 2nd argument should be the doc instead. Correct me if I'm wrong, because I'm new to this feature and still experimenting.

Here's a repro test inspired by this blog post:

var Schema = mongoose.Schema;

describe('Error Handling Middleware', function() {

  it('On findOneAndUpdate error and res arguments of handleE11000 should be different', function(done) {
    var schema = new Schema({
      name: {
        type: String,
        unique: true
      }
    });
    
    var handleE11000 = function(error, res, next) {
      assert.equal(error.name, 'MongoError');
      assert.equal(error.code, 11000);
      assert.equal(typeof next, 'function');
      assert.notStrictEqual(error, res);  // This makes the test fail
      next();
    };

    schema.post('findOneAndUpdate', handleE11000);

    var Person = mongoose.model('Person', schema);

    var person1 = { name: 'p1' };
    var person2 = { name: 'p2' };

    Person.create([person1, person2], function(error) {
      // Cause a duplicate key error
      Person.findOneAndUpdate({name: 'p2'}, {$set:{name: 'p1'}}, {runValidators:true}, function(error) {
        done();
      });
    });
  });
});

Software version:
Node 6.10.2
Mongodb 3.4.1
Mongoose 4.9.x , 4.10.x and 4.11.0 are all affected by this issue

[vkarpov15]

Confirmed, here's a repro:

const assert = require('assert');
const mongoose = require('mongoose');

mongoose.connect('mongodb://localhost:27017/test');

var Schema = mongoose.Schema;

describe('Error Handling Middleware', function() {
  before(async function() {
    await mongoose.connection.dropDatabase();
  });

  it('On findOneAndUpdate error and res arguments of handleE11000 should be different', function(done) {
    var schema = new Schema({
      name: {
        type: String,
        unique: true
      }
    });
    
    var handleE11000 = function(error, res, next) {
      assert.equal(error.name, 'MongoError');
      assert.equal(error.code, 11000);
      assert.equal(typeof next, 'function');
      assert.notStrictEqual(error, res);  // This makes the test fail
      next();
    };

    schema.post('findOneAndUpdate', handleE11000);

    var Person = mongoose.model('Person', schema);

    var person1 = { name: 'p1' };
    var person2 = { name: 'p2' };

    Person.on('index', function(error) {
      assert.ifError(error);
      Person.create([person1, person2], function(error) {
        // Cause a duplicate key error
        Person.findOneAndUpdate({name: 'p2'}, {$set:{name: 'p1'}}, {runValidators:true}, function(error) {
          done();
        });
      });
    });
  });
});

[davebaol]

Person.on('index', callback) .... a nice trick to learn. Thanks

[vkarpov15]

So with this fix the 2nd arg will be null, which is more correct. MongoDB doesn't give you the document back if findOneAndUpdate() fails, so there's no way for mongoose to get the document unless we execute an additional findOne() query if findOneAndUpdate() fails, which I don't think we want to do.

[davebaol]

@vkarpov15
I see, sounds reasonable.
This means that the plugin mongoose-beautiful-unique-validation cannot work correctly with findOneAndUpdate.
Any idea how to retrieve the collection when the doc is not available?
See https://github.com/matteodelabre/mongoose-beautiful-unique-validation/blob/master/index.js#L61

[davebaol]

@vkarpov15
Ok, I've come up with a workaround, see https://github.com/davebaol/mongoose-beautiful-unique-validation/blob/fa7db428ba1f930304dcbb2fbf82364bf925ace2/index.js#L86