Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add top-level error labels to write concern error #766

Merged
merged 3 commits into from Jul 27, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -32,6 +32,7 @@
import com.mongodb.event.CommandStartedEvent;
import com.mongodb.event.CommandSucceededEvent;
import com.mongodb.lang.Nullable;
import org.bson.BsonArray;
import org.bson.BsonBinaryReader;
import org.bson.BsonDocument;
import org.bson.BsonInt32;
Expand Down Expand Up @@ -264,7 +265,16 @@ public static MongoException createSpecialException(final BsonDocument response,
} else if (isNotPrimaryError(errorCode, errorMessage)) {
return new MongoNotPrimaryException(response, serverAddress);
} else if (response.containsKey("writeConcernError")) {
return createSpecialException(response.getDocument("writeConcernError"), serverAddress, "errmsg");
BsonDocument writeConcernError = response.getDocument("writeConcernError");
Copy link
Member

@rozza rozza Jul 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is more complex than it needs to be:

MongoException error = createSpecialException(response.getDocument("writeConcernError"), serverAddress, "errmsg");
for (BsonValue errorLabel : response.getArray("errorLabels", new BsonArray())) {
    error.addLabel(errorLabel.asString().getValue());
}
return error;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's so much better. Thanks. Added a couple of checks to a conditional protecting the loop. One is necessary for correctness, the other just to document intent.

Done.

// Copy error labels from top-level response into write concern error response, so that they are considered in the recursive
// call
if (writeConcernError.getArray("errorLabels", new BsonArray()).isEmpty()
&& !response.getArray("errorLabels", new BsonArray()).isEmpty()) {
// clone so as not to mutate the caller's document
writeConcernError = writeConcernError.clone()
.append("errorLabels", response.getArray("errorLabels"));
}
return createSpecialException(writeConcernError, serverAddress, "errmsg");
} else {
return null;
}
Expand Down
@@ -0,0 +1,205 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added these two new unified tests for this, in scope of already existing DRIVERS-1385 ticket.

"description": "retryable-writes bulkWrite serverErrors",
"schemaVersion": "1.0",
"runOnRequirements": [
{
"minServerVersion": "3.6",
"topologies": [
"replicaset"
]
}
],
"createEntities": [
{
"client": {
"id": "client0",
"useMultipleMongoses": false,
"observeEvents": [
"commandStartedEvent"
]
}
},
{
"database": {
"id": "database0",
"client": "client0",
"databaseName": "retryable-writes-tests"
}
},
{
"collection": {
"id": "collection0",
"database": "database0",
"collectionName": "coll"
}
}
],
"initialData": [
{
"collectionName": "coll",
"databaseName": "retryable-writes-tests",
"documents": [
{
"_id": 1,
"x": 11
},
{
"_id": 2,
"x": 22
}
]
}
],
"tests": [
{
"description": "BulkWrite succeeds after retryable writeConcernError in first batch",
"runOnRequirements": [
{
"minServerVersion": "4.0",
"topologies": [
"replicaset"
]
},
{
"minServerVersion": "4.1.7",
"topologies": [
"sharded-replicaset"
]
}
],
"operations": [
{
"name": "failPoint",
"object": "testRunner",
"arguments": {
"client": "client0",
"failPoint": {
"configureFailPoint": "failCommand",
"mode": {
"times": 1
},
"data": {
"failCommands": [
"insert"
],
"errorLabels": [
"RetryableWriteError"
],
"writeConcernError": {
"code": 91,
"errmsg": "Replication is being shut down"
}
}
}
}
},
{
"name": "bulkWrite",
"object": "collection0",
"arguments": {
"requests": [
{
"insertOne": {
"document": {
"_id": 3,
"x": 33
}
}
},
{
"deleteOne": {
"filter": {
"_id": 2
}
}
}
]
},
"expectResult": {
"deletedCount": 1,
"insertedCount": 1,
"matchedCount": 0,
"modifiedCount": 0,
"upsertedCount": 0,
"insertedIds": {
"$$unsetOrMatches": {
"0": 3
}
},
"upsertedIds": {}
}
}
],
"expectEvents": [
{
"client": "client0",
"events": [
{
"commandStartedEvent": {
"command": {
"insert": "coll",
"documents": [
{
"_id": 3,
"x": 33
}
]
},
"commandName": "insert",
"databaseName": "retryable-writes-tests"
}
},
{
"commandStartedEvent": {
"command": {
"insert": "coll",
"documents": [
{
"_id": 3,
"x": 33
}
]
},
"commandName": "insert",
"databaseName": "retryable-writes-tests"
}
},
{
"commandStartedEvent": {
"command": {
"delete": "coll",
"deletes": [
{
"q": {
"_id": 2
},
"limit": 1
}
]
},
"commandName": "delete",
"databaseName": "retryable-writes-tests"
}
}
]
}
],
"outcome": [
{
"collectionName": "coll",
"databaseName": "retryable-writes-tests",
"documents": [
{
"_id": 1,
"x": 11
},
{
"_id": 3,
"x": 33
}
]
}
]
}
]
}