Make orFail() throw on updateOne() and updateMany() if matchedCount === 0, not modifiedCount === 0

[tudor33sud]

Do you want to request a feature or report a bug?
bug
What is the current behavior?

orFail helper method will throw error on update if document is matched but not changed due to the query nature

No document found for query

  _id: {
    '$in': [
      new ObjectId("614ba802c88dc50018c69782"),
      new ObjectId("6102ea9d87655a0017d72c2c"),
      new ObjectId("61db34127cd9d445a6bc391a")
    ]
  }
}" on model "User"  

This is misleading to figure it out, since document was indeed found.

If the current behavior is a bug, please provide the steps to reproduce.

await User.updateOne(
    {
      _id: userId,
    },
    {
      $set: {
        ['metrics.invites']: 0,
      },
    },
    { timestamps: false },
  ).orFail();

This would throw an error if called on a User object which already has {metrics:{invites:0}};

What is the expected behavior?

I would expect that updateOne statement won't throw error in case matchedCount === 1 in this case, because the query is made in such a way so that if called multiple times will do nothing once user has the same value set.

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
NodeJS V16
"mongoose": "^6.2.8"

[IslandRhythms]

From the docs

// Throws if no document was updated
await Model.updateOne({ foo: 'bar' }, { name: 'test' }).orFail();

I'll mark this as a docs issue as it could be more clear that the update operation needs to update something to not throw.

[vkarpov15]

We made a note about this in the docs. However, we should consider changing this behavior in v7. 2 reasons why:

1. Intuitively it makes more sense for await Model.updateOne({ foo: 'bar' }, { name: 'test' }).orFail(); to succeed if the matched document already has { name: 'test' }
2. It is easier to add an extra check to simulate the current behavior "fail if no documents modified" if "fail if no documents matched" is the default than it is to go the other way and get the "fail if no documents matched" semantics. If we switch, you can always get the old behavior by doing something like const res = await Model.updateOne(filter, update).orFail(); assert.ok(res.nModified);

[tudor33sud]

I think this option totally makes sense, hence my original feedback about the method. Thanks a lot for taking care of it!