Skip to content

RUBY-2525 Expose the Reason an Operation Fails Document Validation #2379

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

Merged
merged 16 commits into from
Dec 14, 2021

Conversation

neilshweky
Copy link
Contributor

@neilshweky neilshweky commented Dec 7, 2021

I decided to add a details property to the OperationFailure class to better conform with the crud spec.
This attribute is defined as follows:

  • For WriteConcernErrors this is document['writeConcernError']['errInfo'].
  • For WriteErrors this is document['writeErrors'][0]['errInfo'].
  • For all other errors this is nil.

Now, this makes a lot of sense for the OperationFailure class, but for BulkWriteError it doesn't quite make sense. The problem is that BulkWriteError can have multiple writeErrors inside it. This is troublesome because I'm not sure what to do with the details now. Should it be a list of the errInfos of each writeError? This didn't seem to make sense, so I just left the details off the BulkWriteError class. If the user wants to find that information, they can get there from the results attr_reader.

Evergreen Patch

@neilshweky neilshweky marked this pull request as draft December 7, 2021 18:37
@neilshweky neilshweky marked this pull request as ready for review December 7, 2021 20:28
@neilshweky neilshweky closed this Dec 8, 2021
@neilshweky neilshweky reopened this Dec 8, 2021
# For WriteErrors this is `document['writeErrors'][0]['errInfo']`.
# For all other errors this is nil.
#
# @since 2.11.0
Copy link
Contributor

Choose a reason for hiding this comment

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

We no longer use @since tags and this one isn't accurate, since the functionality will be added in 2.18. You can remove this @since tag and, if others are in your way going forward, feel free to get rid of them also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@neilshweky neilshweky requested a review from p-mongo December 13, 2021 15:12
it 'succeeds and prints the error' do
begin
collection.insert_one({x: 1})
rescue Mongo::Error => e
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be Mongo::Error::OperationFailure, given the else branch below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@neilshweky neilshweky requested a review from p-mongo December 13, 2021 18:12
Copy link
Contributor

@p-mongo p-mongo left a comment

Choose a reason for hiding this comment

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

@comandeo ready for you

@neilshweky neilshweky merged commit 7c3b6cb into mongodb:master Dec 14, 2021
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.

4 participants