Note: Examples are slightly contrived; minor suspension of disbelief is required.

A while ago, my team was experimenting with mongoose’s pre-find hooks. One of the hooks we wrote modified the query conditions before executing it. We wanted to apply a filter at a global level, for example, only return records that have age >= 18 from the persons collection.

Baking this logic inside the call itself seemed like a good idea initially. It saved us from having to modify every existing Model.find() call in the codebase. This also provided a safeguard against devs forgetting to add the condition to every Model.find() call in the future.

"use strict";

const mongoose = require("mongoose");

const personSchema = mongoose.Schema({
  name: String,
  age: Number,
});

function preFindHook(next) {
  if (this._conditions.age >= 18) next();
  else next(new Error("Invalid query"));
}

personSchema.pre("find", preFindHook);
personSchema.pre("findOne", preFindHook);

const PersonModel = mongoose.model("Person", personSchema);
module.exports = PersonModel;

It worked perfectly; until a bunch of PATCH endpoints started to fail randomly.

router.patch('/persons/${personId}', async (req, res, next) => {
  const { personId } = req.params;
  const updatedPersonInfo = req.body;

  const existingPersonRecord = await PersonModel.findOne({
    _id: personId,
    age: { $gte: 18 },
  });

  if (updatedPersonInfo.name) {
    existingPersonRecord.name = updatedPersonInfo.name;
  }

  await existingPersonRecord.save();
  res.json({ person: existingPersonRecord.toObject() });
})

We started searching for the issue using console.log statements in route handlers. It seemed as if our recently registered pre-find hook was being triggered on Model.save() as well.

router.patch('/persons/${personId}', async (req, res, next) => {
  ...
     existingPersonRecord.name = updatedPersonInfo.name;
   }
 
+  console.log('Reached this far without error');
   await existingPersonRecord.save();
   res.json({ person: existingPersonRecord.toObject() });
 })

Reached this far without error
Error: Invalid query
    at model.Query.preFindHook
    at callMiddlewareFunction

We turned on debug mode in mongoose using mongoose.set('debug', true) and removed the pre-find hook to confirm this. In debug mode, mongoose logs every query that it runs.

Mongoose: people.findOne({ _id: ObjectId("6031d9f91973241ccb4848e9"), age: { '$gte': 18 } }, { projection: {} })
Reached this far without error
Mongoose: people.findOne({ _id: ObjectId("6031d9f91973241ccb4848e9") }, { session: undefined, projection: { _id: 1 } })

It was clear that findOne() was executed when save() was called. This findOne() call did not specify an age property in the filter. This made the pre-find hook throw an error. The very idea of mongoose executing a different operation than the one it was asked to was baffling to me. I spent the next hour scouring the web for hints or explanations but found none.

The mystery unraveled when I read the source code of Model.save as a last resort.
The important parts are shown below. Complete source code can be found here.

Model.prototype.$__handleSave = function (options, callback) {
  if (this.isNew) {
    this[modelCollectionSymbol].insertOne(
      obj,
      saveOptions,
      function (err, ret) {
        if (err) {
          _setIsNew(_this, true);

          callback(err, null);
          return;
        }

        callback(null, ret);
      }
    );
  } else {
    const delta = this.$__delta();

    if (delta) {
      this[modelCollectionSymbol].updateOne(
        where,
        delta[1],
        saveOptions,
        (err, ret) => {
          if (err) {
            this.$__undoReset();

            callback(err);
            return;
          }
          ret.$where = where;
          callback(null, ret);
        }
      );
    } else {
      const optionsWithCustomValues = Object.assign({}, options, saveOptions);
      this.constructor
        .exists(this.$__where(), optionsWithCustomValues)
        .then((documentExists) => {
          if (!documentExists) {
            throw new DocumentNotFoundError(
              this.$__where(),
              this.constructor.modelName
            );
          }

          callback();
        })
        .catch(callback);
      return;
    }
  }
};

Mongoose internally tracks whether a document is ’new’ or not. It also tracks the fields that have been updated on the document. This information is used to decide what operation is performed when the .save() method is called on the document.

• When the document is ’new’, mongoose performs an insert.
• When the document is not new, mongoose calculates the ‘delta’.
• If any fields have been modified, mongoose performs an update.
• If no fields have been modified, mongoose checks that the document exists in the database using findOne.

What went wrong

The reason why only PATCH calls failed was that a PATCH operation is a partial update, and there is a possibility that no field is updated. (Think, pressing ‘Save’ in a form without changing anything at all).

Mongoose called findOne() when our code called .save() on the document, and our hook was rejecting the the query for the findOne() operation.

The hook we wrote was flawed as well. _conditions is a private property of 3rd party code. We shouldn’t have messed with it even if JavaScript allows us to.

This behaviour may seem harmless (and sensible) at first. Mongoose was just ensuring that the document existed in the database because we modified nothing at all. But it’s not the operation we wanted to perform.
ORMs are a double-edged sword. Mongoose is a (heavy) abstraction layer over the Mongodb driver. Its operations are not simple wrappers over those of Mongo’s native driver. We should expect it to behave differently.