PYTHON-3388 Propagate Original Error for Write Errors Labeled NoWritesPerformed (#1117)

Author: juliusgeo

pymongo/mongo_client.py

@@ -1408,12 +1408,18 @@ def is_retrying():
                 if retryable_error:
                     session._unpin()
                 if not retryable_error or (is_retrying() and not multiple_retries):
-                    raise
+                    if exc.has_error_label("NoWritesPerformed") and last_error:
+                        raise last_error from exc
+                    else:
+                        raise
                 if bulk:
                     bulk.retrying = True
                 else:
                     retrying = True
-                last_error = exc
+                if not exc.has_error_label("NoWritesPerformed"):
+                    last_error = exc
+                if last_error is None:
+                    last_error = exc
 
     @_csot.apply
     def _retryable_read(self, func, read_pref, session, address=None, retryable=True):

test/retryable_writes/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": []
+        }
+      ]
+    }
+  ]
+}

test/test_retryable_writes.py

@@ -26,6 +26,7 @@
 from test.utils import (
     CMAPListener,
     DeprecationFilter,
+    EventListener,
     OvertCommandListener,
     TestCreator,
     rs_or_single_client,
@@ -45,6 +46,7 @@
 )
 from pymongo.mongo_client import MongoClient
 from pymongo.monitoring import (
+    CommandSucceededEvent,
     ConnectionCheckedOutEvent,
     ConnectionCheckOutFailedEvent,
     ConnectionCheckOutFailedReason,
@@ -64,6 +66,26 @@
 _TEST_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), "retryable_writes", "legacy")
 
 
+class InsertEventListener(EventListener):
+    def succeeded(self, event: CommandSucceededEvent) -> None:
+        super(InsertEventListener, self).succeeded(event)
+        if (
+            event.command_name == "insert"
+            and event.reply.get("writeConcernError", {}).get("code", None) == 91
+        ):
+            client_context.client.admin.command(
+                {
+                    "configureFailPoint": "failCommand",
+                    "mode": {"times": 1},
+                    "data": {
+                        "errorCode": 10107,
+                        "errorLabels": ["RetryableWriteError", "NoWritesPerformed"],
+                        "failCommands": ["insert"],
+                    },
+                }
+            )
+
+
 class TestAllScenarios(SpecRunner):
     RUN_ON_LOAD_BALANCER = True
     RUN_ON_SERVERLESS = True
@@ -581,6 +603,43 @@ def test_pool_paused_error_is_retryable(self):
         failed = cmd_listener.failed_events
         self.assertEqual(1, len(failed), msg)
 
+    @client_context.require_failCommand_fail_point
+    @client_context.require_replica_set
+    @client_context.require_version_min(
+        6, 0, 0
+    )  # the spec requires that this prose test only be run on 6.0+
+    @client_knobs(heartbeat_frequency=0.05, min_heartbeat_interval=0.05)
+    def test_returns_original_error_code(
+        self,
+    ):
+        cmd_listener = InsertEventListener()
+        client = rs_or_single_client(retryWrites=True, event_listeners=[cmd_listener])
+        client.test.test.drop()
+        self.addCleanup(client.close)
+        cmd_listener.reset()
+        client.admin.command(
+            {
+                "configureFailPoint": "failCommand",
+                "mode": {"times": 1},
+                "data": {
+                    "writeConcernError": {
+                        "code": 91,
+                        "errorLabels": ["RetryableWriteError"],
+                    },
+                    "failCommands": ["insert"],
+                },
+            }
+        )
+        with self.assertRaises(WriteConcernError) as exc:
+            client.test.test.insert_one({"_id": 1})
+        self.assertEqual(exc.exception.code, 91)
+        client.admin.command(
+            {
+                "configureFailPoint": "failCommand",
+                "mode": "off",
+            }
+        )
+
 
 # TODO: Make this a real integration test where we stepdown the primary.
 class TestRetryableWritesTxnNumber(IgnoreDeprecationsTest):