Make executing the same query object twice throw an error

[vkarpov15]

This has caused numerous issues, I'll list them out here as I find them:

#5683 (comment)

Here's a couple examples of queries that will execute twice in non-obvious ways. The below examples use findOne(), but you can imagine this being much worse if you use updateOne() with non-idempotent operators.

await MyModel.findOne({}, function(err, res) {});

mySchema.pre('save', function() {
  return MyModel.findOne({}, function(err, res) {});
});

MyModel.findOne({}, function(err, res) {}).exec();

Most of these problems come from mixing callbacks and promises, we may add a warning for that.

For 6.x, we should consider making it so that executing a query multiple times gives you the same result back immediately, unless you clone() the query beforehand.

[freewil]

Just ran into this issue myself with CoffeeScript. CoffeeScript uses implicit returns, so it can be really easy to accidentally cause multiple query executions by use of a callback, and accidentally returning a mongoose.Query, causing implicit execution of the Query object's then() with promise chaining or use of await.

Here's a more simple example:

// use of callback here will cause execution of query
const query = MyModel.find({}, callback);

// Will cause a duplicate query execution.
// Promise chaining or use of `await` will also
// implicitly call `then()`.
query.then();

[freewil]

CoffeeScript mayhem:

doQuery = (done) ->
  # Use of callback here will cause execution of query.
  #
  # CoffeeScript will implicitly return mongoose.Query object
  MyModel.find({}, done)

Promise.resolve()
  .then ->
      # CoffeeScript will implicitly return mongoose.Query object.
      #
      # Returning mongoose.Query object here in a `then()` callback will 
      # cause implicit call to `then()` on the mongoose.Query object, causing
      # a duplicate query execution.
    doQuery next
  .catch next

[freewil]

For 6.x, we should consider making it so that executing a query multiple times gives you the same result back immediately, unless you clone() the query beforehand.

I agree with this - I also don't think a query object should be returned if you use a callback. If you really still need the query object for some reason you can omit the callback to get the query object and then pass the callback to .exec()

Since Query is a Thenable, it essentially encourages mixing of promises and callbacks.

[vkarpov15]

You're right about coffeescript making this messy. I haven't used coffeescript in a long time so I forgot about this behavior. But then again you'd get a similar issue with ES6 arrow functions. We'll prioritize this change for 6.0. Can't make this change in 5.x because it would be backwards breaking.