Skip to content

GODRIVER-2325 Expose raw server response in relevant errors #871

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
Merged
merged 5 commits into from
Mar 24, 2022
Merged

GODRIVER-2325 Expose raw server response in relevant errors #871

merged 5 commits into from
Mar 24, 2022

Conversation

benjirewis
Copy link
Contributor

@benjirewis benjirewis commented Mar 10, 2022 •
edited
Loading

GODRIVER-2325

Adds Raw fields to various mongo and driver errors. Adds integration tests to ensure the raw server reply can be accessed through these fields.

benjirewis
benjirewis commented Mar 10, 2022
View reviewed changes
@benjirewis benjirewis marked this pull request as ready for review March 10, 2022 20:05
kevinAlbs
kevinAlbs reviewed Mar 15, 2022
View reviewed changes
Copy link
Contributor

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

Thank you for the thorough tests. I suggest removing the RawResponses method from the ServerError interface. Apologies for contradictory ideas.

@benjirewis benjirewis changed the title GODRIVER-2325 Expose raw server response in ServerError GODRIVER-2325 Expose raw server response in relevant errors Mar 16, 2022
@benjirewis benjirewis requested a review from kevinAlbs March 16, 2022 14:38
matthewdale
matthewdale approved these changes Mar 16, 2022
View reviewed changes
Copy link
Collaborator

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

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

Looks good 👍

kevinAlbs
kevinAlbs approved these changes Mar 16, 2022
View reviewed changes
Copy link
Contributor

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

LGTM. Nicely done!

@WenbinZhu can you review to validate this solves the intended use case?

@kevinAlbs kevinAlbs requested a review from WenbinZhu March 16, 2022 18:54
benjirewis and others added 2 commits March 16, 2022 14:55
@benjirewis @kevinAlbs
Update mongo/errors.go
64d24a1 
Co-authored-by: Kevin Albertson <[email protected]>
@benjirewis @kevinAlbs
Update mongo/integration/errors_test.go
84ee2ec 
Co-authored-by: Kevin Albertson <[email protected]>
WenbinZhu
WenbinZhu approved these changes Mar 24, 2022
View reviewed changes
Copy link
Member

@WenbinZhu WenbinZhu left a comment

Choose a reason for hiding this comment

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

Thanks for the change, LGTM! (Really sorry for the delay, my email notification setting routes all emails from non 10gen repos to my personal mailbox and I overlooked the review request email...)

@benjirewis
Copy link
Contributor Author

No worries @WenbinZhu ! Thanks for taking a look.

@benjirewis benjirewis merged commit 57747f8 into mongodb:master Mar 24, 2022
@benjirewis benjirewis deleted the serverReply.2325 branch March 24, 2022 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinAlbs kevinAlbs kevinAlbs approved these changes

@matthewdale matthewdale matthewdale approved these changes

@WenbinZhu WenbinZhu WenbinZhu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@benjirewis @kevinAlbs @matthewdale @WenbinZhu