CSHARP-4827: Don't add RetryableWriteError label on pre 4.4 mongos.

rstam · rstam · commit 830b83a2bfdf · 2024-02-20T15:43:30.000-08:00
diff --git a/src/MongoDB.Driver.Core/Core/Operations/RetryabilityHelper.cs b/src/MongoDB.Driver.Core/Core/Operations/RetryabilityHelper.cs
@@ -16,7 +16,9 @@
 using System;
 using System.Collections.Generic;
 using MongoDB.Bson;
+using MongoDB.Driver.Core.Connections;
 using MongoDB.Driver.Core.Misc;
+using MongoDB.Driver.Core.Servers;
 
 namespace MongoDB.Driver.Core.Operations
 {
@@ -90,9 +92,9 @@ static RetryabilityHelper()
         }
 
         // public static methods
-        public static void AddRetryableWriteErrorLabelIfRequired(MongoException exception, int maxWireVersion)
+        public static void AddRetryableWriteErrorLabelIfRequired(MongoException exception, ConnectionDescription connectionDescription)
         {
-            if (ShouldRetryableWriteExceptionLabelBeAdded(exception, maxWireVersion))
+            if (ShouldRetryableWriteExceptionLabelBeAdded(exception, connectionDescription))
             {
                 exception.AddErrorLabel(RetryableWriteErrorLabel);
             }
@@ -172,18 +174,22 @@ private static bool IsNetworkException(Exception exception)
             return exception is MongoConnectionException mongoConnectionException && mongoConnectionException.IsNetworkException;
         }
 
-        private static bool ShouldRetryableWriteExceptionLabelBeAdded(Exception exception, int maxWireVersion)
+        private static bool ShouldRetryableWriteExceptionLabelBeAdded(Exception exception, ConnectionDescription connectionDescription)
         {
             if (IsNetworkException(exception))
             {
                 return true;
             }
 
+            var maxWireVersion = connectionDescription.MaxWireVersion;
             if (Feature.ServerReturnsRetryableWriteErrorLabel.IsSupported(maxWireVersion))
             {
                 return false;
             }
 
+            // on all servers from 4.4 on we would have returned false in the previous if statement
+            // so from this point on we know we are connected to a pre 4.4 server
+
             if (__retryableWriteExceptions.Contains(exception.GetType()))
             {
                 return true;
@@ -199,29 +205,33 @@ private static bool ShouldRetryableWriteExceptionLabelBeAdded(Exception exceptio
                 }
             }
 
-            var writeConcernException = exception as MongoWriteConcernException;
-            if (writeConcernException != null)
+            var serverType = connectionDescription.HelloResult.ServerType;
+            if (serverType != ServerType.ShardRouter)
             {
-                var writeConcernError = writeConcernException.WriteConcernResult.Response.GetValue("writeConcernError", null)?.AsBsonDocument;
-                if (writeConcernError != null)
+                var writeConcernException = exception as MongoWriteConcernException;
+                if (writeConcernException != null)
                 {
-                    var code = (ServerErrorCode)writeConcernError.GetValue("code", -1).AsInt32;
-                    switch (code)
+                    var writeConcernError = writeConcernException.WriteConcernResult.Response.GetValue("writeConcernError", null)?.AsBsonDocument;
+                    if (writeConcernError != null)
                     {
-                        case ServerErrorCode.InterruptedAtShutdown:
-                        case ServerErrorCode.InterruptedDueToReplStateChange:
-                        case ServerErrorCode.LegacyNotPrimary:
-                        case ServerErrorCode.NotWritablePrimary:
-                        case ServerErrorCode.NotPrimaryNoSecondaryOk:
-                        case ServerErrorCode.NotPrimaryOrSecondary:
-                        case ServerErrorCode.PrimarySteppedDown:
-                        case ServerErrorCode.ShutdownInProgress:
-                        case ServerErrorCode.HostNotFound:
-                        case ServerErrorCode.HostUnreachable:
-                        case ServerErrorCode.NetworkTimeout:
-                        case ServerErrorCode.SocketException:
-                        case ServerErrorCode.ExceededTimeLimit:
-                            return true;
+                        var code = (ServerErrorCode)writeConcernError.GetValue("code", -1).AsInt32;
+                        switch (code)
+                        {
+                            case ServerErrorCode.InterruptedAtShutdown:
+                            case ServerErrorCode.InterruptedDueToReplStateChange:
+                            case ServerErrorCode.LegacyNotPrimary:
+                            case ServerErrorCode.NotWritablePrimary:
+                            case ServerErrorCode.NotPrimaryNoSecondaryOk:
+                            case ServerErrorCode.NotPrimaryOrSecondary:
+                            case ServerErrorCode.PrimarySteppedDown:
+                            case ServerErrorCode.ShutdownInProgress:
+                            case ServerErrorCode.HostNotFound:
+                            case ServerErrorCode.HostUnreachable:
+                            case ServerErrorCode.NetworkTimeout:
+                            case ServerErrorCode.SocketException:
+                            case ServerErrorCode.ExceededTimeLimit:
+                                return true;
+                        }
                     }
                 }
             }
diff --git a/src/MongoDB.Driver.Core/Core/WireProtocol/CommandUsingCommandMessageWireProtocol.cs b/src/MongoDB.Driver.Core/Core/WireProtocol/CommandUsingCommandMessageWireProtocol.cs
@@ -147,7 +147,7 @@ public TCommandResult Execute(IConnection connection, CancellationToken cancella
             }
             catch (Exception exception)
             {
-                AddErrorLabelIfRequired(exception, connection.Description?.MaxWireVersion);
+                AddErrorLabelIfRequired(exception, connection.Description);
 
                 TransactionHelper.UnpinServerIfNeededOnCommandException(_session, exception);
                 throw;
@@ -201,15 +201,15 @@ public async Task<TCommandResult> ExecuteAsync(IConnection connection, Cancellat
             }
             catch (Exception exception)
             {
-                AddErrorLabelIfRequired(exception, connection.Description?.MaxWireVersion);
+                AddErrorLabelIfRequired(exception, connection.Description);
 
                 TransactionHelper.UnpinServerIfNeededOnCommandException(_session, exception);
                 throw;
             }
         }
 
         // private methods
-        private void AddErrorLabelIfRequired(Exception exception, int? maxWireVersion)
+        private void AddErrorLabelIfRequired(Exception exception, ConnectionDescription connectionDescription)
         {
             if (exception is MongoException mongoException)
             {
@@ -218,9 +218,9 @@ private void AddErrorLabelIfRequired(Exception exception, int? maxWireVersion)
                     mongoException.AddErrorLabel("TransientTransactionError");
                 }
 
-                if (RetryabilityHelper.IsCommandRetryable(_command) && maxWireVersion.HasValue)
+                if (RetryabilityHelper.IsCommandRetryable(_command) && connectionDescription != null)
                 {
-                    RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(mongoException, maxWireVersion.Value);
+                    RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(mongoException, connectionDescription);
                 }
             }
         }
diff --git a/tests/MongoDB.Driver.Core.Tests/Core/Operations/RetryabilityHelperTests.cs b/tests/MongoDB.Driver.Core.Tests/Core/Operations/RetryabilityHelperTests.cs
@@ -21,6 +21,7 @@
 using MongoDB.Driver.Core.Misc;
 using MongoDB.Driver.Core.TestHelpers;
 using Xunit;
+using MongoDB.Driver.Core.Connections;
 
 namespace MongoDB.Driver.Core.Operations
 {
@@ -45,8 +46,10 @@ public class RetryabilityHelperTests
         public void AddRetryableWriteErrorLabelIfRequired_should_add_RetryableWriteError_for_MongoWriteConcernException_when_required(int errorCode, bool shouldAddErrorLabel)
         {
             var exception = CoreExceptionHelper.CreateMongoWriteConcernException(BsonDocument.Parse($"{{ writeConcernError : {{ code : {errorCode} }} }}"));
+            var maxWireVersion = Feature.ServerReturnsRetryableWriteErrorLabel.LastNotSupportedWireVersion;
+            var connectionDescription = OperationTestHelper.CreateConnectionDescription(maxWireVersion);
 
-            RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, Feature.ServerReturnsRetryableWriteErrorLabel.LastNotSupportedWireVersion);
+            RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, connectionDescription);
 
             var hasRetryableWriteErrorLabel = exception.HasErrorLabel("RetryableWriteError");
             hasRetryableWriteErrorLabel.Should().Be(shouldAddErrorLabel);
@@ -59,8 +62,9 @@ public void AddRetryableWriteErrorLabelIfRequired_should_add_RetryableWriteError
             var exception = (MongoException)CoreExceptionHelper.CreateException(typeof(MongoConnectionException));
             var feature = Feature.ServerReturnsRetryableWriteErrorLabel;
             var wireVersion = serverReturnsRetryableWriteErrorLabel ? feature.FirstSupportedWireVersion : feature.LastNotSupportedWireVersion;
+            var connectionDescription = OperationTestHelper.CreateConnectionDescription(wireVersion);
 
-            RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, wireVersion);
+            RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, connectionDescription);
 
             var hasRetryableWriteErrorLabel = exception.HasErrorLabel("RetryableWriteError");
             hasRetryableWriteErrorLabel.Should().BeTrue();
@@ -89,8 +93,10 @@ public void AddRetryableWriteErrorLabelIfRequired_should_add_RetryableWriteError
             {
                 exception = CoreExceptionHelper.CreateMongoCommandException((int)exceptionDescription);
             }
+            var maxWireVersion = Feature.ServerReturnsRetryableWriteErrorLabel.LastNotSupportedWireVersion;
+            var connectionDescription = OperationTestHelper.CreateConnectionDescription(maxWireVersion);
 
-            RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, Feature.ServerReturnsRetryableWriteErrorLabel.LastNotSupportedWireVersion);
+            RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(exception, connectionDescription);
 
             var hasRetryableWriteErrorLabel = exception.HasErrorLabel("RetryableWriteError");
             hasRetryableWriteErrorLabel.Should().Be(shouldAddErrorLabel);

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,9 @@`
`16`	`16`	`using System;`
`17`	`17`	`using System.Collections.Generic;`
`18`	`18`	`using MongoDB.Bson;`
	`19`	`+using MongoDB.Driver.Core.Connections;`
`19`	`20`	`using MongoDB.Driver.Core.Misc;`
	`21`	`+using MongoDB.Driver.Core.Servers;`
`20`	`22`
`21`	`23`	`namespace MongoDB.Driver.Core.Operations`
`22`	`24`	`{`
`@@ -90,9 +92,9 @@ static RetryabilityHelper()`
`90`	`92`	`}`
`91`	`93`
`92`	`94`	`// public static methods`
`93`		`- public static void AddRetryableWriteErrorLabelIfRequired(MongoException exception, int maxWireVersion)`
	`95`	`+ public static void AddRetryableWriteErrorLabelIfRequired(MongoException exception, ConnectionDescription connectionDescription)`
`94`	`96`	`{`
`95`		`- if (ShouldRetryableWriteExceptionLabelBeAdded(exception, maxWireVersion))`
	`97`	`+ if (ShouldRetryableWriteExceptionLabelBeAdded(exception, connectionDescription))`
`96`	`98`	`{`
`97`	`99`	`exception.AddErrorLabel(RetryableWriteErrorLabel);`
`98`	`100`	`}`
`@@ -172,18 +174,22 @@ private static bool IsNetworkException(Exception exception)`
`172`	`174`	`return exception is MongoConnectionException mongoConnectionException && mongoConnectionException.IsNetworkException;`
`173`	`175`	`}`
`174`	`176`
`175`		`- private static bool ShouldRetryableWriteExceptionLabelBeAdded(Exception exception, int maxWireVersion)`
	`177`	`+ private static bool ShouldRetryableWriteExceptionLabelBeAdded(Exception exception, ConnectionDescription connectionDescription)`
`176`	`178`	`{`
`177`	`179`	`if (IsNetworkException(exception))`
`178`	`180`	`{`
`179`	`181`	`return true;`
`180`	`182`	`}`
`181`	`183`
	`184`	`+ var maxWireVersion = connectionDescription.MaxWireVersion;`
`182`	`185`	`if (Feature.ServerReturnsRetryableWriteErrorLabel.IsSupported(maxWireVersion))`
`183`	`186`	`{`
`184`	`187`	`return false;`
`185`	`188`	`}`
`186`	`189`
	`190`	`+ // on all servers from 4.4 on we would have returned false in the previous if statement`
	`191`	`+ // so from this point on we know we are connected to a pre 4.4 server`
	`192`	`+`
`187`	`193`	`if (__retryableWriteExceptions.Contains(exception.GetType()))`
`188`	`194`	`{`
`189`	`195`	`return true;`
`@@ -199,29 +205,33 @@ private static bool ShouldRetryableWriteExceptionLabelBeAdded(Exception exceptio`
`199`	`205`	`}`
`200`	`206`	`}`
`201`	`207`
`202`		`- var writeConcernException = exception as MongoWriteConcernException;`
`203`		`- if (writeConcernException != null)`
	`208`	`+ var serverType = connectionDescription.HelloResult.ServerType;`
	`209`	`+ if (serverType != ServerType.ShardRouter)`
`204`	`210`	`{`
`205`		`- var writeConcernError = writeConcernException.WriteConcernResult.Response.GetValue("writeConcernError", null)?.AsBsonDocument;`
`206`		`- if (writeConcernError != null)`
	`211`	`+ var writeConcernException = exception as MongoWriteConcernException;`
	`212`	`+ if (writeConcernException != null)`
`207`	`213`	`{`
`208`		`- var code = (ServerErrorCode)writeConcernError.GetValue("code", -1).AsInt32;`
`209`		`- switch (code)`
	`214`	`+ var writeConcernError = writeConcernException.WriteConcernResult.Response.GetValue("writeConcernError", null)?.AsBsonDocument;`
	`215`	`+ if (writeConcernError != null)`
`210`	`216`	`{`
`211`		`- case ServerErrorCode.InterruptedAtShutdown:`
`212`		`- case ServerErrorCode.InterruptedDueToReplStateChange:`
`213`		`- case ServerErrorCode.LegacyNotPrimary:`
`214`		`- case ServerErrorCode.NotWritablePrimary:`
`215`		`- case ServerErrorCode.NotPrimaryNoSecondaryOk:`
`216`		`- case ServerErrorCode.NotPrimaryOrSecondary:`
`217`		`- case ServerErrorCode.PrimarySteppedDown:`
`218`		`- case ServerErrorCode.ShutdownInProgress:`
`219`		`- case ServerErrorCode.HostNotFound:`
`220`		`- case ServerErrorCode.HostUnreachable:`
`221`		`- case ServerErrorCode.NetworkTimeout:`
`222`		`- case ServerErrorCode.SocketException:`
`223`		`- case ServerErrorCode.ExceededTimeLimit:`
`224`		`- return true;`
	`217`	`+ var code = (ServerErrorCode)writeConcernError.GetValue("code", -1).AsInt32;`
	`218`	`+ switch (code)`
	`219`	`+ {`
	`220`	`+ case ServerErrorCode.InterruptedAtShutdown:`
	`221`	`+ case ServerErrorCode.InterruptedDueToReplStateChange:`
	`222`	`+ case ServerErrorCode.LegacyNotPrimary:`
	`223`	`+ case ServerErrorCode.NotWritablePrimary:`
	`224`	`+ case ServerErrorCode.NotPrimaryNoSecondaryOk:`
	`225`	`+ case ServerErrorCode.NotPrimaryOrSecondary:`
	`226`	`+ case ServerErrorCode.PrimarySteppedDown:`
	`227`	`+ case ServerErrorCode.ShutdownInProgress:`
	`228`	`+ case ServerErrorCode.HostNotFound:`
	`229`	`+ case ServerErrorCode.HostUnreachable:`
	`230`	`+ case ServerErrorCode.NetworkTimeout:`
	`231`	`+ case ServerErrorCode.SocketException:`
	`232`	`+ case ServerErrorCode.ExceededTimeLimit:`
	`233`	`+ return true;`
	`234`	`+ }`
`225`	`235`	`}`
`226`	`236`	`}`
`227`	`237`	`}`
Original file line number	Diff line number	Diff line change
`@@ -147,7 +147,7 @@ public TCommandResult Execute(IConnection connection, CancellationToken cancella`
`147`	`147`	`}`
`148`	`148`	`catch (Exception exception)`
`149`	`149`	`{`
`150`		`- AddErrorLabelIfRequired(exception, connection.Description?.MaxWireVersion);`
	`150`	`+ AddErrorLabelIfRequired(exception, connection.Description);`
`151`	`151`
`152`	`152`	`TransactionHelper.UnpinServerIfNeededOnCommandException(_session, exception);`
`153`	`153`	`throw;`
`@@ -201,15 +201,15 @@ public async Task<TCommandResult> ExecuteAsync(IConnection connection, Cancellat`
`201`	`201`	`}`
`202`	`202`	`catch (Exception exception)`
`203`	`203`	`{`
`204`		`- AddErrorLabelIfRequired(exception, connection.Description?.MaxWireVersion);`
	`204`	`+ AddErrorLabelIfRequired(exception, connection.Description);`
`205`	`205`
`206`	`206`	`TransactionHelper.UnpinServerIfNeededOnCommandException(_session, exception);`
`207`	`207`	`throw;`
`208`	`208`	`}`
`209`	`209`	`}`
`210`	`210`
`211`	`211`	`// private methods`
`212`		`- private void AddErrorLabelIfRequired(Exception exception, int? maxWireVersion)`
	`212`	`+ private void AddErrorLabelIfRequired(Exception exception, ConnectionDescription connectionDescription)`
`213`	`213`	`{`
`214`	`214`	`if (exception is MongoException mongoException)`
`215`	`215`	`{`
`@@ -218,9 +218,9 @@ private void AddErrorLabelIfRequired(Exception exception, int? maxWireVersion)`
`218`	`218`	`mongoException.AddErrorLabel("TransientTransactionError");`
`219`	`219`	`}`
`220`	`220`
`221`		`- if (RetryabilityHelper.IsCommandRetryable(_command) && maxWireVersion.HasValue)`
	`221`	`+ if (RetryabilityHelper.IsCommandRetryable(_command) && connectionDescription != null)`
`222`	`222`	`{`
`223`		`- RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(mongoException, maxWireVersion.Value);`
	`223`	`+ RetryabilityHelper.AddRetryableWriteErrorLabelIfRequired(mongoException, connectionDescription);`
`224`	`224`	`}`
`225`	`225`	`}`
`226`	`226`	`}`