Skip to content

Commit

Permalink
fix: hint should raise error on unacknowledged writes
Browse files Browse the repository at this point in the history
Raise error when hint option is provided on unacknowledged writes
against any server version.

NODE-2576
  • Loading branch information
emadum authored May 5, 2020
1 parent 2a3bb7c commit 665b352
Show file tree
Hide file tree
Showing 43 changed files with 122 additions and 2,249 deletions.
4 changes: 2 additions & 2 deletions lib/operations/find_and_modify.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ class FindAndModifyOperation extends OperationBase {
if (options.hint) {
// TODO: once this method becomes a CommandOperationV2 we will have the server
// in place to check.
const topology = coll.s.topology;
if (maxWireVersion(topology) < 8) {
const unacknowledgedWrite = options.writeConcern && options.writeConcern.w === 0;
if (unacknowledgedWrite || maxWireVersion(coll.s.topology) < 8) {
callback(
new MongoError('The current topology does not support a hint on findAndModify commands')
);
Expand Down
3 changes: 2 additions & 1 deletion lib/sdam/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,8 @@ function executeWriteOperation(args, options, callback) {
callback(new MongoError(`server ${server.name} does not support collation`));
return;
}
if (maxWireVersion(server) < 5) {
const unacknowledgedWrite = options.writeConcern && options.writeConcern.w === 0;
if (unacknowledgedWrite || maxWireVersion(server) < 5) {
if ((op === 'update' || op === 'remove') && ops.find(o => o.hint)) {
callback(new MongoError(`servers < 3.4 do not support hint on ${op}`));
return;
Expand Down
32 changes: 32 additions & 0 deletions test/spec/crud/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -302,3 +302,35 @@ specification, such as ``insertedId`` (for InsertOneResult), ``insertedIds``
(for InsertManyResult), and ``upsertedCount`` (for UpdateResult). Drivers that
do not implement these fields should ignore them when comparing ``actual`` with
``expected``.

Prose Tests
===========

The following tests have not yet been automated, but MUST still be tested.

"errInfo" is propagated
-----------------------
Test that a writeConcernError "errInfo" is propagated to the user in whatever way is idiomatic to the driver (exception, error object, etc.). Using a 4.0+ server, set the following failpoint:

.. code:: javascript
{
"configureFailPoint": "failCommand",
"data": {
"failCommands": ["insert"],
"writeConcernError": {
"code": 100,
"codeName": "UnsatisfiableWriteConcern",
"errmsg": "Not enough data-bearing nodes",
"errInfo": {
"writeConcern": {
"w": 2,
"wtimeout": 0,
"provenance": "clientSupplied"
}
}
}
},
"mode": { "times": 1 }
}
Then, perform an insert on the same database. Assert that an error occurs and that the "errInfo" is accessible and matches the one set in the failpoint.
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
{
"runOn": [
{
"maxServerVersion": "4.3.3"
}
],
"data": [
{
"_id": 1,
Expand All @@ -25,7 +20,7 @@
"collection_name": "BulkWrite_delete_hint",
"tests": [
{
"description": "Unacknowledged bulkWrite deleteOne with hints fails with client-side error on server < 4.4",
"description": "Unacknowledged bulkWrite deleteOne with hints fails with client-side error",
"operations": [
{
"name": "bulkWrite",
Expand Down Expand Up @@ -64,7 +59,7 @@
"error": true
}
],
"expectations": {},
"expectations": [],
"outcome": {
"collection": {
"data": [
Expand All @@ -89,7 +84,7 @@
}
},
{
"description": "Unacknowledged bulkWrite deleteMany with hints fails with client-side error on server < 4.4",
"description": "Unacknowledged bulkWrite deleteMany with hints fails with client-side error",
"operations": [
{
"name": "bulkWrite",
Expand Down Expand Up @@ -132,7 +127,7 @@
"error": true
}
],
"expectations": {},
"expectations": [],
"outcome": {
"collection": {
"data": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
runOn:
# These tests assert that the driver raises a client-side error when the hint
# option is passed with an unacknowledged delete operation against a server
# that does not support hints in delete operations.
- { maxServerVersion: "4.3.3" }

data:
- {_id: 1, x: 11}
- {_id: 2, x: 22}
Expand All @@ -14,7 +8,7 @@ collection_name: &collection_name 'BulkWrite_delete_hint'

tests:
-
description: "Unacknowledged bulkWrite deleteOne with hints fails with client-side error on server < 4.4"
description: "Unacknowledged bulkWrite deleteOne with hints fails with client-side error"
operations:
-
name: "bulkWrite"
Expand All @@ -34,7 +28,7 @@ tests:
hint: &hint_doc { _id: 1 }
options: { ordered: true }
error: true
expectations: {}
expectations: []
outcome: &outcome
collection:
data:
Expand All @@ -43,7 +37,7 @@ tests:
- {_id: 3, x: 33 }
- {_id: 4, x: 44 }
-
description: "Unacknowledged bulkWrite deleteMany with hints fails with client-side error on server < 4.4"
description: "Unacknowledged bulkWrite deleteMany with hints fails with client-side error"
operations:
-
name: "bulkWrite"
Expand All @@ -62,5 +56,5 @@ tests:
hint: *hint_doc
options: { ordered: true }
error: true
expectations: {}
expectations: []
outcome: *outcome
174 changes: 0 additions & 174 deletions test/spec/crud/v2/unacknowledged-bulkWrite-delete-hint.json

This file was deleted.

Loading

0 comments on commit 665b352

Please sign in to comment.