From a46882b586dcce73000a892fbd967cf1786ec81c Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Wed, 16 Nov 2022 20:40:14 -0700 Subject: [PATCH 1/3] Fix MixedBulkWriteOperation such that it does not leak MongoWriteConcernWithResponseException to users JAVA-4775 --- .../operation/MixedBulkWriteOperation.java | 99 +++++++----- ...ixedBulkWriteOperationSpecification.groovy | 8 +- ...WriteConcernWithResponseExceptionTest.java | 31 ++++ ...WriteConcernWithResponseExceptionTest.java | 144 ++++++++++++++++++ .../client/RetryableWritesProseTest.java | 22 +-- 5 files changed, 251 insertions(+), 53 deletions(-) create mode 100644 driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/MongoWriteConcernWithResponseExceptionTest.java create mode 100644 driver-sync/src/test/functional/com/mongodb/client/MongoWriteConcernWithResponseExceptionTest.java diff --git a/driver-core/src/main/com/mongodb/internal/operation/MixedBulkWriteOperation.java b/driver-core/src/main/com/mongodb/internal/operation/MixedBulkWriteOperation.java index 233763afcce..116c544dee0 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/MixedBulkWriteOperation.java +++ b/driver-core/src/main/com/mongodb/internal/operation/MixedBulkWriteOperation.java @@ -180,32 +180,22 @@ public BulkWriteResult execute(final WriteBinding binding) { logRetryExecute(retryState); return withSourceAndConnection(binding::getWriteConnectionSource, true, (source, connection) -> { ConnectionDescription connectionDescription = connection.getDescription(); - int maxWireVersion = connectionDescription.getMaxWireVersion(); // attach `maxWireVersion` ASAP because it is used to check whether we can retry - retryState.attach(AttachmentKeys.maxWireVersion(), maxWireVersion, true); - BulkWriteTracker bulkWriteTracker = retryState.attachment(AttachmentKeys.bulkWriteTracker()) - .orElseThrow(Assertions::fail); + retryState.attach(AttachmentKeys.maxWireVersion(), connectionDescription.getMaxWireVersion(), true); SessionContext sessionContext = binding.getSessionContext(); WriteConcern writeConcern = getAppliedWriteConcern(sessionContext); - if (!retryState.isFirstAttempt() && !isRetryableWrite(retryWrites, writeConcern, source.getServerDescription(), - connectionDescription, sessionContext)) { - RuntimeException prospectiveFailedResult = (RuntimeException) retryState.exception().orElse(null); - retryState.breakAndThrowIfRetryAnd(() -> !(prospectiveFailedResult instanceof MongoWriteConcernWithResponseException)); - bulkWriteTracker.batch().ifPresent(bulkWriteBatch -> { - assertTrue(prospectiveFailedResult instanceof MongoWriteConcernWithResponseException); - bulkWriteBatch.addResult((BsonDocument) ((MongoWriteConcernWithResponseException) prospectiveFailedResult) - .getResponse()); - BulkWriteTracker.attachNext(retryState, bulkWriteBatch); - }); + if (!isRetryableWrite(retryWrites, getAppliedWriteConcern(sessionContext), + source.getServerDescription(), connectionDescription, sessionContext)) { + handleMongoWriteConcernWithResponseException(retryState, true); } validateWriteRequests(connectionDescription, bypassDocumentValidation, writeRequests, writeConcern); - if (!bulkWriteTracker.batch().isPresent()) { + if (!retryState.attachment(AttachmentKeys.bulkWriteTracker()).orElseThrow(Assertions::fail).batch().isPresent()) { BulkWriteTracker.attachNew(retryState, BulkWriteBatch.createBulkWriteBatch(namespace, source.getServerDescription(), connectionDescription, ordered, writeConcern, bypassDocumentValidation, retryWrites, writeRequests, sessionContext, comment, variables)); } logRetryExecute(retryState); - return executeBulkWriteBatch(retryState, binding, connection, maxWireVersion); + return executeBulkWriteBatch(retryState, binding, connection); }); }); try { @@ -226,33 +216,22 @@ public void executeAsync(final AsyncWriteBinding binding, final SingleResultCall withAsyncSourceAndConnection(binding::getWriteConnectionSource, true, funcCallback, (source, connection, releasingCallback) -> { ConnectionDescription connectionDescription = connection.getDescription(); - int maxWireVersion = connectionDescription.getMaxWireVersion(); // attach `maxWireVersion` ASAP because it is used to check whether we can retry - retryState.attach(AttachmentKeys.maxWireVersion(), maxWireVersion, true); - BulkWriteTracker bulkWriteTracker = retryState.attachment(AttachmentKeys.bulkWriteTracker()) - .orElseThrow(Assertions::fail); + retryState.attach(AttachmentKeys.maxWireVersion(), connectionDescription.getMaxWireVersion(), true); SessionContext sessionContext = binding.getSessionContext(); WriteConcern writeConcern = getAppliedWriteConcern(sessionContext); - if (!retryState.isFirstAttempt() && !isRetryableWrite(retryWrites, writeConcern, source.getServerDescription(), - connectionDescription, sessionContext)) { - Throwable prospectiveFailedResult = retryState.exception().orElse(null); - if (retryState.breakAndCompleteIfRetryAnd(() -> - !(prospectiveFailedResult instanceof MongoWriteConcernWithResponseException), releasingCallback)) { - return; - } - bulkWriteTracker.batch().ifPresent(bulkWriteBatch -> { - assertTrue(prospectiveFailedResult instanceof MongoWriteConcernWithResponseException); - bulkWriteBatch.addResult((BsonDocument) ((MongoWriteConcernWithResponseException) prospectiveFailedResult) - .getResponse()); - BulkWriteTracker.attachNext(retryState, bulkWriteBatch); - }); + if (!isRetryableWrite(retryWrites, getAppliedWriteConcern(sessionContext), + source.getServerDescription(), + connectionDescription, sessionContext) + && handleMongoWriteConcernWithResponseExceptionAsync(retryState, releasingCallback)) { + return; } if (validateWriteRequestsAndCompleteIfInvalid(connectionDescription, bypassDocumentValidation, writeRequests, writeConcern, releasingCallback)) { return; } try { - if (!bulkWriteTracker.batch().isPresent()) { + if (!retryState.attachment(AttachmentKeys.bulkWriteTracker()).orElseThrow(Assertions::fail).batch().isPresent()) { BulkWriteTracker.attachNew(retryState, BulkWriteBatch.createBulkWriteBatch(namespace, source.getServerDescription(), connectionDescription, ordered, writeConcern, bypassDocumentValidation, retryWrites, writeRequests, sessionContext, comment, variables)); @@ -262,17 +241,17 @@ public void executeAsync(final AsyncWriteBinding binding, final SingleResultCall return; } logRetryExecute(retryState); - executeBulkWriteBatchAsync(retryState, binding, connection, maxWireVersion, releasingCallback); + executeBulkWriteBatchAsync(retryState, binding, connection, releasingCallback); }); }).whenComplete(binding::release); retryingBulkWrite.get(exceptionTransformingCallback(errorHandlingCallback(callback, LOGGER))); } - private BulkWriteResult executeBulkWriteBatch(final RetryState retryState, final WriteBinding binding, final Connection connection, - final int maxWireVersion) { + private BulkWriteResult executeBulkWriteBatch(final RetryState retryState, final WriteBinding binding, final Connection connection) { BulkWriteTracker currentBulkWriteTracker = retryState.attachment(AttachmentKeys.bulkWriteTracker()) .orElseThrow(Assertions::fail); BulkWriteBatch currentBatch = currentBulkWriteTracker.batch().orElseThrow(Assertions::fail); + int maxWireVersion = connection.getDescription().getMaxWireVersion(); while (currentBatch.shouldProcessBatch()) { try { BsonDocument result = executeCommand(connection, currentBatch, binding); @@ -292,9 +271,10 @@ private BulkWriteResult executeBulkWriteBatch(final RetryState retryState, final currentBulkWriteTracker = BulkWriteTracker.attachNext(retryState, currentBatch); currentBatch = currentBulkWriteTracker.batch().orElseThrow(Assertions::fail); } catch (MongoException exception) { - if (!(retryState.isFirstAttempt() || (exception instanceof MongoWriteConcernWithResponseException))) { + if (!retryState.isFirstAttempt() && !(exception instanceof MongoWriteConcernWithResponseException)) { addRetryableWriteErrorLabel(exception, maxWireVersion); } + handleMongoWriteConcernWithResponseException(retryState, false); throw exception; } } @@ -307,13 +287,14 @@ private BulkWriteResult executeBulkWriteBatch(final RetryState retryState, final } private void executeBulkWriteBatchAsync(final RetryState retryState, final AsyncWriteBinding binding, final AsyncConnection connection, - final int maxWireVersion, final SingleResultCallback callback) { + final SingleResultCallback callback) { LoopState loopState = new LoopState(); AsyncCallbackRunnable loop = new AsyncCallbackLoop(loopState, iterationCallback -> { BulkWriteTracker currentBulkWriteTracker = retryState.attachment(AttachmentKeys.bulkWriteTracker()) .orElseThrow(Assertions::fail); loopState.attach(AttachmentKeys.bulkWriteTracker(), currentBulkWriteTracker, true); BulkWriteBatch currentBatch = currentBulkWriteTracker.batch().orElseThrow(Assertions::fail); + int maxWireVersion = connection.getDescription().getMaxWireVersion(); if (loopState.breakAndCompleteIf(() -> !currentBatch.shouldProcessBatch(), iterationCallback)) { return; } @@ -340,9 +321,12 @@ private void executeBulkWriteBatchAsync(final RetryState retryState, final Async } else { if (t instanceof MongoException) { MongoException exception = (MongoException) t; - if (!(retryState.isFirstAttempt() || (exception instanceof MongoWriteConcernWithResponseException))) { + if (!retryState.isFirstAttempt() && !(exception instanceof MongoWriteConcernWithResponseException)) { addRetryableWriteErrorLabel(exception, maxWireVersion); } + if (handleMongoWriteConcernWithResponseExceptionAsync(retryState, null)) { + return; + } } iterationCallback.onResult(null, t); } @@ -368,6 +352,41 @@ private void executeBulkWriteBatchAsync(final RetryState retryState, final Async }); } + private void handleMongoWriteConcernWithResponseException(final RetryState retryState, final boolean breakAndThrowIfDifferent) { + if (!retryState.isFirstAttempt()) { + RuntimeException prospectiveFailedResult = (RuntimeException) retryState.exception().orElse(null); + boolean prospectiveResultIsWriteConcernException = prospectiveFailedResult instanceof MongoWriteConcernWithResponseException; + retryState.breakAndThrowIfRetryAnd(() -> breakAndThrowIfDifferent && !prospectiveResultIsWriteConcernException); + if (prospectiveResultIsWriteConcernException) { + retryState.attachment(AttachmentKeys.bulkWriteTracker()).orElseThrow(Assertions::fail) + .batch().ifPresent(bulkWriteBatch -> { + bulkWriteBatch.addResult( + (BsonDocument) ((MongoWriteConcernWithResponseException) prospectiveFailedResult).getResponse()); + BulkWriteTracker.attachNext(retryState, bulkWriteBatch); + }); + } + } + } + + private boolean handleMongoWriteConcernWithResponseExceptionAsync(final RetryState retryState, + @Nullable final SingleResultCallback callback) { + if (!retryState.isFirstAttempt()) { + RuntimeException prospectiveFailedResult = (RuntimeException) retryState.exception().orElse(null); + boolean prospectiveResultIsWriteConcernException = prospectiveFailedResult instanceof MongoWriteConcernWithResponseException; + if (callback != null && retryState.breakAndCompleteIfRetryAnd(() -> !prospectiveResultIsWriteConcernException, callback)) { + return true; + } + if (prospectiveResultIsWriteConcernException) { + retryState.attachment(AttachmentKeys.bulkWriteTracker()).orElseThrow(Assertions::fail) + .batch().ifPresent(bulkWriteBatch -> { + bulkWriteBatch.addResult( + (BsonDocument) ((MongoWriteConcernWithResponseException) prospectiveFailedResult).getResponse()); + BulkWriteTracker.attachNext(retryState, bulkWriteBatch); + }); + } + } + return false; + } private BsonDocument executeCommand(final Connection connection, final BulkWriteBatch batch, final WriteBinding binding) { return connection.command(namespace.getDatabaseName(), batch.getCommand(), NO_OP_FIELD_NAME_VALIDATOR, diff --git a/driver-core/src/test/functional/com/mongodb/internal/operation/MixedBulkWriteOperationSpecification.groovy b/driver-core/src/test/functional/com/mongodb/internal/operation/MixedBulkWriteOperationSpecification.groovy index 89fac6a4123..7e7938acfe2 100644 --- a/driver-core/src/test/functional/com/mongodb/internal/operation/MixedBulkWriteOperationSpecification.groovy +++ b/driver-core/src/test/functional/com/mongodb/internal/operation/MixedBulkWriteOperationSpecification.groovy @@ -1151,16 +1151,16 @@ class MixedBulkWriteOperationSpecification extends OperationFunctionalSpecificat def originalException = new MongoSocketException('Some failure', new ServerAddress()) when: - testRetryableOperationThrowsOriginalError(operation, [[3, 6, 0], [3, 6, 0]], - [REPLICA_SET_PRIMARY, STANDALONE], originalException, async) + testRetryableOperationThrowsOriginalError(operation, [[3, 6, 0], [3, 6, 0], [3, 6, 0]], + [REPLICA_SET_PRIMARY, REPLICA_SET_PRIMARY, STANDALONE], originalException, async) then: Exception commandException = thrown() commandException == originalException when: - testRetryableOperationThrowsOriginalError(operation, [[3, 6, 0]], - [REPLICA_SET_PRIMARY], originalException, async, 1) + testRetryableOperationThrowsOriginalError(operation, [[3, 6, 0], [3, 6, 0]], + [REPLICA_SET_PRIMARY, REPLICA_SET_PRIMARY], originalException, async, 1) then: commandException = thrown() diff --git a/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/MongoWriteConcernWithResponseExceptionTest.java b/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/MongoWriteConcernWithResponseExceptionTest.java new file mode 100644 index 00000000000..6de9dc9052c --- /dev/null +++ b/driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/MongoWriteConcernWithResponseExceptionTest.java @@ -0,0 +1,31 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.mongodb.reactivestreams.client; + +import com.mongodb.reactivestreams.client.syncadapter.SyncMongoClient; +import org.junit.Test; + +/** + * See {@link com.mongodb.client.MongoWriteConcernWithResponseExceptionTest}. + */ +public class MongoWriteConcernWithResponseExceptionTest { + @Test + public void doesNotLeak() throws InterruptedException { + com.mongodb.client.MongoWriteConcernWithResponseExceptionTest.doesNotLeak( + mongoClientSettings -> new SyncMongoClient(MongoClients.create(mongoClientSettings))); + } +} diff --git a/driver-sync/src/test/functional/com/mongodb/client/MongoWriteConcernWithResponseExceptionTest.java b/driver-sync/src/test/functional/com/mongodb/client/MongoWriteConcernWithResponseExceptionTest.java new file mode 100644 index 00000000000..6f90b3f5f01 --- /dev/null +++ b/driver-sync/src/test/functional/com/mongodb/client/MongoWriteConcernWithResponseExceptionTest.java @@ -0,0 +1,144 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.mongodb.client; + +import com.mongodb.Function; +import com.mongodb.MongoClientSettings; +import com.mongodb.MongoWriteConcernException; +import com.mongodb.ServerAddress; +import com.mongodb.assertions.Assertions; +import com.mongodb.event.CommandEvent; +import com.mongodb.event.CommandFailedEvent; +import com.mongodb.event.CommandListener; +import com.mongodb.event.CommandSucceededEvent; +import com.mongodb.internal.connection.MongoWriteConcernWithResponseException; +import org.bson.BsonArray; +import org.bson.BsonDocument; +import org.bson.BsonInt32; +import org.bson.BsonString; +import org.bson.Document; +import org.junit.Test; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static com.mongodb.ClusterFixture.isDiscoverableReplicaSet; +import static com.mongodb.ClusterFixture.serverVersionAtLeast; +import static com.mongodb.client.Fixture.getDefaultDatabaseName; +import static com.mongodb.client.Fixture.getMongoClientSettingsBuilder; +import static java.util.Collections.singletonList; +import static org.junit.Assert.assertThrows; +import static org.junit.Assume.assumeTrue; + +/** + * Tests in this class check that the internal {@link MongoWriteConcernWithResponseException} does not leak from our API. + */ +public final class MongoWriteConcernWithResponseExceptionTest { + /** + * This test is similar to {@link RetryableWritesProseTest#originalErrorMustBePropagatedIfNoWritesPerformed()}. + * The difference is in the assertions, it also verifies situations when `writeConcernError` happens on the first attempt + * and on the last attempt. + */ + @Test + public void doesNotLeak() throws InterruptedException { + doesNotLeak(MongoClients::create); + } + + public static void doesNotLeak(final Function clientCreator) throws InterruptedException { + BsonDocument writeConcernErrorFpDoc = new BsonDocument() + .append("configureFailPoint", new BsonString("failCommand")) + .append("mode", new BsonDocument() + .append("times", new BsonInt32(2))) + .append("data", new BsonDocument() + .append("writeConcernError", new BsonDocument() + .append("code", new BsonInt32(91)) + .append("errorLabels", new BsonArray(Stream.of("RetryableWriteError") + .map(BsonString::new).collect(Collectors.toList()))) + .append("errmsg", new BsonString("")) + ) + .append("failCommands", new BsonArray(singletonList(new BsonString("insert"))))); + BsonDocument noWritesPerformedFpDoc = new BsonDocument() + .append("configureFailPoint", new BsonString("failCommand")) + .append("mode", new BsonDocument() + .append("times", new BsonInt32(1))) + .append("data", new BsonDocument() + .append("failCommands", new BsonArray(singletonList(new BsonString("insert")))) + .append("errorCode", new BsonInt32(10107)) + .append("errorLabels", new BsonArray(Stream.of("RetryableWriteError", "NoWritesPerformed") + .map(BsonString::new).collect(Collectors.toList())))); + doesNotLeak(clientCreator, writeConcernErrorFpDoc, true, noWritesPerformedFpDoc); + doesNotLeak(clientCreator, noWritesPerformedFpDoc, false, writeConcernErrorFpDoc); + } + + @SuppressWarnings("try") + private static void doesNotLeak( + final Function clientCreator, + final BsonDocument firstAttemptFpDoc, + final boolean firstAttemptCommandSucceededEvent, + final BsonDocument lastAttemptFpDoc) throws InterruptedException { + assumeTrue(serverVersionAtLeast(6, 0) && isDiscoverableReplicaSet()); + ServerAddress primaryServerAddress = Fixture.getPrimary(); + CompletableFuture futureFailPointFromListener = new CompletableFuture<>(); + CommandListener commandListener = new CommandListener() { + private final AtomicBoolean configureFailPoint = new AtomicBoolean(true); + + @Override + public void commandSucceeded(final CommandSucceededEvent event) { + if (firstAttemptCommandSucceededEvent) { + enableLastAttemptFp(event); + } + } + + @Override + public void commandFailed(final CommandFailedEvent event) { + if (!firstAttemptCommandSucceededEvent) { + enableLastAttemptFp(event); + } + } + + private void enableLastAttemptFp(final CommandEvent event) { + if (event.getCommandName().equals("insert") && configureFailPoint.compareAndSet(true, false)) { + Assertions.assertTrue(futureFailPointFromListener.complete(FailPoint.enable(lastAttemptFpDoc, primaryServerAddress))); + } + } + }; + try (MongoClient client = clientCreator.apply(getMongoClientSettingsBuilder() + .retryWrites(true) + .addCommandListener(commandListener) + .applyToServerSettings(builder -> builder.heartbeatFrequency(50, TimeUnit.MILLISECONDS)) + .build()); + FailPoint ignored = FailPoint.enable(firstAttemptFpDoc, primaryServerAddress)) { + MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) + .getCollection("originalErrorMustBePropagatedIfNoWritesPerformed"); + collection.drop(); + assertThrows(MongoWriteConcernException.class, () -> { + // We want to see an exception indicating `writeConcernError`, + // but not in the form of `MongoWriteConcernWithResponseException`. + try { + collection.insertOne(new Document()); + } catch (MongoWriteConcernWithResponseException e) { + throw new AssertionError("The internal exception leaked.", e); + } + }); + } finally { + futureFailPointFromListener.thenAccept(FailPoint::close); + } + } +} diff --git a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java index 0d7e9cd2019..6f6f251ab21 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java @@ -20,6 +20,7 @@ import com.mongodb.MongoClientException; import com.mongodb.MongoClientSettings; import com.mongodb.MongoException; +import com.mongodb.MongoWriteConcernException; import com.mongodb.ServerAddress; import com.mongodb.assertions.Assertions; import com.mongodb.event.CommandListener; @@ -63,9 +64,9 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; +import static org.junit.jupiter.api.Assertions.assertThrows; /** * See @@ -233,7 +234,9 @@ public void commandSucceeded(final CommandSucceededEvent event) { .append("writeConcernError", new BsonDocument() .append("code", new BsonInt32(91)) .append("errorLabels", new BsonArray(Stream.of("RetryableWriteError") - .map(BsonString::new).collect(Collectors.toList())))) + .map(BsonString::new).collect(Collectors.toList()))) + .append("errmsg", new BsonString("")) + ) .append("failCommands", new BsonArray(singletonList(new BsonString("insert"))))); try (MongoClient client = clientCreator.apply(getMongoClientSettingsBuilder() .retryWrites(true) @@ -246,13 +249,14 @@ public void commandSucceeded(final CommandSucceededEvent event) { MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) .getCollection("originalErrorMustBePropagatedIfNoWritesPerformed"); collection.drop(); - try { - collection.insertOne(new Document()); - } catch (MongoException e) { - assertEquals(e.getCode(), 91); - return; - } - fail("must not reach"); + assertThrows(MongoWriteConcernException.class, () -> { + try { + collection.insertOne(new Document()); + } catch (MongoWriteConcernException e) { + assertEquals(91, e.getCode()); + throw e; + } + }); } finally { futureFailPointFromListener.thenAccept(FailPoint::close); } From 5da59c203e0d21fa63ff07066b14ff207a814370 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Mon, 21 Nov 2022 19:30:15 -0700 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Jeff Yemin --- .../com/mongodb/client/RetryableWritesProseTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java index 6f6f251ab21..ef76eacf634 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java @@ -249,7 +249,8 @@ public void commandSucceeded(final CommandSucceededEvent event) { MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) .getCollection("originalErrorMustBePropagatedIfNoWritesPerformed"); collection.drop(); - assertThrows(MongoWriteConcernException.class, () -> { + MongoWriteConcernException e = assertThrows(MongoWriteConcernException.class, () -> collection.insertOne(new Document())); + assertEquals(91, e.getCode()); try { collection.insertOne(new Document()); } catch (MongoWriteConcernException e) { From 478caf5d6c588cdac3ad3bc999bf3a3f964b1e98 Mon Sep 17 00:00:00 2001 From: Valentin Kovalenko Date: Mon, 21 Nov 2022 19:33:30 -0700 Subject: [PATCH 3/3] Fix the broken code: the proposed change was problematic --- .../com/mongodb/client/RetryableWritesProseTest.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java index ef76eacf634..eb3d83ee12d 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java @@ -251,13 +251,6 @@ public void commandSucceeded(final CommandSucceededEvent event) { collection.drop(); MongoWriteConcernException e = assertThrows(MongoWriteConcernException.class, () -> collection.insertOne(new Document())); assertEquals(91, e.getCode()); - try { - collection.insertOne(new Document()); - } catch (MongoWriteConcernException e) { - assertEquals(91, e.getCode()); - throw e; - } - }); } finally { futureFailPointFromListener.thenAccept(FailPoint::close); }