PYTHON-2984 Fix retry behavior for bulk write writeConcernError (#800)

Author: ShaneHarvey

pymongo/bulk.py

@@ -28,7 +28,7 @@
                             validate_is_document_type,
                             validate_ok_for_replace,
                             validate_ok_for_update)
-from pymongo.helpers import _RETRYABLE_ERROR_CODES
+from pymongo.helpers import _RETRYABLE_ERROR_CODES, _get_wce_doc
 from pymongo.collation import validate_collation_or_none
 from pymongo.errors import (BulkWriteError,
                             ConfigurationError,
@@ -119,9 +119,9 @@ def _merge_command(run, full_result, offset, result):
             replacement["op"] = run.ops[idx]
             full_result["writeErrors"].append(replacement)
 
-    wc_error = result.get("writeConcernError")
-    if wc_error:
-        full_result["writeConcernErrors"].append(wc_error)
+    wce = _get_wce_doc(result)
+    if wce:
+        full_result["writeConcernErrors"].append(wce)
 
 
 def _raise_bulk_write_error(full_result):

pymongo/helpers.py

@@ -188,6 +188,18 @@ def _raise_write_concern_error(error):
         error.get("errmsg"), error.get("code"), error)
 
 
+def _get_wce_doc(result):
+    """Return the writeConcernError or None."""
+    wce = result.get("writeConcernError")
+    if wce:
+        # The server reports errorLabels at the top level but it's more
+        # convenient to attach it to the writeConcernError doc itself.
+        error_labels = result.get("errorLabels")
+        if error_labels:
+            wce["errorLabels"] = error_labels
+    return wce
+
+
 def _check_write_command_response(result):
     """Backward compatibility helper for write command error handling.
     """
@@ -196,12 +208,9 @@ def _check_write_command_response(result):
     if write_errors:
         _raise_last_write_error(write_errors)
 
-    error = result.get("writeConcernError")
-    if error:
-        error_labels = result.get("errorLabels")
-        if error_labels:
-            error.update({'errorLabels': error_labels})
-        _raise_write_concern_error(error)
+    wce = _get_wce_doc(result)
+    if wce:
+        _raise_write_concern_error(wce)
 
 
 def _fields_list_to_dict(fields, option_name):

test/retryable_writes/bulkWrite-errorLabels.json test/retryable_writes/legacy/bulkWrite-errorLabels.json

@@ -119,12 +119,12 @@
           "failCommands": [
             "insert"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },

test/retryable_writes/bulkWrite-serverErrors.json test/retryable_writes/legacy/bulkWrite-serverErrors.json

@@ -75,12 +75,12 @@
           "failCommands": [
             "delete"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },

test/retryable_writes/bulkWrite.json test/retryable_writes/legacy/bulkWrite.json

@@ -81,12 +81,12 @@
           "failCommands": [
             "findAndModify"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },

test/retryable_writes/deleteMany.json test/retryable_writes/legacy/deleteMany.json

@@ -85,12 +85,12 @@
           "failCommands": [
             "findAndModify"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },

test/retryable_writes/deleteOne-errorLabels.json test/retryable_writes/legacy/deleteOne-errorLabels.json

@@ -86,12 +86,12 @@
           "failCommands": [
             "findAndModify"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },

test/retryable_writes/deleteOne-serverErrors.json test/retryable_writes/legacy/deleteOne-serverErrors.json

@@ -92,12 +92,12 @@
           "failCommands": [
             "insert"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },

test/retryable_writes/deleteOne.json test/retryable_writes/legacy/deleteOne.json

@@ -761,12 +761,12 @@
           "failCommands": [
             "insert"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 11600,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },
@@ -812,12 +812,12 @@
           "failCommands": [
             "insert"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 11602,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },
@@ -863,12 +863,12 @@
           "failCommands": [
             "insert"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 189,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },
@@ -914,12 +914,12 @@
           "failCommands": [
             "insert"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },
@@ -965,12 +965,12 @@
           "failCommands": [
             "insert"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },

test/retryable_writes/findOneAndDelete-errorLabels.json test/retryable_writes/legacy/findOneAndDelete-errorLabels.json

@@ -85,12 +85,12 @@
           "failCommands": [
             "update"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },

test/retryable_writes/findOneAndDelete-serverErrors.json test/retryable_writes/legacy/findOneAndDelete-serverErrors.json

@@ -86,12 +86,12 @@
           "failCommands": [
             "update"
           ],
+          "errorLabels": [
+            "RetryableWriteError"
+          ],
           "writeConcernError": {
             "code": 91,
-            "errmsg": "Replication is being shut down",
-            "errorLabels": [
-              "RetryableWriteError"
-            ]
+            "errmsg": "Replication is being shut down"
           }
         }
       },