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

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

merged 3 commits into from Jul 19, 2023

Conversation

IslandRhythms
Copy link
Collaborator

closes #4628

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

Couple of minor issues but LGTM overall

test/model.create.test.js Outdated Show resolved Hide resolved

const Count = db.model('gh4628', countSchema);

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.

@hasezoey
Copy link
Collaborator

dosnt #13544 already (somewhat) address this? because enabling aggregateErrors all errors are returned in the result and all operations are waited until finished?

@vkarpov15
Copy link
Collaborator

@hasezoey #13544 addresses a similar issue, but this change applies to cases where aggregateErrors is false. Specifically, this change makes it so that you don't need to add an extra option or change your function signature to get the benefits of #4628. create() still works as usual, it just waits for all documents to finish inserting before throwing.

@vkarpov15 vkarpov15 merged commit 2798cfd into 8.0 Jul 19, 2023
33 of 34 checks passed
@vkarpov15 vkarpov15 deleted the IslandRhythms/4628 branch September 1, 2023 18:36
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

Successfully merging this pull request may close these issues.

None yet

3 participants