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.

Sign up for GitHub

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

Jump to bottom

throw error after all saves are done #13621

Merged
merged 3 commits into from
Jul 19, 2023
Merged

throw error after all saves are done #13621

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
66c4a8e
throw error after all the saves are done
IslandRhythms Jul 18, 2023
1e3c96f
fix:lint
IslandRhythms Jul 18, 2023
6254c9c
made adjustments
IslandRhythms Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 17 additions & 7 deletions lib/model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2878,24 +2878,34 @@ Model.create = async function create(doc, options) {
}
return res;
} else {
let firstError = null;
res = await Promise.all(args.map(async doc => {
const Model = this.discriminators && doc[discriminatorKey] != null ?
this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) :
this;
if (Model == null) {
throw new MongooseError(`Discriminator "${doc[discriminatorKey]}" not ` +
`found for model "${this.modelName}"`);
`found for model "${this.modelName}"`);
}
let toSave = doc;
try {
let toSave = doc;

if (!(toSave instanceof Model)) {
toSave = new Model(toSave);
}
if (!(toSave instanceof Model)) {
toSave = new Model(toSave);
}

await toSave.$save(options);
await toSave.$save(options);

return toSave;
return toSave;
} catch (err) {
if (!firstError) {
firstError = err;
}
}
}));
if (firstError) {
throw firstError;
}
}


Expand Down
21 changes: 21 additions & 0 deletions test/model.create.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,27 @@ describe('model', function() {
const docs = await Test.find();
assert.equal(docs.length, 5);
});
it('should throw an error only after all the documents have finished saving gh-4628', async function() {
const countSchema = new Schema({ n: Number });
const testSchema = new Schema({ name: { type: String, unique: true }, reference: Number });

const Count = db.model('gh4628', countSchema);
IslandRhythms marked this conversation as resolved.
Show resolved Hide resolved

testSchema.pre('save', async function(next) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this pre hook here? Doesn't seem to be necessary. Are you just looking for a way to leave some saves in progress to test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just copied the above test and modified it slightly. I see now that for this test that part is not necessary.

const doc = await Count.findOneAndUpdate({}, { $inc: { n: 1 } }, { new: true, upsert: true });
this.reference = doc.n;
next();
});

const Test = db.model('gh4628Test', testSchema);
const data = [];
for (let i = 0; i < 11; i++) {
data.push({ name: 'Test' + Math.abs(i - 4) });
}
await Test.create(data, { ordered: false }).catch(err => err);
const docs = await Test.find();
assert.equal(docs.length, 7); // docs 1,2,3,4 should not go through 11-4 == 7
});
});
});
});