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

continueOnError option for eachAsync() #11601

Merged
merged 3 commits into from
Apr 2, 2022
Merged

continueOnError option for eachAsync() #11601

merged 3 commits into from
Apr 2, 2022

Conversation

vkarpov15
Copy link
Collaborator

Fix #6355

Copy link
Collaborator

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

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

I didn't tested it but only reading the code I think there is a small bug in there.

The preview in EachAsyncMultiError is only generated in the constructor with the passed Array, but not when we add an Error to the errors Array of the EachAsyncMultiError. So I assume the EachAsyncMultiError will not have the correct Error Message.

I guess we could change the let aggregatedErrors = null to const aggregatedErrors = continueOnError && [] and push Errors directly in there and in the last Moment when we want to pass the Error Object to the callback we construct the actual EachAsyncMultiError-Object.



/**
* The connection failed to reconnect and will never successfully reconnect to
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the comment correct?

lib/helpers/cursor/eachAsync.js Outdated Show resolved Hide resolved
types/cursor.d.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@AbdelrahmanHafez AbdelrahmanHafez left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, with only a couple of comments.

@vkarpov15 vkarpov15 merged commit 4c1f01c into 6.3 Apr 2, 2022
@vkarpov15 vkarpov15 deleted the gh-6355 branch April 2, 2022 16:05
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