Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking bug: v3.6.12 breaks promise-style queries (no callbacks) #1542

Closed
einhorndesign opened this issue Jun 26, 2013 · 4 comments
Closed

Comments

@einhorndesign
Copy link

Queries such as the one that follows no longer work:

var promise = Widgets.count().exec()

Here is a trace for an example of this failure (forgive the Heroku timestamps).

15:37:24 web.1  | TypeError: undefined is not a function
15:37:24 web.1  |     at /Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/collection.js:587:12
15:37:24 web.1  |     at Cursor.nextObject (/Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/cursor.js:677:5)
15:37:24 web.1  |     at commandHandler (/Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/cursor.js:658:14)
15:37:24 web.1  |     at null.<anonymous> (/Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/db.js:1645:20)
15:37:24 web.1  |     at g (events.js:175:14)
15:37:24 web.1  |     at EventEmitter.emit (events.js:106:17)
15:37:24 web.1  |     at Server.Base._callHandler (/Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/connection/base.js:409:25)
15:37:24 web.1  |     at /Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/connection/server.js:563:20
15:37:24 web.1  |     at MongoReply.parseBody (/Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/responses/mongo_reply.js:131:5)
15:37:24 web.1  |     at null.<anonymous> (/Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/connection/server.js:521:22)
15:37:24 web.1  |     at EventEmitter.emit (events.js:95:17)
15:37:24 web.1  |     at null.<anonymous> (/Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/connection/connection_pool.js:190:13)
15:37:24 web.1  |     at EventEmitter.emit (events.js:98:17)
15:37:24 web.1  |     at Socket.<anonymous> (/Users/dylan/Sites/AndiIrwin/node_modules/mongoose/node_modules/mongodb/lib/mongodb/connection/connection.js:382:22)
15:37:24 web.1  |     at Socket.EventEmitter.emit (events.js:95:17)
15:37:24 web.1  |     at Socket.<anonymous> (_stream_readable.js:710:14)
15:37:24 web.1  |     at Socket.EventEmitter.emit (events.js:92:17)
15:37:24 web.1  |     at emitReadable_ (_stream_readable.js:382:10)
15:37:24 web.1  |     at emitReadable (_stream_readable.js:378:5)
15:37:24 web.1  |     at readableAddChunk (_stream_readable.js:143:7)
15:37:24 web.1  |     at Socket.Readable.push (_stream_readable.js:113:10)
15:37:24 web.1  |     at TCP.onread (net.js:511:21)
15:37:24 web.1  | exited with code 8

I'm not entirely sure what causes this bug, but it seems to be a problem with the mechanism by which Mongoose handles passing callbacks to the native driver when one doesn't exist. The code for exec() shows that it seemingly just resolves the promise when an op isn't passed, which also seems odd. I'd like to look into this more, but I'm lacking in time.

If there's anything further anyone would like me to check, please let me know.

In my testing I found this only to be the case on 3.6.12, 3.6.11 works perfectly fine.

@aheckmann
Copy link
Collaborator

Why are you executing a count without a callback?

@einhorndesign
Copy link
Author

It's not that there isn't a callback, it's that it's not supplied immediately at the point where the query is created. In this case, both the information from a count and from querying itself are necessary (say, for pagination purposes). So, two queries are made, and then the promises are "combined". For example, like this:

var api = module.exports = {}

api.widgets = {
  page: function (req, res) {
    var page = Math.max(req.params.page - 1, 0) || 0 // reindex from 0, api is 1-indexed

    var countP   = Widgets.find().count().exec();
    var widgetsP = Widgets.find()
      .sort('order')
      .skip(page * PAGE_SIZE)
      .limit(PAGE_SIZE)
      .exec();

    Q.all([widgetsP, countP]).spread(function(widgets, count) {
      res.json({
        prevCount: countPieces(count, page - 1)
        nextCount: countPieces(count, page + 1)
        widgets: widgets
      });
    });
  }
}

var countPieces = function(count, pageIdx) {
  if (page < 0) { return 0; }
  return Math.max(0, Math.min(PAGE_SIZE, total - (page * PAGE_SIZE)));
}

The code sample may not be perfect, it's just an attempt to remember off the top of my head a simple use case. In any case, I have often found that one query is not sufficient to retrieve all of the information needed, and this style is far more succinct than chaining queries. It also means I can condense error handling...

@aheckmann
Copy link
Collaborator

Ohhhh ok. Thanks, we'll get a fix out soon.

On Thursday, June 27, 2013, Einhorn Media Group wrote:

It's not that there isn't a callback, it's that it's not supplied
immediately at the point where the query is created. In this case, both the
information from a count and from querying itself are necessary (say, for
pagination purposes). So, two queries are made, and then the promises are
"combined". For example, like this:

var api = module.exports = {}
api.widgets = {
page: function (req, res) {
var page = Math.max(req.params.page - 1, 0) || 0 // reindex from 0, api is 1-indexed

var countP   = Widgets.find().count().exec();
var widgetsP = Widgets.find()
  .sort('order')
  .skip(page * PAGE_SIZE)
  .limit(PAGE_SIZE)
  .exec();

Q.all([widgetsP, countP]).spread(function(widgets, count) {
  res.json({
    prevCount: countPieces(count, page - 1)
    nextCount: countPieces(count, page + 1)
    widgets: widgets
  });
});

}}
var countPieces = function(count, pageIdx) {
if (page < 0) { return 0; }
return Math.max(0, Math.min(PAGE_SIZE, total - (page * PAGE_SIZE)));}

The code sample may not be perfect, it's just an attempt to remember off
the top of my head a simple use case. In any case, I have often found that
one query is not sufficient to retrieve all of the information needed, and
this style is far more succinct than chaining queries. It also means I can
condense error handling...


Reply to this email directly or view it on GitHubhttps://github.com//issues/1542#issuecomment-20132155
.

Aaron
@aaronheckmann https://twitter.com/#!/aaronheckmann

aheckmann added a commit that referenced this issue Jun 27, 2013
@einhorndesign
Copy link
Author

Awesome, thanks 👍

aheckmann added a commit that referenced this issue Jul 26, 2013
aheckmann added a commit to mongodbinc-interns/mongoose that referenced this issue Aug 16, 2013
This was referenced Mar 8, 2021
This was referenced Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants