SERVER-30049 applyOperation_inlock() allows exceptions from Collection::insertDocument() to percolate to caller

This specifically addresses the case where a WriteConflictException is thrown from insertDocument().
Converting this exception into a status will not allow the write conflict retry loops in callers to
work properly.

(cherry picked from commit f0d41183e735546c83ff96d7d5afc11b9c94cb9f)

3 files changed, 96 insertions, 7 deletions

diff --git a/jstests/replsets/apply_ops_insert_write_conflict_nonatomic.js b/jstests/replsets/apply_ops_insert_write_conflict_nonatomic.js
new file mode 100644
index 00000000000..2e91de8637a
--- /dev/null
+++ b/jstests/replsets/apply_ops_insert_write_conflict_nonatomic.js
@@ -0,0 +1,10 @@
+(function() {
+    'use strict';
+
+    load("jstests/replsets/libs/apply_ops_insert_write_conflict.js");
+
+    new ApplyOpsInsertWriteConflictTest({
+        testName: 'apply_ops_insert_write_conflict_nonatomic',
+        atomic: false
+    }).run();
+}());
diff --git a/jstests/replsets/libs/apply_ops_insert_write_conflict.js b/jstests/replsets/libs/apply_ops_insert_write_conflict.js
new file mode 100644
index 00000000000..88b299c08b6
--- /dev/null
+++ b/jstests/replsets/libs/apply_ops_insert_write_conflict.js
@@ -0,0 +1,84 @@
+/**
+ * Sets up a test for WriteConflictException handling in applyOps with an insert workload.
+ */
+var ApplyOpsInsertWriteConflictTest = function(options) {
+    'use strict';
+
+    if (!(this instanceof ApplyOpsInsertWriteConflictTest)) {
+        return new ApplyOpsInsertWriteConflictTest(options);
+    }
+
+    // Capture the 'this' reference
+    var self = this;
+
+    self.options = options;
+
+    /**
+     * Runs the test.
+     */
+    this.run = function() {
+        var options = this.options;
+
+        var replTest = new ReplSetTest({nodes: 1});
+        replTest.startSet();
+        replTest.initiate();
+
+        var primary = replTest.getPrimary();
+        var primaryDB = primary.getDB('test');
+
+        var t = primaryDB.getCollection(options.testName);
+        t.drop();
+
+        assert.commandWorked(primaryDB.createCollection(t.getName()));
+
+        var numOps = 1000;
+        var ops = Array(numOps).fill('ignored').map((unused, i) => {
+            return {op: 'i', ns: t.getFullName(), o: {_id: i}};
+        });
+
+        if (!options.atomic) {
+            // Adding a command to the list of operations to prevent the applyOps command from
+            // applying
+            // all the operations atomically.
+            ops.push({ns: "test.$cmd", op: "c", o: {applyOps: []}});
+            numOps++;
+        }
+
+        // Probabilities for WCE are chosen based on empirical testing.
+        // The probability for WCE during an atomic applyOps should be much smaller than that for
+        // the non-atomic case because we have to attempt to re-apply the entire batch of 'numOps'
+        // operations on WCE in the atomic case.
+        var probability = (options.atomic ? 0.1 : 5.0) / numOps;
+
+        // Set up failpoint to trigger WriteConflictException during write operations.
+        assert.commandWorked(
+            primaryDB.adminCommand({setParameter: 1, traceWriteConflictExceptions: true}));
+        assert.commandWorked(primaryDB.adminCommand({
+            configureFailPoint: 'WTWriteConflictException',
+            mode: {activationProbability: probability}
+        }));
+
+        // This logs each operation being applied.
+        var previousLogLevel =
+            assert.commandWorked(primaryDB.setLogLevel(3, 'replication')).was.replication.verbosity;
+
+        var applyOpsResult = primaryDB.adminCommand({applyOps: ops});
+
+        // Reset log level.
+        primaryDB.setLogLevel(previousLogLevel, 'replication');
+
+        assert.eq(
+            numOps,
+            applyOpsResult.applied,
+            'number of operations applied did not match list of generated insert operations. ' +
+                'applyOps result: ' + tojson(applyOpsResult));
+        applyOpsResult.results.forEach((operationSucceeded, i) => {
+            assert(operationSucceeded,
+                   'applyOps failed: operation with index ' + i + ' failed: operation: ' +
+                       tojson(ops[i], '', true) + '. applyOps result: ' + tojson(applyOpsResult));
+        });
+        assert.commandWorked(applyOpsResult);
+
+        replTest.stopSet();
+    };
+};
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 69bc38cd3e5..43a1eba5042 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -838,7 +838,6 @@ Status applyOperation_inlock(OperationContext* txn,
             // 2. If okay, commit
             // 3. If not, do upsert (and commit)
             // 4. If both !Ok, return status
-            Status status{ErrorCodes::NotYetInitialized, ""};
 
             // We cannot rely on a DuplicateKey error if we'repart of a larger transaction, because
             // that would require the transaction to abort. So instead, use upsert in that case.
@@ -846,12 +845,8 @@ Status applyOperation_inlock(OperationContext* txn,
 
             if (!needToDoUpsert) {
                 WriteUnitOfWork wuow(txn);
-                try {
-                    OpDebug* const nullOpDebug = nullptr;
-                    status = collection->insertDocument(txn, o, nullOpDebug, true);
-                } catch (DBException dbe) {
-                    status = dbe.toStatus();
-                }
+                OpDebug* const nullOpDebug = nullptr;
+                auto status = collection->insertDocument(txn, o, nullOpDebug, true);
                 if (status.isOK()) {
                     wuow.commit();
                 } else if (status == ErrorCodes::DuplicateKey) {