From ed23feeaaa69e7b7a47c2553c6a7866a595b57d9 Mon Sep 17 00:00:00 2001 From: Kevin Albertson Date: Wed, 9 Nov 2022 14:53:34 -0500 Subject: [PATCH 01/17] add a failing test with both definite errors --- .../retryable_writes_prose_test.go | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/mongo/integration/retryable_writes_prose_test.go b/mongo/integration/retryable_writes_prose_test.go index 11198c0ad3..a02b8060f0 100644 --- a/mongo/integration/retryable_writes_prose_test.go +++ b/mongo/integration/retryable_writes_prose_test.go @@ -282,7 +282,37 @@ func TestRetryableWritesProse(t *testing.T) { require.True(mt, secondFailPointConfigured) - // Assert that the "NotWritablePrimary" error is returned. + // Assert that the "ShutdownInProgress" error is returned. require.True(mt, err.(mongo.WriteException).HasErrorCode(int(shutdownInProgressErrorCode))) }) + + mt.RunOpts(fmt.Sprintf("%s label on first and second error returns first", driver.NoWritesPerformed), mtNWPOpts, + func(mt *mtest.T) { + const shutdownInProgressErrorCode int32 = 91 + const notWritablePrimaryErrorCode int32 = 10107 + + monitor := new(event.CommandMonitor) + mt.ResetClient(options.Client().SetRetryWrites(true).SetMonitor(monitor)) + + mt.SetFailPoint(mtest.FailPoint{ + ConfigureFailPoint: "failCommand", + Mode: mtest.FailPointMode{Times: 2}, + Data: mtest.FailPointData{ + ErrorCode: notWritablePrimaryErrorCode, + ErrorLabels: &[]string{ + driver.NoWritesPerformed, + driver.RetryableWriteError, + }, + FailCommands: []string{"insert"}, + }, + }) + + // Attempt to insert a document. + _, err := mt.Coll.InsertOne(context.Background(), bson.D{{"x", 1}}) + + require.NotNil(mt, err) + + // Assert that the "NotWritablePrimary" error is returned. + require.True(mt, err.(mongo.WriteException).HasErrorCode(int(notWritablePrimaryErrorCode))) + }) } From f1ee4d15777e71b113808bcf8a5169cfe3826bd5 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Thu, 17 Nov 2022 14:29:10 -0700 Subject: [PATCH 02/17] GODRIVER-2651 Break NoWritesPerformed-Only Error Sequence --- .../insertOne-noWritesPerformedError.yml | 57 +++++++ .../insertOne-noWritesPerformedErrors.json | 98 +++++++++++ x/mongo/driver/errors.go | 61 +++++++ x/mongo/driver/errors_test.go | 158 ++++++++++++++++++ x/mongo/driver/operation.go | 28 +++- 5 files changed, 393 insertions(+), 9 deletions(-) create mode 100644 testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml create mode 100644 testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json create mode 100644 x/mongo/driver/errors_test.go diff --git a/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml b/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml new file mode 100644 index 0000000000..2a11bdd7ce --- /dev/null +++ b/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml @@ -0,0 +1,57 @@ +description: "retryable-writes insertOne noWritesPerformedErrors" + + schemaVersion: "1.0" + + runOnRequirements: + - minServerVersion: "3.6" + topologies: [ replicaset ] + + createEntities: + - client: + id: &client0 client0 + useMultipleMongoses: false + observeEvents: [ commandFailedEvent ] + - database: + id: &database0 database0 + client: *client0 + databaseName: &databaseName retryable-writes-tests + - collection: + id: &collection0 collection0 + database: *database0 + collectionName: &collectionName coll + + tests: + - description: "InsertOne fails after NoWritesPerformed error" + runOnRequirements: + - minServerVersion: "4.0" + topologies: [ replicaset ] + operations: + - name: failPoint + object: testRunner + arguments: + client: *client0 + failPoint: + configureFailPoint: failCommand + mode: + times: 2 + data: + failCommands: + - insert + errorCode: 64 + errorLabels: + - NoWritesPerformed + - RetryableWriteError + - name: insertOne + object: *collection0 + arguments: + document: + x: 1 + expectError: + errorCode: 64 + errorLabelsContain: + - NoWritesPerformed + - RetryableWriteError + outcome: + - collectionName: *collectionName + databaseName: *databaseName + documents: [] diff --git a/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json new file mode 100644 index 0000000000..4acdbd25e2 --- /dev/null +++ b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json @@ -0,0 +1,98 @@ +{ + "description": "retryable-writes insertOne noWritesPerformedErrors", + "schemaVersion": "1.0", + "runOnRequirements": [ + { + "minServerVersion": "3.6", + "topologies": [ + "replicaset" + ] + } + ], + "createEntities": [ + { + "client": { + "id": "client0", + "useMultipleMongoses": false, + "observeEvents": [ + "commandFailedEvent" + ] + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "retryable-writes-tests" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "coll" + } + } + ], + "tests": [ + { + "description": "InsertOne fails after NoWritesPerformed error", + "runOnRequirements": [ + { + "minServerVersion": "4.0", + "topologies": [ + "replicaset" + ] + } + ], + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 2 + }, + "data": { + "failCommands": [ + "insert" + ], + "errorCode": 64, + "errorLabels": [ + "NoWritesPerformed", + "RetryableWriteError" + ] + } + } + } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "x": 1 + } + }, + "expectError": { + "errorCode": 64, + "errorLabelsContain": [ + "NoWritesPerformed", + "RetryableWriteError" + ] + } + } + ], + "outcome": [ + { + "collectionName": "coll", + "databaseName": "retryable-writes-tests", + "documents": [] + } + ] + } + ] + } diff --git a/x/mongo/driver/errors.go b/x/mongo/driver/errors.go index cb56b84f50..6faeda4985 100644 --- a/x/mongo/driver/errors.go +++ b/x/mongo/driver/errors.go @@ -117,6 +117,37 @@ func (wce WriteCommandError) Error() string { return buf.String() } +// Is reports whether any error in err's chain matches target by comparing the error codes for the WriteConcernError +// and all of the WriteErrors. If any values from either field are "false", the function will return false. +func (wce WriteCommandError) Is(tgt error) bool { + target, ok := tgt.(WriteCommandError) + if !ok { + return false + } + + targetWCE := target.WriteConcernError + wceWCE := wce.WriteConcernError + + if targetWCE != nil && wceWCE != nil && !wceWCE.Is(*targetWCE) { + return false + } + + // If the WriteError lenghts are not equal, then the errors are not equal. + if len(target.WriteErrors) != len(wce.WriteErrors) { + return false + } + + if len(wce.WriteErrors) > 0 { + for idx, twe := range target.WriteErrors { + if !wce.WriteErrors[idx].Is(twe) { + return false + } + } + } + + return true +} + // Retryable returns true if the error is retryable func (wce WriteCommandError) Retryable(wireVersion *description.VersionRange) bool { for _, label := range wce.Labels { @@ -165,6 +196,16 @@ func (wce WriteConcernError) Error() string { return wce.Message } +// Is reports whether any error in err's chain matches target by comparing the error codes for the WriteConcernError. +func (wce WriteConcernError) Is(tgt error) bool { + target, ok := tgt.(WriteConcernError) + if !ok { + return false + } + + return wce.Code == target.Code +} + // Retryable returns true if the error is retryable func (wce WriteConcernError) Retryable() bool { for _, code := range retryableCodes { @@ -221,6 +262,16 @@ type WriteError struct { func (we WriteError) Error() string { return we.Message } +// Is reports whether any error in err's chain matches target by comparing the error codes for the WriteError. +func (we WriteError) Is(tgt error) bool { + target, ok := tgt.(WriteError) + if !ok { + return false + } + + return we.Code == target.Code +} + // WriteErrors is a group of non-write concern failures that occurred as a result // of a write operation. type WriteErrors []WriteError @@ -267,6 +318,16 @@ func (e Error) Unwrap() error { return e.Wrapped } +// Is reports whether any error in err's chain matches target by comparing the error codes for the Error. +func (e Error) Is(tgt error) bool { + target, ok := tgt.(Error) + if !ok { + return false + } + + return errors.Is(e.Wrapped, target.Wrapped) && e.Code == target.Code +} + // HasErrorLabel returns true if the error contains the specified label. func (e Error) HasErrorLabel(label string) bool { if e.Labels != nil { diff --git a/x/mongo/driver/errors_test.go b/x/mongo/driver/errors_test.go new file mode 100644 index 0000000000..cee98dcf06 --- /dev/null +++ b/x/mongo/driver/errors_test.go @@ -0,0 +1,158 @@ +package driver + +import ( + "errors" + "testing" +) + +func TestErrorIs(t *testing.T) { + t.Parallel() + + for _, tcase := range []struct { + name string + want bool + err1 error + err2 error + }{ + { + "Error with same codes", + true, + Error{Code: 1}, + Error{Code: 1}, + }, + { + "Error with different codes", + false, + Error{Code: 1}, + Error{Code: 2}, + }, + { + "Error with different types", + false, + Error{Code: 1}, + errors.New("foo"), + }, + { + "WriteError with same codes", + true, + WriteError{Code: 1}, + WriteError{Code: 1}, + }, + { + "WriteError with different codes", + false, + WriteError{Code: 1}, + WriteError{Code: 2}, + }, + { + "WriteError with different types", + false, + WriteError{Code: 1}, + errors.New("foo"), + }, + { + "WriteErrors with same codes", + true, + WriteConcernError{Code: 1}, + WriteConcernError{Code: 1}, + }, + { + "WriteErrors with different codes", + false, + WriteConcernError{Code: 1}, + WriteConcernError{Code: 2}, + }, + { + "WriteErrors with different types", + false, + WriteConcernError{Code: 1}, + errors.New("foo"), + }, + { + "WriteCommandError with same WriteConcernError and nil WriteErrors", + true, + WriteCommandError{WriteConcernError: &WriteConcernError{Code: 1}}, + WriteCommandError{WriteConcernError: &WriteConcernError{Code: 1}}, + }, + { + "WriteException with different WriteConcernError and nil WriteErrors", + false, + WriteCommandError{WriteConcernError: &WriteConcernError{Code: 1}}, + WriteCommandError{WriteConcernError: &WriteConcernError{Code: 2}}, + }, + { + "WriteCommandError with same WriteConcernError and same WriteError", + true, + WriteCommandError{ + WriteConcernError: &WriteConcernError{Code: 1}, + WriteErrors: []WriteError{{Code: 1}}, + }, + WriteCommandError{ + WriteConcernError: &WriteConcernError{Code: 1}, + WriteErrors: []WriteError{{Code: 1}}, + }, + }, + { + "WriteCommandError with different WriteConcernErrors and same WriteErrors", + false, + WriteCommandError{ + WriteConcernError: &WriteConcernError{Code: 1}, + WriteErrors: []WriteError{{Code: 1}}, + }, + WriteCommandError{ + WriteConcernError: &WriteConcernError{Code: 2}, + WriteErrors: []WriteError{{Code: 1}}, + }, + }, + { + "WriteCommandError with same WriteConcernErrors and different WriteErrors", + false, + WriteCommandError{ + WriteConcernError: &WriteConcernError{Code: 1}, + WriteErrors: []WriteError{{Code: 1}}, + }, + WriteCommandError{ + WriteConcernError: &WriteConcernError{Code: 1}, + WriteErrors: []WriteError{{Code: 2}}, + }, + }, + { + "WriteCommandError with different WriteConcernErrors and different WriteErrors", + false, + WriteCommandError{ + WriteConcernError: &WriteConcernError{Code: 1}, + WriteErrors: []WriteError{{Code: 1}}, + }, + WriteCommandError{ + WriteConcernError: &WriteConcernError{Code: 2}, + WriteErrors: []WriteError{{Code: 2}}, + }, + }, + { + "WriteCommandError with nil WriteConcernError and same WriteErrors", + true, + WriteCommandError{ + WriteErrors: []WriteError{{Code: 1}}, + }, + WriteCommandError{ + WriteErrors: []WriteError{{Code: 1}}, + }, + }, + { + "WriteCommandError with different types", + false, + WriteCommandError{WriteConcernError: &WriteConcernError{Code: 1}}, + errors.New("foo"), + }, + } { + tcase := tcase + + t.Run(tcase.name, func(t *testing.T) { + t.Parallel() + + if got := errors.Is(tcase.err1, tcase.err2); got != tcase.want { + t.Errorf("Expected %v, got %v", tcase.want, got) + } + }) + } +} diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index 6324e95119..af6e6e8df4 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -397,10 +397,13 @@ func (op Operation) Execute(ctx context.Context) error { // Set the previous indefinite error to be returned in any case where a retryable write error does not have a // NoWritesPerfomed label (the definite case). - switch err := err.(type) { + switch typedError := err.(type) { case LabeledError: - if !err.HasErrorLabel(NoWritesPerformed) && err.HasErrorLabel(RetryableWriteError) { - prevIndefiniteErr = err.(error) + // If the error is not labeled NoWritesPerformed and is retryable, or if the previous error is + // undefined, then set the previous indefinite error to be the current error. + if !typedError.HasErrorLabel(NoWritesPerformed) && + typedError.HasErrorLabel(RetryableWriteError) || prevIndefiniteErr == nil { + prevIndefiniteErr = err } } @@ -595,6 +598,7 @@ func (op Operation) Execute(ctx context.Context) error { finishedInfo.cmdErr = err op.publishFinishedEvent(ctx, finishedInfo) + checkError: var perr error switch tt := err.(type) { case WriteCommandError: @@ -627,9 +631,12 @@ func (op Operation) Execute(ctx context.Context) error { } // If the error is no longer retryable and has the NoWritesPerformed label, then we should - // return the previous indefinite error. - if tt.HasErrorLabel(NoWritesPerformed) { - return prevIndefiniteErr + // set the error to the "previous indefinite error" unless the current error is already the + // "previous indefinite error". After reseting, repeat the error check. + if tt.HasErrorLabel(NoWritesPerformed) && !errors.Is(tt, prevIndefiniteErr) { + err = prevIndefiniteErr + + goto checkError } // If the operation isn't being retried, process the response @@ -720,9 +727,12 @@ func (op Operation) Execute(ctx context.Context) error { } // If the error is no longer retryable and has the NoWritesPerformed label, then we should - // return the previous indefinite error. - if tt.HasErrorLabel(NoWritesPerformed) { - return prevIndefiniteErr + // set the error to the "previous indefinite error" unless the current error is already the + // "previous indefinite error". After reseting, repeat the error check. + if tt.HasErrorLabel(NoWritesPerformed) && !errors.Is(tt, prevIndefiniteErr) { + err = prevIndefiniteErr + + goto checkError } // If the operation isn't being retried, process the response From ae05ad2844ad086e2d64218868e19fb6517a1693 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Thu, 17 Nov 2022 14:32:21 -0700 Subject: [PATCH 03/17] GODRIVER-2651 remove prose test --- .../retryable_writes_prose_test.go | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/mongo/integration/retryable_writes_prose_test.go b/mongo/integration/retryable_writes_prose_test.go index a02b8060f0..1f496ea328 100644 --- a/mongo/integration/retryable_writes_prose_test.go +++ b/mongo/integration/retryable_writes_prose_test.go @@ -285,34 +285,4 @@ func TestRetryableWritesProse(t *testing.T) { // Assert that the "ShutdownInProgress" error is returned. require.True(mt, err.(mongo.WriteException).HasErrorCode(int(shutdownInProgressErrorCode))) }) - - mt.RunOpts(fmt.Sprintf("%s label on first and second error returns first", driver.NoWritesPerformed), mtNWPOpts, - func(mt *mtest.T) { - const shutdownInProgressErrorCode int32 = 91 - const notWritablePrimaryErrorCode int32 = 10107 - - monitor := new(event.CommandMonitor) - mt.ResetClient(options.Client().SetRetryWrites(true).SetMonitor(monitor)) - - mt.SetFailPoint(mtest.FailPoint{ - ConfigureFailPoint: "failCommand", - Mode: mtest.FailPointMode{Times: 2}, - Data: mtest.FailPointData{ - ErrorCode: notWritablePrimaryErrorCode, - ErrorLabels: &[]string{ - driver.NoWritesPerformed, - driver.RetryableWriteError, - }, - FailCommands: []string{"insert"}, - }, - }) - - // Attempt to insert a document. - _, err := mt.Coll.InsertOne(context.Background(), bson.D{{"x", 1}}) - - require.NotNil(mt, err) - - // Assert that the "NotWritablePrimary" error is returned. - require.True(mt, err.(mongo.WriteException).HasErrorCode(int(notWritablePrimaryErrorCode))) - }) } From 5b885fee5286ee3f48977154aaefe16ed96ee8a1 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Thu, 17 Nov 2022 15:32:23 -0700 Subject: [PATCH 04/17] GODRIVER-2651 update collection name --- .../unified/insertOne-noWritesPerformedError.yml | 2 +- .../unified/insertOne-noWritesPerformedErrors.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml b/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml index 2a11bdd7ce..58a330b267 100644 --- a/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml +++ b/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml @@ -18,7 +18,7 @@ description: "retryable-writes insertOne noWritesPerformedErrors" - collection: id: &collection0 collection0 database: *database0 - collectionName: &collectionName coll + collectionName: &collectionName no-writes-performed-collection tests: - description: "InsertOne fails after NoWritesPerformed error" diff --git a/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json index 4acdbd25e2..8a685f2fc4 100644 --- a/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json +++ b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json @@ -30,7 +30,7 @@ "collection": { "id": "collection0", "database": "database0", - "collectionName": "coll" + "collectionName": "no-writes-performed-collection" } } ], @@ -88,7 +88,7 @@ ], "outcome": [ { - "collectionName": "coll", + "collectionName": "no-writes-performed-collection", "databaseName": "retryable-writes-tests", "documents": [] } From 1ef1195d832404b5a3ef1ce370d3fdfd13d8d050 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Thu, 17 Nov 2022 18:10:28 -0700 Subject: [PATCH 05/17] GODRIVER-2651 set mwv to 4.4 --- .../unified/insertOne-noWritesPerformedError.yml | 2 +- .../unified/insertOne-noWritesPerformedErrors.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml b/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml index 58a330b267..5707a2075a 100644 --- a/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml +++ b/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml @@ -23,7 +23,7 @@ description: "retryable-writes insertOne noWritesPerformedErrors" tests: - description: "InsertOne fails after NoWritesPerformed error" runOnRequirements: - - minServerVersion: "4.0" + - minServerVersion: "4.4" topologies: [ replicaset ] operations: - name: failPoint diff --git a/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json index 8a685f2fc4..4b8f3134ee 100644 --- a/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json +++ b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json @@ -39,7 +39,7 @@ "description": "InsertOne fails after NoWritesPerformed error", "runOnRequirements": [ { - "minServerVersion": "4.0", + "minServerVersion": "4.4", "topologies": [ "replicaset" ] From ab07adb90bc3225f1ad2c8dd627f6efa94cf3fa9 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Fri, 18 Nov 2022 08:41:01 -0700 Subject: [PATCH 06/17] GODRIVER-2651 update min wire version to 6.0 --- .../unified/insertOne-noWritesPerformedErrors.json | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json index 4b8f3134ee..62fdb1adaa 100644 --- a/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json +++ b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json @@ -3,9 +3,10 @@ "schemaVersion": "1.0", "runOnRequirements": [ { - "minServerVersion": "3.6", + "minServerVersion": "6.0", "topologies": [ - "replicaset" + "replicaset", + "sharded-replicaset" ] } ], @@ -37,14 +38,6 @@ "tests": [ { "description": "InsertOne fails after NoWritesPerformed error", - "runOnRequirements": [ - { - "minServerVersion": "4.4", - "topologies": [ - "replicaset" - ] - } - ], "operations": [ { "name": "failPoint", From e9772dba665d2f914d0a29f6817b53a662b00745 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Fri, 18 Nov 2022 10:28:55 -0700 Subject: [PATCH 07/17] GODRIVER-2651 fix length typo --- x/mongo/driver/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/mongo/driver/errors.go b/x/mongo/driver/errors.go index 6faeda4985..1f49010a99 100644 --- a/x/mongo/driver/errors.go +++ b/x/mongo/driver/errors.go @@ -132,7 +132,7 @@ func (wce WriteCommandError) Is(tgt error) bool { return false } - // If the WriteError lenghts are not equal, then the errors are not equal. + // If the WriteError lengths are not equal, then the errors are not equal. if len(target.WriteErrors) != len(wce.WriteErrors) { return false } From c8edc324c8794ac5ec3d29f55fcdad772954b72c Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Fri, 18 Nov 2022 10:31:36 -0700 Subject: [PATCH 08/17] GODRIVER-2651 sync spec tests --- .../insertOne-noWritesPerformedError.yml | 101 +++++----- .../insertOne-noWritesPerformedErrors.json | 179 +++++++++--------- 2 files changed, 138 insertions(+), 142 deletions(-) diff --git a/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml b/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml index 5707a2075a..3295d153dd 100644 --- a/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml +++ b/testdata/retryable-writes/unified/insertOne-noWritesPerformedError.yml @@ -1,57 +1,54 @@ description: "retryable-writes insertOne noWritesPerformedErrors" - schemaVersion: "1.0" +schemaVersion: "1.0" - runOnRequirements: - - minServerVersion: "3.6" - topologies: [ replicaset ] +runOnRequirements: + - minServerVersion: "6.0" + topologies: [ replicaset ] - createEntities: - - client: - id: &client0 client0 - useMultipleMongoses: false - observeEvents: [ commandFailedEvent ] - - database: - id: &database0 database0 - client: *client0 - databaseName: &databaseName retryable-writes-tests - - collection: - id: &collection0 collection0 - database: *database0 - collectionName: &collectionName no-writes-performed-collection +createEntities: + - client: + id: &client0 client0 + useMultipleMongoses: false + observeEvents: [ commandFailedEvent ] + - database: + id: &database0 database0 + client: *client0 + databaseName: &databaseName retryable-writes-tests + - collection: + id: &collection0 collection0 + database: *database0 + collectionName: &collectionName no-writes-performed-collection - tests: - - description: "InsertOne fails after NoWritesPerformed error" - runOnRequirements: - - minServerVersion: "4.4" - topologies: [ replicaset ] - operations: - - name: failPoint - object: testRunner - arguments: - client: *client0 - failPoint: - configureFailPoint: failCommand - mode: - times: 2 - data: - failCommands: - - insert - errorCode: 64 - errorLabels: - - NoWritesPerformed - - RetryableWriteError - - name: insertOne - object: *collection0 - arguments: - document: - x: 1 - expectError: - errorCode: 64 - errorLabelsContain: - - NoWritesPerformed - - RetryableWriteError - outcome: - - collectionName: *collectionName - databaseName: *databaseName - documents: [] +tests: + - description: "InsertOne fails after NoWritesPerformed error" + operations: + - name: failPoint + object: testRunner + arguments: + client: *client0 + failPoint: + configureFailPoint: failCommand + mode: + times: 2 + data: + failCommands: + - insert + errorCode: 64 + errorLabels: + - NoWritesPerformed + - RetryableWriteError + - name: insertOne + object: *collection0 + arguments: + document: + x: 1 + expectError: + errorCode: 64 + errorLabelsContain: + - NoWritesPerformed + - RetryableWriteError + outcome: + - collectionName: *collectionName + databaseName: *databaseName + documents: [] diff --git a/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json index 62fdb1adaa..3194e91c5c 100644 --- a/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json +++ b/testdata/retryable-writes/unified/insertOne-noWritesPerformedErrors.json @@ -1,91 +1,90 @@ { - "description": "retryable-writes insertOne noWritesPerformedErrors", - "schemaVersion": "1.0", - "runOnRequirements": [ - { - "minServerVersion": "6.0", - "topologies": [ - "replicaset", - "sharded-replicaset" - ] - } - ], - "createEntities": [ - { - "client": { - "id": "client0", - "useMultipleMongoses": false, - "observeEvents": [ - "commandFailedEvent" - ] - } - }, - { - "database": { - "id": "database0", - "client": "client0", - "databaseName": "retryable-writes-tests" - } - }, - { - "collection": { - "id": "collection0", - "database": "database0", - "collectionName": "no-writes-performed-collection" - } - } - ], - "tests": [ - { - "description": "InsertOne fails after NoWritesPerformed error", - "operations": [ - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "client0", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 2 - }, - "data": { - "failCommands": [ - "insert" - ], - "errorCode": 64, - "errorLabels": [ - "NoWritesPerformed", - "RetryableWriteError" - ] - } - } - } - }, - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "x": 1 - } - }, - "expectError": { - "errorCode": 64, - "errorLabelsContain": [ - "NoWritesPerformed", - "RetryableWriteError" - ] - } - } - ], - "outcome": [ - { - "collectionName": "no-writes-performed-collection", - "databaseName": "retryable-writes-tests", - "documents": [] - } - ] - } - ] - } + "description": "retryable-writes insertOne noWritesPerformedErrors", + "schemaVersion": "1.0", + "runOnRequirements": [ + { + "minServerVersion": "6.0", + "topologies": [ + "replicaset" + ] + } + ], + "createEntities": [ + { + "client": { + "id": "client0", + "useMultipleMongoses": false, + "observeEvents": [ + "commandFailedEvent" + ] + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "retryable-writes-tests" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "no-writes-performed-collection" + } + } + ], + "tests": [ + { + "description": "InsertOne fails after NoWritesPerformed error", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 2 + }, + "data": { + "failCommands": [ + "insert" + ], + "errorCode": 64, + "errorLabels": [ + "NoWritesPerformed", + "RetryableWriteError" + ] + } + } + } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "x": 1 + } + }, + "expectError": { + "errorCode": 64, + "errorLabelsContain": [ + "NoWritesPerformed", + "RetryableWriteError" + ] + } + } + ], + "outcome": [ + { + "collectionName": "no-writes-performed-collection", + "databaseName": "retryable-writes-tests", + "documents": [] + } + ] + } + ] +} From c7ab0077b05acdaf1a0fcd24b097cd87816f9839 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Fri, 2 Dec 2022 12:02:15 -0700 Subject: [PATCH 09/17] GODRIVER-2651 clean up branches --- x/mongo/driver/operation.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index af6e6e8df4..6b5ac087b2 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -399,10 +399,18 @@ func (op Operation) Execute(ctx context.Context) error { // NoWritesPerfomed label (the definite case). switch typedError := err.(type) { case LabeledError: - // If the error is not labeled NoWritesPerformed and is retryable, or if the previous error is - // undefined, then set the previous indefinite error to be the current error. + // If the "previousError" is "null", then the "currentError" is the first error encountered + // during the retry attempt cycle. We must persist the first error in the case where all + // succeeding errors are labeled "NoWritesPerformed", which would otherwise raise "null" as the + // error. + if prevIndefiniteErr == nil { + prevIndefiniteErr = err + } + + // If the error is not labeled NoWritesPerformed and is retryable, then set the previous + // indefinite error to be the current error. if !typedError.HasErrorLabel(NoWritesPerformed) && - typedError.HasErrorLabel(RetryableWriteError) || prevIndefiniteErr == nil { + typedError.HasErrorLabel(RetryableWriteError) { prevIndefiniteErr = err } } From 33122974a52129c1349a229b38a1bbf7a285fb4c Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Fri, 2 Dec 2022 12:04:30 -0700 Subject: [PATCH 10/17] GODRIVER-2651 rename typedError to terr --- x/mongo/driver/operation.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index 6b5ac087b2..5a3ac35c59 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -397,7 +397,7 @@ func (op Operation) Execute(ctx context.Context) error { // Set the previous indefinite error to be returned in any case where a retryable write error does not have a // NoWritesPerfomed label (the definite case). - switch typedError := err.(type) { + switch terr := err.(type) { case LabeledError: // If the "previousError" is "null", then the "currentError" is the first error encountered // during the retry attempt cycle. We must persist the first error in the case where all @@ -409,8 +409,7 @@ func (op Operation) Execute(ctx context.Context) error { // If the error is not labeled NoWritesPerformed and is retryable, then set the previous // indefinite error to be the current error. - if !typedError.HasErrorLabel(NoWritesPerformed) && - typedError.HasErrorLabel(RetryableWriteError) { + if !terr.HasErrorLabel(NoWritesPerformed) && terr.HasErrorLabel(RetryableWriteError) { prevIndefiniteErr = err } } From d97e6d87478c8b94b217806ba3a970d86507e67d Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Fri, 2 Dec 2022 12:05:41 -0700 Subject: [PATCH 11/17] GODRIVER-2651 clean up comments --- x/mongo/driver/operation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index 5a3ac35c59..19b251aaa1 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -399,9 +399,9 @@ func (op Operation) Execute(ctx context.Context) error { // NoWritesPerfomed label (the definite case). switch terr := err.(type) { case LabeledError: - // If the "previousError" is "null", then the "currentError" is the first error encountered + // If the "prevIndefiniteErr" is nil, then the current error is the first error encountered // during the retry attempt cycle. We must persist the first error in the case where all - // succeeding errors are labeled "NoWritesPerformed", which would otherwise raise "null" as the + // succeeding errors are labeled "NoWritesPerformed", which would otherwise raise nil as the // error. if prevIndefiniteErr == nil { prevIndefiniteErr = err From 482798f2e52bfbb541ac54832bcbf756db8506dd Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Tue, 6 Dec 2022 16:58:27 -0700 Subject: [PATCH 12/17] GODRIVER-2651 add license to errors_test --- x/mongo/driver/errors_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x/mongo/driver/errors_test.go b/x/mongo/driver/errors_test.go index cee98dcf06..e0bf5e43e6 100644 --- a/x/mongo/driver/errors_test.go +++ b/x/mongo/driver/errors_test.go @@ -1,3 +1,8 @@ +// Copyright (C) MongoDB, Inc. 2022-present. +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. You may obtain +// a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 package driver import ( From d4a087b11b9a97af5b5de02e004648638040349b Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Wed, 7 Dec 2022 10:22:07 -0700 Subject: [PATCH 13/17] GODRIVER-2651 create WriteErrors Is method --- x/mongo/driver/errors.go | 28 ++++++++++++++++++++++------ x/mongo/driver/operation.go | 11 ++++++----- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/x/mongo/driver/errors.go b/x/mongo/driver/errors.go index 1f49010a99..116b7b1d7c 100644 --- a/x/mongo/driver/errors.go +++ b/x/mongo/driver/errors.go @@ -137,12 +137,8 @@ func (wce WriteCommandError) Is(tgt error) bool { return false } - if len(wce.WriteErrors) > 0 { - for idx, twe := range target.WriteErrors { - if !wce.WriteErrors[idx].Is(twe) { - return false - } - } + if !wce.WriteErrors.Is(target.WriteErrors) { + return false } return true @@ -289,6 +285,26 @@ func (we WriteErrors) Error() string { return buf.String() } +// Is reports whether any error in err's chain matches target by comparing the error codes for the WriteErrors. +func (we WriteErrors) Is(tgt error) bool { + target, ok := tgt.(WriteErrors) + if !ok { + return false + } + + if len(we) != len(target) { + return false + } + + for idx, err := range we { + if !err.Is(target[idx]) { + return false + } + } + + return true +} + // Error is a command execution error from the database. type Error struct { Code int32 diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index 19b251aaa1..ea26ecb692 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -59,8 +59,9 @@ type RetryablePoolError interface { Retryable() bool } -// LabeledError is an error that can have error labels added to it. -type LabeledError interface { +// labeledError is an error that can have error labels added to it. +type labeledError interface { + error HasErrorLabel(string) bool } @@ -397,8 +398,8 @@ func (op Operation) Execute(ctx context.Context) error { // Set the previous indefinite error to be returned in any case where a retryable write error does not have a // NoWritesPerfomed label (the definite case). - switch terr := err.(type) { - case LabeledError: + switch err := err.(type) { + case labeledError: // If the "prevIndefiniteErr" is nil, then the current error is the first error encountered // during the retry attempt cycle. We must persist the first error in the case where all // succeeding errors are labeled "NoWritesPerformed", which would otherwise raise nil as the @@ -409,7 +410,7 @@ func (op Operation) Execute(ctx context.Context) error { // If the error is not labeled NoWritesPerformed and is retryable, then set the previous // indefinite error to be the current error. - if !terr.HasErrorLabel(NoWritesPerformed) && terr.HasErrorLabel(RetryableWriteError) { + if !err.HasErrorLabel(NoWritesPerformed) && err.HasErrorLabel(RetryableWriteError) { prevIndefiniteErr = err } } From 6429404872c2a263989e54868ae4f1e2d7dc2e66 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Wed, 7 Dec 2022 10:45:03 -0700 Subject: [PATCH 14/17] GODRIVER-2651 remove redundant length check --- x/mongo/driver/errors.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/x/mongo/driver/errors.go b/x/mongo/driver/errors.go index 116b7b1d7c..1c75a79212 100644 --- a/x/mongo/driver/errors.go +++ b/x/mongo/driver/errors.go @@ -132,11 +132,6 @@ func (wce WriteCommandError) Is(tgt error) bool { return false } - // If the WriteError lengths are not equal, then the errors are not equal. - if len(target.WriteErrors) != len(wce.WriteErrors) { - return false - } - if !wce.WriteErrors.Is(target.WriteErrors) { return false } From 475f7ec15c15ce49b988d11f8f4f3418272dc0b3 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Wed, 7 Dec 2022 14:44:49 -0700 Subject: [PATCH 15/17] GODRIVER-2651 remove the Error.Is logic --- x/mongo/driver/errors.go | 72 --------------- x/mongo/driver/errors_test.go | 163 ---------------------------------- x/mongo/driver/operation.go | 12 ++- 3 files changed, 10 insertions(+), 237 deletions(-) delete mode 100644 x/mongo/driver/errors_test.go diff --git a/x/mongo/driver/errors.go b/x/mongo/driver/errors.go index 1c75a79212..cb56b84f50 100644 --- a/x/mongo/driver/errors.go +++ b/x/mongo/driver/errors.go @@ -117,28 +117,6 @@ func (wce WriteCommandError) Error() string { return buf.String() } -// Is reports whether any error in err's chain matches target by comparing the error codes for the WriteConcernError -// and all of the WriteErrors. If any values from either field are "false", the function will return false. -func (wce WriteCommandError) Is(tgt error) bool { - target, ok := tgt.(WriteCommandError) - if !ok { - return false - } - - targetWCE := target.WriteConcernError - wceWCE := wce.WriteConcernError - - if targetWCE != nil && wceWCE != nil && !wceWCE.Is(*targetWCE) { - return false - } - - if !wce.WriteErrors.Is(target.WriteErrors) { - return false - } - - return true -} - // Retryable returns true if the error is retryable func (wce WriteCommandError) Retryable(wireVersion *description.VersionRange) bool { for _, label := range wce.Labels { @@ -187,16 +165,6 @@ func (wce WriteConcernError) Error() string { return wce.Message } -// Is reports whether any error in err's chain matches target by comparing the error codes for the WriteConcernError. -func (wce WriteConcernError) Is(tgt error) bool { - target, ok := tgt.(WriteConcernError) - if !ok { - return false - } - - return wce.Code == target.Code -} - // Retryable returns true if the error is retryable func (wce WriteConcernError) Retryable() bool { for _, code := range retryableCodes { @@ -253,16 +221,6 @@ type WriteError struct { func (we WriteError) Error() string { return we.Message } -// Is reports whether any error in err's chain matches target by comparing the error codes for the WriteError. -func (we WriteError) Is(tgt error) bool { - target, ok := tgt.(WriteError) - if !ok { - return false - } - - return we.Code == target.Code -} - // WriteErrors is a group of non-write concern failures that occurred as a result // of a write operation. type WriteErrors []WriteError @@ -280,26 +238,6 @@ func (we WriteErrors) Error() string { return buf.String() } -// Is reports whether any error in err's chain matches target by comparing the error codes for the WriteErrors. -func (we WriteErrors) Is(tgt error) bool { - target, ok := tgt.(WriteErrors) - if !ok { - return false - } - - if len(we) != len(target) { - return false - } - - for idx, err := range we { - if !err.Is(target[idx]) { - return false - } - } - - return true -} - // Error is a command execution error from the database. type Error struct { Code int32 @@ -329,16 +267,6 @@ func (e Error) Unwrap() error { return e.Wrapped } -// Is reports whether any error in err's chain matches target by comparing the error codes for the Error. -func (e Error) Is(tgt error) bool { - target, ok := tgt.(Error) - if !ok { - return false - } - - return errors.Is(e.Wrapped, target.Wrapped) && e.Code == target.Code -} - // HasErrorLabel returns true if the error contains the specified label. func (e Error) HasErrorLabel(label string) bool { if e.Labels != nil { diff --git a/x/mongo/driver/errors_test.go b/x/mongo/driver/errors_test.go deleted file mode 100644 index e0bf5e43e6..0000000000 --- a/x/mongo/driver/errors_test.go +++ /dev/null @@ -1,163 +0,0 @@ -// Copyright (C) MongoDB, Inc. 2022-present. -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may -// not use this file except in compliance with the License. You may obtain -// a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 -package driver - -import ( - "errors" - "testing" -) - -func TestErrorIs(t *testing.T) { - t.Parallel() - - for _, tcase := range []struct { - name string - want bool - err1 error - err2 error - }{ - { - "Error with same codes", - true, - Error{Code: 1}, - Error{Code: 1}, - }, - { - "Error with different codes", - false, - Error{Code: 1}, - Error{Code: 2}, - }, - { - "Error with different types", - false, - Error{Code: 1}, - errors.New("foo"), - }, - { - "WriteError with same codes", - true, - WriteError{Code: 1}, - WriteError{Code: 1}, - }, - { - "WriteError with different codes", - false, - WriteError{Code: 1}, - WriteError{Code: 2}, - }, - { - "WriteError with different types", - false, - WriteError{Code: 1}, - errors.New("foo"), - }, - { - "WriteErrors with same codes", - true, - WriteConcernError{Code: 1}, - WriteConcernError{Code: 1}, - }, - { - "WriteErrors with different codes", - false, - WriteConcernError{Code: 1}, - WriteConcernError{Code: 2}, - }, - { - "WriteErrors with different types", - false, - WriteConcernError{Code: 1}, - errors.New("foo"), - }, - { - "WriteCommandError with same WriteConcernError and nil WriteErrors", - true, - WriteCommandError{WriteConcernError: &WriteConcernError{Code: 1}}, - WriteCommandError{WriteConcernError: &WriteConcernError{Code: 1}}, - }, - { - "WriteException with different WriteConcernError and nil WriteErrors", - false, - WriteCommandError{WriteConcernError: &WriteConcernError{Code: 1}}, - WriteCommandError{WriteConcernError: &WriteConcernError{Code: 2}}, - }, - { - "WriteCommandError with same WriteConcernError and same WriteError", - true, - WriteCommandError{ - WriteConcernError: &WriteConcernError{Code: 1}, - WriteErrors: []WriteError{{Code: 1}}, - }, - WriteCommandError{ - WriteConcernError: &WriteConcernError{Code: 1}, - WriteErrors: []WriteError{{Code: 1}}, - }, - }, - { - "WriteCommandError with different WriteConcernErrors and same WriteErrors", - false, - WriteCommandError{ - WriteConcernError: &WriteConcernError{Code: 1}, - WriteErrors: []WriteError{{Code: 1}}, - }, - WriteCommandError{ - WriteConcernError: &WriteConcernError{Code: 2}, - WriteErrors: []WriteError{{Code: 1}}, - }, - }, - { - "WriteCommandError with same WriteConcernErrors and different WriteErrors", - false, - WriteCommandError{ - WriteConcernError: &WriteConcernError{Code: 1}, - WriteErrors: []WriteError{{Code: 1}}, - }, - WriteCommandError{ - WriteConcernError: &WriteConcernError{Code: 1}, - WriteErrors: []WriteError{{Code: 2}}, - }, - }, - { - "WriteCommandError with different WriteConcernErrors and different WriteErrors", - false, - WriteCommandError{ - WriteConcernError: &WriteConcernError{Code: 1}, - WriteErrors: []WriteError{{Code: 1}}, - }, - WriteCommandError{ - WriteConcernError: &WriteConcernError{Code: 2}, - WriteErrors: []WriteError{{Code: 2}}, - }, - }, - { - "WriteCommandError with nil WriteConcernError and same WriteErrors", - true, - WriteCommandError{ - WriteErrors: []WriteError{{Code: 1}}, - }, - WriteCommandError{ - WriteErrors: []WriteError{{Code: 1}}, - }, - }, - { - "WriteCommandError with different types", - false, - WriteCommandError{WriteConcernError: &WriteConcernError{Code: 1}}, - errors.New("foo"), - }, - } { - tcase := tcase - - t.Run(tcase.name, func(t *testing.T) { - t.Parallel() - - if got := errors.Is(tcase.err1, tcase.err2); got != tcase.want { - t.Errorf("Expected %v, got %v", tcase.want, got) - } - }) - } -} diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index ea26ecb692..a211bff73a 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -606,6 +606,12 @@ func (op Operation) Execute(ctx context.Context) error { finishedInfo.cmdErr = err op.publishFinishedEvent(ctx, finishedInfo) + // prevIndefiniteErrorIsSet is "true" if the "err" variable has been set to the "prevIndefiniteErr" in + // a case in the switch statement below. + var prevIndefiniteErrIsSet bool + + // TODO(GODRIVERS-2579): When refactoring the "Execute" method, consider creating a separate method for the + // error handling logic below. This will remove the necessity of the "checkError" goto label. checkError: var perr error switch tt := err.(type) { @@ -641,8 +647,9 @@ func (op Operation) Execute(ctx context.Context) error { // If the error is no longer retryable and has the NoWritesPerformed label, then we should // set the error to the "previous indefinite error" unless the current error is already the // "previous indefinite error". After reseting, repeat the error check. - if tt.HasErrorLabel(NoWritesPerformed) && !errors.Is(tt, prevIndefiniteErr) { + if tt.HasErrorLabel(NoWritesPerformed) && !prevIndefiniteErrIsSet { err = prevIndefiniteErr + prevIndefiniteErrIsSet = true goto checkError } @@ -737,8 +744,9 @@ func (op Operation) Execute(ctx context.Context) error { // If the error is no longer retryable and has the NoWritesPerformed label, then we should // set the error to the "previous indefinite error" unless the current error is already the // "previous indefinite error". After reseting, repeat the error check. - if tt.HasErrorLabel(NoWritesPerformed) && !errors.Is(tt, prevIndefiniteErr) { + if tt.HasErrorLabel(NoWritesPerformed) && !prevIndefiniteErrIsSet { err = prevIndefiniteErr + prevIndefiniteErrIsSet = true goto checkError } From f958132f940285b96ae8fd879b3ccbfc6d1567bc Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Thu, 8 Dec 2022 09:53:57 -0700 Subject: [PATCH 16/17] Update x/mongo/driver/operation.go Co-authored-by: Qingyang Hu <103950869+qingyang-hu@users.noreply.github.com> --- x/mongo/driver/operation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index a211bff73a..4ced8a8557 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -610,7 +610,7 @@ func (op Operation) Execute(ctx context.Context) error { // a case in the switch statement below. var prevIndefiniteErrIsSet bool - // TODO(GODRIVERS-2579): When refactoring the "Execute" method, consider creating a separate method for the + // TODO(GODRIVER-2579): When refactoring the "Execute" method, consider creating a separate method for the // error handling logic below. This will remove the necessity of the "checkError" goto label. checkError: var perr error From d5ca59e5b8022cf83540e3f7d20c963a60daffc5 Mon Sep 17 00:00:00 2001 From: Preston Vasquez <24281431+prestonvasquez@users.noreply.github.com> Date: Thu, 8 Dec 2022 10:19:28 -0700 Subject: [PATCH 17/17] Update x/mongo/driver/operation.go Co-authored-by: Benjamin Rewis <32186188+benjirewis@users.noreply.github.com> --- x/mongo/driver/operation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/mongo/driver/operation.go b/x/mongo/driver/operation.go index 4ced8a8557..bbc25ecf22 100644 --- a/x/mongo/driver/operation.go +++ b/x/mongo/driver/operation.go @@ -402,7 +402,7 @@ func (op Operation) Execute(ctx context.Context) error { case labeledError: // If the "prevIndefiniteErr" is nil, then the current error is the first error encountered // during the retry attempt cycle. We must persist the first error in the case where all - // succeeding errors are labeled "NoWritesPerformed", which would otherwise raise nil as the + // following errors are labeled "NoWritesPerformed", which would otherwise raise nil as the // error. if prevIndefiniteErr == nil { prevIndefiniteErr = err