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

feedback: tool should return an error in its callback if not connected to Mongo #3698

Closed
jcollum opened this issue Dec 20, 2015 · 8 comments
Closed
Labels
backwards-breaking discussion If you have any thoughts or comments on this issue, please share them!
Milestone

Comments

@jcollum
Copy link

jcollum commented Dec 20, 2015

JS:

var Reference, mongoose;

Reference = require('.././data/schema/reference');

mongoose = require('mongoose');

console.log("querying");

Reference.find().exec(function(err, docs) {
  var doc, i, len, results;
  if (err != null) {
    console.dir(err);
  }
  results = [];
  for (i = 0, len = docs.length; i < len; i++) {
    doc = docs[i];
    results.push(console.dir(doc));
  }
  return results;
});

Output:

$ node .tmp/temp.js
querying
$

I suspect that I'm not connected to the database and that's why I'm not getting any output. That's bad UX though -- if the tool needs me to do X then Y before running a query but I fail to do Y, the tool should tell me that I didn't do Y (or at least try to). As it is, the tool is silently failing.

@vkarpov15
Copy link
Collaborator

Turn off the bufferCommands option if you don't like this behavior.

@jcollum
Copy link
Author

jcollum commented Dec 20, 2015

I don't think that really addresses my issue. Why would it be buffering a command if there is only one command? The whole point is that there is something that the tool needs to happen before it can work. That thing did not happen. The tool simply ignored it and did not return an error. It should return an error when the query is executed if there are no connections present.

Silently failing is always the wrong choice.

@vkarpov15
Copy link
Collaborator

The point is that it isn't a failure, silent or otherwise, if buffering is on. The point of buffering is that you don't have to wait for a connection to be established to start using your models, nor do you have to worry about connection interruptions.

@jcollum
Copy link
Author

jcollum commented Dec 21, 2015

If the tool never connects to the database, the tool just seems to shrug and ignore it. I see what you are saying but you should put a timeout on that initial connection -- if it doesn't connect within 60 seconds (configurable), emit an error of some sort. Buffering is fine but in this situation the tool appears to be doing nothing and be totally ok with that. What if my database URL is wrong, will mongoose just ignore that too? Seems like it might.

@vkarpov15
Copy link
Collaborator

You'll get an error on the connection if your database URL is wrong, but if buffering is on then mongoose will happily buffer operations until you successfully connect. It's meant as some syntactic sugar so you don't have to wait for a connection to be established before doing database operations - if you don't like it, turn it off.

@jcollum
Copy link
Author

jcollum commented Dec 21, 2015

And again I have to say: silently failing is always the wrong choice.

How would I know to turn off something that is on by default and never results in an error?

Again, this is terrible UX. I've offered a suggestion for fixing it. Your answer seems to be that it's fine as it is.

@vkarpov15
Copy link
Collaborator

You make a fair argument, I admit to having been caught by this a couple times as well. There may be a better way to do this, we'll take a look for future releases. However, this behavior can't change by default without bumping 5.0.

@vkarpov15 vkarpov15 reopened this Dec 23, 2015
@vkarpov15 vkarpov15 added discussion If you have any thoughts or comments on this issue, please share them! backwards-breaking labels Dec 23, 2015
@vkarpov15 vkarpov15 added this to the 5.0 milestone Dec 23, 2015
@vkarpov15
Copy link
Collaborator

We have bufferTimeoutMS in more recent versions of Mongoose, so now Mongoose by default will throw an error after 10 seconds if still buffering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-breaking discussion If you have any thoughts or comments on this issue, please share them!
Projects
None yet
Development

No branches or pull requests

2 participants