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

eachAsync not waiting for all promises when parallel and populate is used #8352

Closed
simllll opened this issue Nov 17, 2019 · 3 comments
Closed
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@simllll
Copy link
Contributor

simllll commented Nov 17, 2019

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
when using eachAsync with parallel option set to 10 and having a populate on the query, mongoose returns even before all awaits are finished.

see test script, output:

a
a
a
a
b
DONE <-- this sohuld be defintily after all 'b's.. 
b
b
b
b
b
b
b
b
b

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

(async () => {
	const jobsRes = await DBJobModel.find({})
		.populate('_location _field _type _company', '-searchQueries') // <-- need to populate some stuff, it works when uncommenting this line.
		.lean()
		.limit(10)
		.cursor();

	await jobsRes.eachAsync(
		async jobOrg => {
			console.log('a');
			await new Promise((resolve) => setTimeout(resolve, 1000));
			console.log('b');
		}
	, { parallel: 10 });

	console.log('DONE');
})();

What is the expected behavior?
eachAsync needs to wait for all promises

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
mongoose 5.7.11
node v12.13.0
mongodb 4.2.1

@lineus
Copy link
Collaborator

lineus commented Nov 21, 2019

Here's a repro script for this one:

#!/usr/bin/env node
'use strict';

const mongoose = require('mongoose');
const { Schema, connection} = mongoose;
const DB = '8352';
const URI = `mongodb://localhost:27017/${DB}`;
const OPTS = { useNewUrlParser: true, useUnifiedTopology: true };

const schema = new Schema({
  name: String
});

const otherSchema = new Schema({
  tests: [Schema.Types.ObjectId]
});

const Test = mongoose.model('test', schema);
const Other = mongoose.model('other', otherSchema);

function makeTests() {
  return Array.from({ length: 10 }).map((_, i) => {
    return new Test({
      name: `name${i}`
    });
  });
}

const tests1 = makeTests();
const tests2 = makeTests();

const other1 = new Other({ others: tests1.map(t => t._id) });
const other2 = new Other({ others: tests2.map(t => t._id) });

async function run() {
  await mongoose.connect(URI, OPTS);
  await connection.dropDatabase();
  await Test.create(tests1.concat(tests2));
  await Other.create([other1, other2]);
  const others = await Other.find({})
    .populate('tests')
    .lean()
    .limit(10)
    .cursor();
  await others.eachAsync(
    async () => {
      console.log('a');
      await new Promise((resolve) => setTimeout(resolve, 1000));
      console.log('b');
    }
    , { parallel: 10 });
  console.log('done');
  await connection.close();
}

run();

output:

issues: ./8352.js 
a
a
done
b
b
issues: 

@lineus lineus added the has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue label Nov 21, 2019
@vkarpov15 vkarpov15 added this to the 5.7.14 milestone Dec 1, 2019
@vkarpov15 vkarpov15 added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue labels Dec 4, 2019
vkarpov15 added a commit that referenced this issue Dec 4, 2019
@simllll
Copy link
Contributor Author

simllll commented Dec 5, 2019

Thanks a lot :)

@vkarpov15
Copy link
Collaborator

No worries, thanks for pointing out this issue! Clearly our tests weren't quite rigorous enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Projects
None yet
Development

No branches or pull requests

3 participants