GODRIVER-3086 Add ErrorCodes to ServerError API#1894
GODRIVER-3086 Add ErrorCodes to ServerError API#1894prestonvasquez merged 7 commits intomongodb:masterfrom
Conversation
mongodb-drivers-pr-bot
Bot
added
the
review-priority-low
API Change Report./v2/mongocompatible changesBulkWriteException.ErrorCodes: added |
| HasErrorCodeWithMessage(int, string) bool | ||
|
|
||
| // ErrorCodes returns a deduplicated list of error codes returned by the | ||
| // server. |
There was a problem hiding this comment.
It’d be good to mention if there’s a defined sort order.
There was a problem hiding this comment.
Also, I suggest:
ErrorCodes returns all error codes (unsorted) in the server’s response. This includes nested errors (e.g., write concern errors) as well as the top-level error code.
| return hasErrorCode(e, code) | ||
| } | ||
|
|
||
| // ErrorCodes returns a list of error codes returned by the server. |
There was a problem hiding this comment.
It’d be good to mention if there’s a defined sort order.
There was a problem hiding this comment.
ErrorCodes returns all error codes (unsorted) in the server’s response. This includes nested errors (e.g., write concern errors) as well as the top-level error code.
| } | ||
| return false | ||
|
|
||
| // Deduplicate error codes. |
There was a problem hiding this comment.
Not that it affects us :), but: I see the same logic here and in WriteException’s method. This seems like the sort of thing you’d deduplicate.
| // HasErrorCodeWithMessage returns true if any of the contained errors have the specified code and message. | ||
| HasErrorCodeWithMessage(int, string) bool | ||
|
|
||
| // ErrorCodes returns a deduplicated list of error codes returned by the |
There was a problem hiding this comment.
The latest revision does not deduplicate error codes.
| // ErrorCodes returns a deduplicated list of error codes returned by the | |
| // ErrorCodes returns a list of error codes returned by the |
FGasper
left a comment
FGasper
left a comment
There was a problem hiding this comment.
I’ve made a few suggestions on verbiage; see what you think.
Also: are there any cases where it is, in fact, correct just to check HasErrorCode() without also checking for other, unexpected error codes? If not, should HasErrorCode()’s documentation also be updated to discourage its use?
Basically ISTM that anyplace an application calls HasErrorCode(123) should instead be a check of whether ErrorCodes()’s return contains solely 123. Is that reasonable?
| HasErrorCodeWithMessage(int, string) bool | ||
|
|
||
| // ErrorCodes returns a deduplicated list of error codes returned by the | ||
| // server. |
| HasErrorCodeWithMessage(int, string) bool | ||
|
|
||
| // ErrorCodes returns a deduplicated list of error codes returned by the | ||
| // server. |
There was a problem hiding this comment.
Also, I suggest:
ErrorCodes returns all error codes (unsorted) in the server’s response. This includes nested errors (e.g., write concern errors) as well as the top-level error code.
| return hasErrorCode(e, code) | ||
| } | ||
|
|
||
| // ErrorCodes returns a list of error codes returned by the server. |
There was a problem hiding this comment.
ErrorCodes returns all error codes (unsorted) in the server’s response. This includes nested errors (e.g., write concern errors) as well as the top-level error code.
I agree with the changes to the interface documentation but don't think we need to be more specific on the methods.
HasErrorCode's logic has not changed and so we should maintain the current intentions and usage guidelines for our users. The current documentation correctly expresses the purpose of the method for each error type. |
|
@FGasper I believe there are plenty of cases where |
|
I’ve moved the |
3 participants
GODRIVER-3086
Summary
Extend the
ServerErrorinterface to includeErrorCodeswhich will return a list of deduplicated error codes returned by the server during the lifetime of operation execution.Background & Motivation
The Go Driver provides a way to check if a ServerError has a specific error code, but no way to determine all error codes that occurred during the operation.