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.

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

GODRIVER-1955 create labeledError interface #651

Merged
merged 3 commits into from
May 4, 2021
Merged

GODRIVER-1955 create labeledError interface #651

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
06b48e6
GODRIVER-1955 create labeledError interface
iwysiu Apr 26, 2021
4d6443e
remove labeledError from public interface
iwysiu Apr 27, 2021
0c34089
put HasErrorCode back
iwysiu Apr 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
GODRIVER-1955 create labeledError interface
  • Loading branch information
@iwysiu
iwysiu committed Apr 26, 2021
commit 06b48e68e333f65921505a8272a643daaf5bbd9d
16 changes: 10 additions & 6 deletions mongo/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func IsTimeout(err error) bool {
return ne.Timeout()
}
//timeout error labels
if se, ok := err.(ServerError); ok {
if se, ok := err.(labeledError); ok {
benjirewis marked this conversation as resolved.
Show resolved Hide resolved
if se.HasErrorLabel("NetworkTimeoutError") || se.HasErrorLabel("ExceededTimeLimitError") {
return true
}
Expand All @@ -130,8 +130,8 @@ func unwrap(err error) error {
// errorHasLabel returns true if err contains the specified label
func errorHasLabel(err error, label string) bool {
for ; err != nil; err = unwrap(err) {
if e, ok := err.(ServerError); ok {
return e.HasErrorLabel(label)
if e, ok := err.(labeledError); ok && e.HasErrorLabel(label) {
return true
}
}
return false
Expand Down Expand Up @@ -184,14 +184,18 @@ func (e MongocryptdError) Unwrap() error {
return e.Wrapped
}

type labeledError interface {
error
// HasErrorLabel returns true if the error contains the specified label.
HasErrorLabel(string) bool
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a reasonable change to fix this bug.

To check my understanding:

  1. driver.Error represents a server error. It is usually hidden from the caller. We do not want to expose x types directly. errors.replaceErrors will replace the top-level driver.Error with a CommandError, which implements ServerError.
  2. In this case, since the driver.Error is wrapped in a topology.ConnectionError and other auth errors, it won't be replaced.

Would another alternative be to have driver.Error implement the full ServerError interface? I am thinking it may let a caller use errors.As on a handshake error and call other ServerError methods.


// ServerError is the interface implemented by errors returned from the server. Custom implementations of this
// interface should not be used in production.
type ServerError interface {
error
labeledError
divjotarora marked this conversation as resolved.
Show resolved Hide resolved
// HasErrorCode returns true if the error has the specified code.
HasErrorCode(int) bool
// HasErrorLabel returns true if the error contains the specified label.
HasErrorLabel(string) bool
// HasErrorMessage returns true if the error contains the specified message.
HasErrorMessage(string) bool
// HasErrorCodeWithMessage returns true if any of the contained errors have the specified code and message.
Expand Down
1 change: 1 addition & 0 deletions mongo/integration/sdam_error_handling_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func TestSDAMErrorHandling(t *testing.T) {
_, err := mt.Coll.InsertOne(timeoutCtx, bson.D{{"test", 1}})
assert.NotNil(mt, err, "expected InsertOne error, got nil")
assert.True(mt, mongo.IsTimeout(err), "expected timeout error, got %v", err)
assert.True(mt, mongo.IsNetworkError(err), "expected network error, got %v", err)
assert.True(mt, isPoolCleared(), "expected pool to be cleared but was not")
})
mt.RunOpts("pool cleared on non-timeout network error", noClientOpts, func(mt *mtest.T) {
Expand Down