DRIVERS-2501 Break NoWritesPerformed-Only Error Sequence

source/retryable-writes/retryable-writes.rst

@@ -405,7 +405,8 @@ If a retry attempt also fails, drivers MUST update their topology according to
 the SDAM spec (see: `Error Handling`_). If an error would not allow the caller
 to infer that an attempt was made (e.g. connection pool exception originating
 from the driver) or the error is labeled "NoWritesPerformed", the error from
-the previous attempt should be raised.
+the previous attempt should be raised. If all server errors are labeled
+"NoWritesPerformed", then the first error should be raised.
 
 The above rules are implemented in the following pseudo-code:
 
@@ -454,29 +455,40 @@ The above rules are implemented in the following pseudo-code:
     while true {
       try {
         return executeCommand(server, retryableCommand);
-      } catch (Exception originalError) {
-        handleError(originalError);
+      } catch (Exception currentError) {
+        handleError(currentError);
 
         /* If the error has a RetryableWriteError label, remember the exception
          * and proceed with retrying the operation.
          *
          * IllegalOperation (code 20) with errmsg starting with "Transaction
          * numbers" MUST be re-raised with an actionable error message.
          */
-        if (!originalError.hasErrorLabel("RetryableWriteError")) {
-          if ( originalError.code == 20 && originalError.errmsg.startsWith("Transaction numbers") ) {
-            originalError.errmsg = "This MongoDB deployment does not support retryable...";
+        if (!currentError.hasErrorLabel("RetryableWriteError")) {
+          if ( currentError.code == 20 && originalError.errmsg.startsWith("Transaction numbers") ) {
+            currentError.errmsg = "This MongoDB deployment does not support retryable...";
           }
-          throw originalError
+          throw currentError;
+        }
+
+        /*
+         * 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 (previousError == null) {
+          previousError = currentError;
         }
 
         /*
          * For exceptions that originate from the driver (e.g. no socket available
          * from the connection pool), we should raise the previous error if there
          * was one.
          */
-        if (originalError is not DriverException && ! originalError.hasErrorLabel("NoWritesPerformed")) {
-          previousError = originalError;
+        if (currentError is not DriverException && ! originalError.hasErrorLabel("NoWritesPerformed")) {
+          previousError = currentError;
         }
       }
 
@@ -489,12 +501,12 @@ The above rules are implemented in the following pseudo-code:
         throw previousError;
       }
 
-      /* If the server selected for retrying is too old, throw the original error.
+      /* If the server selected for retrying is too old, throw the previous error.
        * The caller can then infer that an attempt was made and failed. This case
        * is very rare, and likely means that the cluster is in the midst of a
        * downgrade. */
       if ( ! isRetryableWritesSupported(server)) {
-        throw originalError;
+        throw previousError;
       }
 
       /* CSOT is enabled and the operation has timed out. */
@@ -810,6 +822,8 @@ inconsistent with the server and potentially confusing to developers.
 Changelog
 =========
 
+:2022-11-17: Add logic for persisting "currentError" as "previousError" on first
+             retry attempt, avoiding raising "null" errors.
 :2022-11-09: CLAM must apply both events and log messages.
 :2022-10-18: When CSOT is enabled multiple retry attempts may occur.
 :2022-10-05: Remove spec front matter and reformat changelog.

source/retryable-writes/tests/unified/insertOne-noWritesPerformedError.json

@@ -0,0 +1,90 @@
+{
+  "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": []
+        }
+      ]
+    }
+  ]
+}

source/retryable-writes/tests/unified/insertOne-noWritesPerformedError.yml

@@ -0,0 +1,54 @@
+description: "retryable-writes insertOne noWritesPerformedErrors"
+
+schemaVersion: "1.0"
+
+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
+
+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: []