diff options
33 files changed, 149 insertions, 125 deletions
diff --git a/src/mongo/db/auth/sasl_commands.cpp b/src/mongo/db/auth/sasl_commands.cpp index fdf625b4979..00cb221a6cc 100644 --- a/src/mongo/db/auth/sasl_commands.cpp +++ b/src/mongo/db/auth/sasl_commands.cpp @@ -334,7 +334,7 @@ bool CmdSaslContinue::run(OperationContext* opCtx, } Status status = doSaslContinue(opCtx, session, cmdObj, &result); - CommandHelpers::appendCommandStatus(result, status); + CommandHelpers::appendCommandStatusNoThrow(result, status); if (mechanism.isDone()) { audit::logAuthentication( diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index 74977309411..6bfc1bbcd5c 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -91,7 +91,7 @@ BSONObj CommandHelpers::runCommandDirectly(OperationContext* opCtx, const OpMsgR } catch (const DBException& ex) { auto body = crb.getBodyBuilder(); body.resetToEmpty(); - appendCommandStatus(body, ex.toStatus()); + appendCommandStatusNoThrow(body, ex.toStatus()); } return BSONObj(bb.release()); } @@ -186,7 +186,13 @@ Command* CommandHelpers::findCommand(StringData name) { } bool CommandHelpers::appendCommandStatus(BSONObjBuilder& result, const Status& status) { - appendCommandStatus(result, status.isOK(), status.reason()); + uassertStatusOK(status); + appendSimpleCommandStatus(result, true); + return true; +} + +bool CommandHelpers::appendCommandStatusNoThrow(BSONObjBuilder& result, const Status& status) { + appendSimpleCommandStatus(result, status.isOK(), status.reason()); BSONObj tmp = result.asTempObj(); if (!status.isOK() && !tmp.hasField("code")) { result.append("code", status.code()); @@ -198,9 +204,9 @@ bool CommandHelpers::appendCommandStatus(BSONObjBuilder& result, const Status& s return status.isOK(); } -void CommandHelpers::appendCommandStatus(BSONObjBuilder& result, - bool ok, - const std::string& errmsg) { +void CommandHelpers::appendSimpleCommandStatus(BSONObjBuilder& result, + bool ok, + const std::string& errmsg) { BSONObj tmp = result.asTempObj(); bool have_ok = tmp.hasField("ok"); bool need_errmsg = !ok && !tmp.hasField("errmsg"); @@ -219,7 +225,7 @@ bool CommandHelpers::extractOrAppendOk(BSONObjBuilder& reply) { return okField.trueValue(); } // Missing "ok" field is an implied success. - CommandHelpers::appendCommandStatus(reply, true); + CommandHelpers::appendSimpleCommandStatus(reply, true); return true; } @@ -350,7 +356,7 @@ void CommandReplyBuilder::fillFrom(const Status& status) { reset(); } auto bob = getBodyBuilder(); - CommandHelpers::appendCommandStatus(bob, status); + CommandHelpers::appendCommandStatusNoThrow(bob, status); } ////////////////////////////////////////////////////////////// @@ -384,7 +390,7 @@ private: try { BSONObjBuilder bob = result->getBodyBuilder(); bool ok = _command->run(opCtx, _dbName, _request->body, bob); - CommandHelpers::appendCommandStatus(bob, ok); + CommandHelpers::appendSimpleCommandStatus(bob, ok); } catch (const ExceptionFor<ErrorCodes::Unauthorized>& e) { CommandHelpers::logAuthViolation(opCtx, this, *_request, e.code()); throw; @@ -521,7 +527,7 @@ bool ErrmsgCommandDeprecated::run(OperationContext* opCtx, std::string errmsg; auto ok = errmsgRun(opCtx, db, cmdObj, errmsg, result); if (!errmsg.empty()) { - CommandHelpers::appendCommandStatus(result, ok, errmsg); + CommandHelpers::appendSimpleCommandStatus(result, ok, errmsg); } return ok; } diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index b0573ab3a97..1bb669945c8 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -90,12 +90,25 @@ struct CommandHelpers { static Command* findCommand(StringData name); - // Helper for setting errmsg and ok field in command result object. - static void appendCommandStatus(BSONObjBuilder& result, - bool ok, - const std::string& errmsg = {}); + /** + * Helper for setting errmsg and ok field in command result object. + * + * This should generally only be called from the command dispatch code or to finish off the + * result of serializing a reply BSONObj in the case when it isn't going directly into a real + * command reply to be returned to the user. + */ + static void appendSimpleCommandStatus(BSONObjBuilder& result, + bool ok, + const std::string& errmsg = {}); + + /** + * Adds the status fields to command replies. + * + * Calling this inside of commands to produce their reply is now deprecated. Just throw instead. + */ + static bool appendCommandStatusNoThrow(BSONObjBuilder& result, const Status& status); - // @return s.isOK() + // About to be deleted static bool appendCommandStatus(BSONObjBuilder& result, const Status& status); /** diff --git a/src/mongo/db/commands/apply_ops_cmd.cpp b/src/mongo/db/commands/apply_ops_cmd.cpp index 976a85bcec8..d78414e95ea 100644 --- a/src/mongo/db/commands/apply_ops_cmd.cpp +++ b/src/mongo/db/commands/apply_ops_cmd.cpp @@ -267,7 +267,7 @@ public: << repl::ApplyOps::kOplogApplicationModeFieldName)); } - auto applyOpsStatus = CommandHelpers::appendCommandStatus( + auto applyOpsStatus = CommandHelpers::appendCommandStatusNoThrow( result, repl::applyOps(opCtx, dbname, cmdObj, oplogApplicationMode, &result)); return applyOpsStatus; diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp index bf1edf90884..0e08effe965 100644 --- a/src/mongo/db/commands/create_indexes.cpp +++ b/src/mongo/db/commands/create_indexes.cpp @@ -369,19 +369,15 @@ public: // that day, to avoid data corruption due to lack of index cleanup. opCtx->recoveryUnit()->abandonSnapshot(); dbLock.relockWithMode(MODE_X); - if (!repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, ns)) { - return CommandHelpers::appendCommandStatus( - result, - Status(ErrorCodes::NotMaster, - str::stream() - << "Not primary while creating background indexes in " - << ns.ns() - << ": cleaning up index build failure due to " - << e.toString())); - } } catch (...) { std::terminate(); } + uassert(ErrorCodes::NotMaster, + str::stream() << "Not primary while creating background indexes in " + << ns.ns() + << ": cleaning up index build failure due to " + << e.toString(), + repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, ns)); } throw; } diff --git a/src/mongo/db/commands/do_txn_cmd.cpp b/src/mongo/db/commands/do_txn_cmd.cpp index d0dd99ec645..15183f8524c 100644 --- a/src/mongo/db/commands/do_txn_cmd.cpp +++ b/src/mongo/db/commands/do_txn_cmd.cpp @@ -159,8 +159,8 @@ public: // was acknowledged. To fix this, we should wait for replication of the node’s last applied // OpTime if the last write operation was a no-op write. - auto doTxnStatus = - CommandHelpers::appendCommandStatus(result, doTxn(opCtx, dbname, cmdObj, &result)); + auto doTxnStatus = CommandHelpers::appendCommandStatusNoThrow( + result, doTxn(opCtx, dbname, cmdObj, &result)); return doTxnStatus; } diff --git a/src/mongo/db/commands/get_last_error.cpp b/src/mongo/db/commands/get_last_error.cpp index 7d8b277e798..2cb45c3b445 100644 --- a/src/mongo/db/commands/get_last_error.cpp +++ b/src/mongo/db/commands/get_last_error.cpp @@ -177,7 +177,7 @@ public: Status status = bsonExtractOpTimeField(cmdObj, "wOpTime", &lastOpTime); if (!status.isOK()) { result.append("badGLE", cmdObj); - return CommandHelpers::appendCommandStatus(result, status); + return CommandHelpers::appendCommandStatusNoThrow(result, status); } } else { return CommandHelpers::appendCommandStatus( @@ -195,7 +195,7 @@ public: FieldParser::extract(cmdObj, wElectionIdField, &electionId, &errmsg); if (!extracted) { result.append("badGLE", cmdObj); - CommandHelpers::appendCommandStatus(result, false, errmsg); + CommandHelpers::appendSimpleCommandStatus(result, false, errmsg); return false; } @@ -242,7 +242,7 @@ public: if (!status.isOK()) { result.append("badGLE", writeConcernDoc); - return CommandHelpers::appendCommandStatus(result, status); + return CommandHelpers::appendCommandStatusNoThrow(result, status); } // Don't wait for replication if there was an error reported - this matches 2.4 behavior @@ -298,7 +298,7 @@ public: return true; } - return CommandHelpers::appendCommandStatus(result, status); + return CommandHelpers::appendCommandStatusNoThrow(result, status); } } cmdGetLastError; diff --git a/src/mongo/db/commands/user_management_commands.cpp b/src/mongo/db/commands/user_management_commands.cpp index fd3519d4871..1fb2582eee5 100644 --- a/src/mongo/db/commands/user_management_commands.cpp +++ b/src/mongo/db/commands/user_management_commands.cpp @@ -1463,7 +1463,7 @@ public: return CommandHelpers::appendCommandStatus(result, status); } - CommandHelpers::appendCommandStatus(responseBuilder, Status::OK()); + CommandHelpers::appendSimpleCommandStatus(responseBuilder, true); auto swResponse = CursorResponse::parseFromBSON(responseBuilder.obj()); if (!swResponse.isOK()) { return CommandHelpers::appendCommandStatus(result, swResponse.getStatus()); diff --git a/src/mongo/db/commands/validate.cpp b/src/mongo/db/commands/validate.cpp index b6fd8a4b9e2..c98c02d629d 100644 --- a/src/mongo/db/commands/validate.cpp +++ b/src/mongo/db/commands/validate.cpp @@ -176,7 +176,7 @@ public: opCtx->waitForConditionOrInterrupt(_validationNotifier, lock); } } catch (AssertionException& e) { - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( result, {ErrorCodes::CommandFailed, str::stream() << "Exception during validation: " << e.toString()}); @@ -196,7 +196,7 @@ public: Status status = collection->validate(opCtx, level, background, std::move(collLk), &results, &result); if (!status.isOK()) { - return CommandHelpers::appendCommandStatus(result, status); + return CommandHelpers::appendCommandStatusNoThrow(result, status); } CollectionCatalogEntry* catalogEntry = collection->getCatalogEntry(); diff --git a/src/mongo/db/commands_test.cpp b/src/mongo/db/commands_test.cpp index 0dbee80db37..b2e9752e3fa 100644 --- a/src/mongo/db/commands_test.cpp +++ b/src/mongo/db/commands_test.cpp @@ -40,7 +40,7 @@ namespace { TEST(Commands, appendCommandStatusOK) { BSONObjBuilder actualResult; - CommandHelpers::appendCommandStatus(actualResult, Status::OK()); + CommandHelpers::appendCommandStatusNoThrow(actualResult, Status::OK()); BSONObjBuilder expectedResult; expectedResult.append("ok", 1.0); @@ -51,7 +51,7 @@ TEST(Commands, appendCommandStatusOK) { TEST(Commands, appendCommandStatusError) { BSONObjBuilder actualResult; const Status status(ErrorCodes::InvalidLength, "Response payload too long"); - CommandHelpers::appendCommandStatus(actualResult, status); + CommandHelpers::appendCommandStatusNoThrow(actualResult, status); BSONObjBuilder expectedResult; expectedResult.append("ok", 0.0); @@ -68,7 +68,7 @@ TEST(Commands, appendCommandStatusNoOverwrite) { actualResult.append("c", "d"); actualResult.append("ok", "not ok"); const Status status(ErrorCodes::InvalidLength, "Response payload too long"); - CommandHelpers::appendCommandStatus(actualResult, status); + CommandHelpers::appendCommandStatusNoThrow(actualResult, status); BSONObjBuilder expectedResult; expectedResult.append("a", "b"); @@ -84,7 +84,7 @@ TEST(Commands, appendCommandStatusNoOverwrite) { TEST(Commands, appendCommandStatusErrorExtraInfo) { BSONObjBuilder actualResult; const Status status(ErrorExtraInfoExample(123), "not again!"); - CommandHelpers::appendCommandStatus(actualResult, status); + CommandHelpers::appendCommandStatusNoThrow(actualResult, status); BSONObjBuilder expectedResult; expectedResult.append("ok", 0.0); @@ -362,7 +362,7 @@ struct IncrementTestCommon { CommandHelpers::extractOrAppendOk(bob); } catch (const DBException& e) { auto bob = crb.getBodyBuilder(); - CommandHelpers::appendCommandStatus(bob, e.toStatus()); + CommandHelpers::appendCommandStatusNoThrow(bob, e.toStatus()); } return BSONObj(bb.release()); }(); diff --git a/src/mongo/db/repl/repl_set_commands.cpp b/src/mongo/db/repl/repl_set_commands.cpp index 1c16974ff06..a6a97ceaa48 100644 --- a/src/mongo/db/repl/repl_set_commands.cpp +++ b/src/mongo/db/repl/repl_set_commands.cpp @@ -619,7 +619,8 @@ public: if (status == ErrorCodes::InvalidReplicaSetConfig) { result.append("configVersion", configVersion); } - return CommandHelpers::appendCommandStatus(result, status); + // TODO convert to uassertStatusOK once SERVER-34806 is done. + return CommandHelpers::appendCommandStatusNoThrow(result, status); } else { // Parsing error from UpdatePositionArgs. return CommandHelpers::appendCommandStatus(result, status); diff --git a/src/mongo/db/repl/repl_set_request_votes.cpp b/src/mongo/db/repl/repl_set_request_votes.cpp index b917a06ed09..7f7682831ef 100644 --- a/src/mongo/db/repl/repl_set_request_votes.cpp +++ b/src/mongo/db/repl/repl_set_request_votes.cpp @@ -66,7 +66,7 @@ private: status = ReplicationCoordinator::get(opCtx)->processReplSetRequestVotes( opCtx, parsedArgs, &response); response.addToBSON(&result); - return CommandHelpers::appendCommandStatus(result, status); + return CommandHelpers::appendCommandStatusNoThrow(result, status); } } cmdReplSetRequestVotes; diff --git a/src/mongo/db/s/balancer/migration_manager_test.cpp b/src/mongo/db/s/balancer/migration_manager_test.cpp index 89fa1da483d..ac54e2aa185 100644 --- a/src/mongo/db/s/balancer/migration_manager_test.cpp +++ b/src/mongo/db/s/balancer/migration_manager_test.cpp @@ -278,7 +278,7 @@ void MigrationManagerTest::expectMoveChunkCommand(const ChunkType& chunk, const ShardId& toShardId, const Status& returnStatus) { BSONObjBuilder resultBuilder; - CommandHelpers::appendCommandStatus(resultBuilder, returnStatus); + CommandHelpers::appendCommandStatusNoThrow(resultBuilder, returnStatus); expectMoveChunkCommand(chunk, toShardId, resultBuilder.obj()); } diff --git a/src/mongo/db/s/config/sharding_catalog_manager_create_collection_test.cpp b/src/mongo/db/s/config/sharding_catalog_manager_create_collection_test.cpp index e3ca4da2ffb..10caed254c6 100644 --- a/src/mongo/db/s/config/sharding_catalog_manager_create_collection_test.cpp +++ b/src/mongo/db/s/config/sharding_catalog_manager_create_collection_test.cpp @@ -78,7 +78,7 @@ public: ASSERT_EQUALS(expectedNs.toString(), nss.toString()); BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus(responseBuilder, response); + CommandHelpers::appendCommandStatusNoThrow(responseBuilder, response); return responseBuilder.obj(); }); } @@ -98,8 +98,8 @@ public: BSONObjBuilder responseBuilder; if (!collectionOptionsReponse.isOK()) { - CommandHelpers::appendCommandStatus(responseBuilder, - collectionOptionsReponse.getStatus()); + CommandHelpers::appendCommandStatusNoThrow(responseBuilder, + collectionOptionsReponse.getStatus()); } else { BSONObjBuilder listCollResponse(responseBuilder.subobjStart("cursor")); BSONArrayBuilder collArrayBuilder(listCollResponse.subarrayStart("firstBatch")); diff --git a/src/mongo/db/s/config/sharding_catalog_manager_shard_collection_test.cpp b/src/mongo/db/s/config/sharding_catalog_manager_shard_collection_test.cpp index 35b564c9139..76fb5c25fbb 100644 --- a/src/mongo/db/s/config/sharding_catalog_manager_shard_collection_test.cpp +++ b/src/mongo/db/s/config/sharding_catalog_manager_shard_collection_test.cpp @@ -102,7 +102,7 @@ public: } BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus(responseBuilder, response.getStatus()); + CommandHelpers::appendCommandStatusNoThrow(responseBuilder, response.getStatus()); return responseBuilder.obj(); }); } diff --git a/src/mongo/db/s/implicit_create_collection_test.cpp b/src/mongo/db/s/implicit_create_collection_test.cpp index 4199d6d324c..065e9108c90 100644 --- a/src/mongo/db/s/implicit_create_collection_test.cpp +++ b/src/mongo/db/s/implicit_create_collection_test.cpp @@ -59,7 +59,7 @@ public: ASSERT_EQ(expectedNss.ns(), request.cmdObj.firstElement().String()); BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus(responseBuilder, response); + CommandHelpers::appendCommandStatusNoThrow(responseBuilder, response); return responseBuilder.obj(); }); } diff --git a/src/mongo/db/service_entry_point_mongod.cpp b/src/mongo/db/service_entry_point_mongod.cpp index 13704ca7ddc..4e9cdb89d9c 100644 --- a/src/mongo/db/service_entry_point_mongod.cpp +++ b/src/mongo/db/service_entry_point_mongod.cpp @@ -94,7 +94,7 @@ public: if (!waitForWCStatus.isOK() && invocation->definition()->isUserManagementCommand()) { BSONObj temp = commandResponseBuilder.asTempObj().copy(); commandResponseBuilder.resetToEmpty(); - CommandHelpers::appendCommandStatus(commandResponseBuilder, waitForWCStatus); + CommandHelpers::appendCommandStatusNoThrow(commandResponseBuilder, waitForWCStatus); commandResponseBuilder.appendElementsUnique(temp); } } diff --git a/src/mongo/executor/network_test_env.cpp b/src/mongo/executor/network_test_env.cpp index a1b16aa192e..bc782c8ce8c 100644 --- a/src/mongo/executor/network_test_env.cpp +++ b/src/mongo/executor/network_test_env.cpp @@ -51,7 +51,7 @@ void NetworkTestEnv::onCommand(OnCommandFunction func) { if (resultStatus.isOK()) { BSONObjBuilder result(std::move(resultStatus.getValue())); - CommandHelpers::appendCommandStatus(result, resultStatus.getStatus()); + CommandHelpers::appendCommandStatusNoThrow(result, resultStatus.getStatus()); const RemoteCommandResponse response(result.obj(), BSONObj(), Milliseconds(1)); _mockNetwork->scheduleResponse(noi, _mockNetwork->now(), response); @@ -74,7 +74,7 @@ void NetworkTestEnv::onCommandWithMetadata(OnCommandWithMetadataFunction func) { if (cmdResponseStatus.isOK()) { BSONObjBuilder result(std::move(cmdResponseStatus.data)); - CommandHelpers::appendCommandStatus(result, cmdResponseStatus.status); + CommandHelpers::appendCommandStatusNoThrow(result, cmdResponseStatus.status); const RemoteCommandResponse response( result.obj(), cmdResponseStatus.metadata, Milliseconds(1)); diff --git a/src/mongo/s/catalog/sharding_catalog_client_impl.cpp b/src/mongo/s/catalog/sharding_catalog_client_impl.cpp index 8987bc20c41..5175e44a1c9 100644 --- a/src/mongo/s/catalog/sharding_catalog_client_impl.cpp +++ b/src/mongo/s/catalog/sharding_catalog_client_impl.cpp @@ -657,12 +657,12 @@ bool ShardingCatalogClientImpl::runUserManagementWriteCommand(OperationContext* if (initialCmdHadWriteConcern) { Status status = writeConcern.parse(writeConcernElement.Obj()); if (!status.isOK()) { - return CommandHelpers::appendCommandStatus(*result, status); + return CommandHelpers::appendCommandStatusNoThrow(*result, status); } if (!(writeConcern.wNumNodes == 1 || writeConcern.wMode == WriteConcernOptions::kMajority)) { - return CommandHelpers::appendCommandStatus( + return CommandHelpers::appendCommandStatusNoThrow( *result, {ErrorCodes::InvalidOptions, str::stream() << "Invalid replication write concern. User management write " @@ -701,13 +701,15 @@ bool ShardingCatalogClientImpl::runUserManagementWriteCommand(OperationContext* Shard::RetryPolicy::kNotIdempotent); if (!response.isOK()) { - return CommandHelpers::appendCommandStatus(*result, response.getStatus()); + return CommandHelpers::appendCommandStatusNoThrow(*result, response.getStatus()); } if (!response.getValue().commandStatus.isOK()) { - return CommandHelpers::appendCommandStatus(*result, response.getValue().commandStatus); + return CommandHelpers::appendCommandStatusNoThrow(*result, + response.getValue().commandStatus); } if (!response.getValue().writeConcernStatus.isOK()) { - return CommandHelpers::appendCommandStatus(*result, response.getValue().writeConcernStatus); + return CommandHelpers::appendCommandStatusNoThrow(*result, + response.getValue().writeConcernStatus); } CommandHelpers::filterCommandReplyForPassthrough(response.getValue().response, result); @@ -731,7 +733,7 @@ bool ShardingCatalogClientImpl::runUserManagementReadCommand(OperationContext* o return resultStatus.getValue().commandStatus.isOK(); } - return CommandHelpers::appendCommandStatus(*result, resultStatus.getStatus()); + return CommandHelpers::appendCommandStatusNoThrow(*result, resultStatus.getStatus()); // XXX } Status ShardingCatalogClientImpl::applyChunkOpsDeprecated(OperationContext* opCtx, diff --git a/src/mongo/s/catalog/sharding_catalog_log_change_test.cpp b/src/mongo/s/catalog/sharding_catalog_log_change_test.cpp index 429a11ec39b..124b148cde6 100644 --- a/src/mongo/s/catalog/sharding_catalog_log_change_test.cpp +++ b/src/mongo/s/catalog/sharding_catalog_log_change_test.cpp @@ -109,7 +109,7 @@ protected: }); BSONObjBuilder createResponseBuilder; - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( createResponseBuilder, Status(ErrorCodes::NamespaceExists, "coll already exists")); expectConfigCollectionCreate( configHost, getConfigCollName(), _cappedSize, createResponseBuilder.obj()); @@ -146,7 +146,7 @@ protected: }); BSONObjBuilder createResponseBuilder; - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( createResponseBuilder, Status(ErrorCodes::ExceededTimeLimit, "operation timed out")); expectConfigCollectionCreate( configHost, getConfigCollName(), _cappedSize, createResponseBuilder.obj()); diff --git a/src/mongo/s/catalog/sharding_catalog_test.cpp b/src/mongo/s/catalog/sharding_catalog_test.cpp index dd844ddb41d..c8a15fb5c36 100644 --- a/src/mongo/s/catalog/sharding_catalog_test.cpp +++ b/src/mongo/s/catalog/sharding_catalog_test.cpp @@ -588,7 +588,7 @@ TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandSuccess) { rpc::TrackingMetadata::removeTrackingData(request.metadata)); BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( responseBuilder, Status(ErrorCodes::UserNotFound, "User test@test not found")); return responseBuilder.obj(); }); @@ -661,7 +661,7 @@ TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandRewriteWriteConce rpc::TrackingMetadata::removeTrackingData(request.metadata)); BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( responseBuilder, Status(ErrorCodes::UserNotFound, "User test@test not found")); return responseBuilder.obj(); }); @@ -690,8 +690,8 @@ TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandNotMaster) { for (int i = 0; i < 3; ++i) { onCommand([](const RemoteCommandRequest& request) { BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus(responseBuilder, - Status(ErrorCodes::NotMaster, "not master")); + CommandHelpers::appendCommandStatusNoThrow(responseBuilder, + Status(ErrorCodes::NotMaster, "not master")); return responseBuilder.obj(); }); } @@ -724,8 +724,8 @@ TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandNotMasterRetrySuc ASSERT_EQUALS(host1, request.target); BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus(responseBuilder, - Status(ErrorCodes::NotMaster, "not master")); + CommandHelpers::appendCommandStatusNoThrow(responseBuilder, + Status(ErrorCodes::NotMaster, "not master")); // Ensure that when the catalog manager tries to retarget after getting the // NotMaster response, it will get back a new target. @@ -1126,12 +1126,18 @@ TEST_F(ShardingCatalogClientTest, UpdateDatabaseExceededTimeLimit) { }); onCommand([host1](const RemoteCommandRequest& request) { - ASSERT_EQUALS(host1, request.target); + try { + ASSERT_EQUALS(host1, request.target); - BatchedCommandResponse response; - response.setStatus({ErrorCodes::ExceededTimeLimit, "operation timed out"}); + BatchedCommandResponse response; + response.setStatus({ErrorCodes::ExceededTimeLimit, "operation timed out"}); - return response.toBSON(); + return response.toBSON(); + } catch (const DBException& ex) { + BSONObjBuilder bb; + CommandHelpers::appendCommandStatusNoThrow(bb, ex.toStatus()); + return bb.obj(); + } }); // Now wait for the updateDatabase call to return @@ -1211,7 +1217,7 @@ TEST_F(ShardingCatalogClientTest, ApplyChunkOpsDeprecatedSuccessfulWithCheck) { onCommand([&](const RemoteCommandRequest& request) { BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( responseBuilder, Status(ErrorCodes::DuplicateKey, "precondition failed")); return responseBuilder.obj(); }); @@ -1259,8 +1265,8 @@ TEST_F(ShardingCatalogClientTest, ApplyChunkOpsDeprecatedFailedWithCheck) { onCommand([&](const RemoteCommandRequest& request) { BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus(responseBuilder, - Status(ErrorCodes::NoMatchingDocument, "some error")); + CommandHelpers::appendCommandStatusNoThrow( + responseBuilder, Status(ErrorCodes::NoMatchingDocument, "some error")); return responseBuilder.obj(); }); diff --git a/src/mongo/s/catalog/sharding_catalog_write_retry_test.cpp b/src/mongo/s/catalog/sharding_catalog_write_retry_test.cpp index e15217ade8e..061d3bee333 100644 --- a/src/mongo/s/catalog/sharding_catalog_write_retry_test.cpp +++ b/src/mongo/s/catalog/sharding_catalog_write_retry_test.cpp @@ -368,10 +368,9 @@ TEST_F(UpdateRetryTest, NotMasterErrorReturnedPersistently) { for (int i = 0; i < 3; ++i) { onCommand([](const RemoteCommandRequest& request) { - BatchedCommandResponse response; - response.setStatus({ErrorCodes::NotMaster, "not master"}); - - return response.toBSON(); + BSONObjBuilder bb; + CommandHelpers::appendCommandStatusNoThrow(bb, {ErrorCodes::NotMaster, "not master"}); + return bb.obj(); }); } @@ -430,13 +429,13 @@ TEST_F(UpdateRetryTest, NotMasterOnceSuccessAfterRetry) { onCommand([&](const RemoteCommandRequest& request) { ASSERT_EQUALS(host1, request.target); - BatchedCommandResponse response; - response.setStatus({ErrorCodes::NotMaster, "not master"}); - // Ensure that when the catalog manager tries to retarget after getting the // NotMaster response, it will get back a new target. configTargeter()->setFindHostReturnValue(host2); - return response.toBSON(); + + BSONObjBuilder bb; + CommandHelpers::appendCommandStatusNoThrow(bb, {ErrorCodes::NotMaster, "not master"}); + return bb.obj(); }); onCommand([&](const RemoteCommandRequest& request) { diff --git a/src/mongo/s/commands/cluster_aggregate_test.cpp b/src/mongo/s/commands/cluster_aggregate_test.cpp index c6b79c7d346..4cd178d7bcb 100644 --- a/src/mongo/s/commands/cluster_aggregate_test.cpp +++ b/src/mongo/s/commands/cluster_aggregate_test.cpp @@ -106,7 +106,7 @@ protected: void expectAggReturnsError(ErrorCodes::Error code) { onCommandForPoolExecutor([code](const executor::RemoteCommandRequest& request) { BSONObjBuilder resBob; - CommandHelpers::appendCommandStatus(resBob, Status(code, "dummy error")); + CommandHelpers::appendCommandStatusNoThrow(resBob, Status(code, "dummy error")); return resBob.obj(); }); } diff --git a/src/mongo/s/commands/cluster_commands_helpers.cpp b/src/mongo/s/commands/cluster_commands_helpers.cpp index 9bdab8d5df0..777cbc141b8 100644 --- a/src/mongo/s/commands/cluster_commands_helpers.cpp +++ b/src/mongo/s/commands/cluster_commands_helpers.cpp @@ -350,7 +350,7 @@ bool appendRawResponses(OperationContext* opCtx, // Convert the error status back into the form of a command result and append it as the // raw response. BSONObjBuilder statusObjBob; - CommandHelpers::appendCommandStatus(statusObjBob, sendStatus); + CommandHelpers::appendCommandStatusNoThrow(statusObjBob, sendStatus); subobj.append(shardConnStr, statusObjBob.obj()); errors.push_back(std::make_pair(shardConnStr, sendStatus)); diff --git a/src/mongo/s/commands/cluster_explain.cpp b/src/mongo/s/commands/cluster_explain.cpp index a45043fe171..5b79d6e1f44 100644 --- a/src/mongo/s/commands/cluster_explain.cpp +++ b/src/mongo/s/commands/cluster_explain.cpp @@ -119,7 +119,7 @@ std::vector<Strategy::CommandResult> ClusterExplain::downconvert( } // Convert the error status back into the format of a command result. BSONObjBuilder statusObjBob; - CommandHelpers::appendCommandStatus(statusObjBob, status); + CommandHelpers::appendCommandStatusNoThrow(statusObjBob, status); // Get the Shard object in order to get the ConnectionString. auto shard = diff --git a/src/mongo/s/commands/cluster_get_last_error_cmd.cpp b/src/mongo/s/commands/cluster_get_last_error_cmd.cpp index f1e00de2930..8eab649a956 100644 --- a/src/mongo/s/commands/cluster_get_last_error_cmd.cpp +++ b/src/mongo/s/commands/cluster_get_last_error_cmd.cpp @@ -324,7 +324,7 @@ public: // Need to always return err result.appendNull("err"); - return CommandHelpers::appendCommandStatus( + return CommandHelpers::appendCommandStatusNoThrow( result, Status(ErrorCodes::WriteConcernFailed, "multiple write concern errors occurred")); } diff --git a/src/mongo/s/commands/cluster_multicast.cpp b/src/mongo/s/commands/cluster_multicast.cpp index d80979bf03b..54991980075 100644 --- a/src/mongo/s/commands/cluster_multicast.cpp +++ b/src/mongo/s/commands/cluster_multicast.cpp @@ -132,7 +132,7 @@ public: { BSONObjBuilder subbob(bob.subobjStart(host.toString())); - if (CommandHelpers::appendCommandStatus(subbob, response.status)) { + if (CommandHelpers::appendCommandStatusNoThrow(subbob, response.status)) { subbob.append("data", response.data); subbob.append("metadata", response.metadata); if (response.elapsedMillis) { diff --git a/src/mongo/s/commands/cluster_write_cmd.cpp b/src/mongo/s/commands/cluster_write_cmd.cpp index 2e413ca0c09..84a4036fe1e 100644 --- a/src/mongo/s/commands/cluster_write_cmd.cpp +++ b/src/mongo/s/commands/cluster_write_cmd.cpp @@ -306,7 +306,7 @@ private: try { BSONObjBuilder bob = result->getBodyBuilder(); bool ok = runImpl(opCtx, *_request, _batchedRequest, bob); - CommandHelpers::appendCommandStatus(bob, ok); + CommandHelpers::appendSimpleCommandStatus(bob, ok); } catch (const ExceptionFor<ErrorCodes::Unauthorized>&) { CommandHelpers::logAuthViolation(opCtx, this, *_request, ErrorCodes::Unauthorized); throw; diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index 612b5b61191..850ff70d14e 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -172,7 +172,7 @@ void execCommandClient(OperationContext* opCtx, help << "help for: " << c->getName() << " " << c->help(); auto body = result->getBodyBuilder(); body.append("help", help.str()); - CommandHelpers::appendCommandStatus(body, true, ""); + CommandHelpers::appendSimpleCommandStatus(body, true, ""); return; } @@ -186,7 +186,7 @@ void execCommandClient(OperationContext* opCtx, invocation->checkAuthorization(opCtx, request); } catch (const DBException& e) { auto body = result->getBodyBuilder(); - CommandHelpers::appendCommandStatus(body, e.toStatus()); + CommandHelpers::appendCommandStatusNoThrow(body, e.toStatus()); return; } @@ -200,7 +200,7 @@ void execCommandClient(OperationContext* opCtx, WriteConcernOptions::extractWCFromCommand(request.body); if (!wcResult.isOK()) { auto body = result->getBodyBuilder(); - CommandHelpers::appendCommandStatus(body, wcResult.getStatus()); + CommandHelpers::appendCommandStatusNoThrow(body, wcResult.getStatus()); return; } @@ -210,7 +210,7 @@ void execCommandClient(OperationContext* opCtx, // If we did not use the default writeConcern, one was provided when it shouldn't have // been by the user. auto body = result->getBodyBuilder(); - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( body, Status(ErrorCodes::InvalidOptions, "Command does not support writeConcern")); return; } @@ -225,7 +225,7 @@ void execCommandClient(OperationContext* opCtx, // TODO SERVER-33708. if (!invocation->supportsReadConcern(readConcernArgs.getLevel())) { auto body = result->getBodyBuilder(); - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( body, Status(ErrorCodes::InvalidOptions, str::stream() @@ -236,7 +236,7 @@ void execCommandClient(OperationContext* opCtx, if (!opCtx->getTxnNumber()) { auto body = result->getBodyBuilder(); - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( body, Status(ErrorCodes::InvalidOptions, "read concern snapshot is supported only in a transaction")); @@ -245,7 +245,7 @@ void execCommandClient(OperationContext* opCtx, if (readConcernArgs.getArgsAtClusterTime()) { auto body = result->getBodyBuilder(); - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( body, Status(ErrorCodes::InvalidOptions, "read concern snapshot is not supported with atClusterTime on mongos")); @@ -261,7 +261,7 @@ void execCommandClient(OperationContext* opCtx, auto metadataStatus = processCommandMetadata(opCtx, request.body); if (!metadataStatus.isOK()) { auto body = result->getBodyBuilder(); - CommandHelpers::appendCommandStatus(body, metadataStatus); + CommandHelpers::appendCommandStatusNoThrow(body, metadataStatus); return; } @@ -292,7 +292,7 @@ void runCommand(OperationContext* opCtx, auto const command = CommandHelpers::findCommand(commandName); if (!command) { ON_BLOCK_EXIT([opCtx, &builder] { appendRequiredFieldsToResponse(opCtx, &builder); }); - CommandHelpers::appendCommandStatus( + CommandHelpers::appendCommandStatusNoThrow( builder, {ErrorCodes::CommandNotFound, str::stream() << "no such cmd: " << commandName}); globalCommandRegistry()->incrementUnknownCommands(); @@ -338,7 +338,7 @@ void runCommand(OperationContext* opCtx, auto& readConcernArgs = repl::ReadConcernArgs::get(opCtx); auto readConcernParseStatus = readConcernArgs.initialize(request.body); if (!readConcernParseStatus.isOK()) { - CommandHelpers::appendCommandStatus(builder, readConcernParseStatus); + CommandHelpers::appendCommandStatusNoThrow(builder, readConcernParseStatus); return; } @@ -416,7 +416,7 @@ void runCommand(OperationContext* opCtx, LastError::get(opCtx->getClient()).setLastError(e.code(), e.reason()); crb.reset(); BSONObjBuilder bob = crb.getBodyBuilder(); - CommandHelpers::appendCommandStatus(bob, e.toStatus()); + CommandHelpers::appendCommandStatusNoThrow(bob, e.toStatus()); appendRequiredFieldsToResponse(opCtx, &bob); } } @@ -565,7 +565,7 @@ DbResponse Strategy::clientCommand(OperationContext* opCtx, const Message& m) { } reply->reset(); auto bob = reply->getInPlaceReplyBuilder(0); - CommandHelpers::appendCommandStatus(bob, ex.toStatus()); + CommandHelpers::appendCommandStatusNoThrow(bob, ex.toStatus()); appendRequiredFieldsToResponse(opCtx, &bob); } diff --git a/src/mongo/s/query/cluster_find_test.cpp b/src/mongo/s/query/cluster_find_test.cpp index 91c948f536f..e588521c80b 100644 --- a/src/mongo/s/query/cluster_find_test.cpp +++ b/src/mongo/s/query/cluster_find_test.cpp @@ -98,7 +98,7 @@ protected: void expectFindReturnsError(ErrorCodes::Error code) { onCommandForPoolExecutor([code](const executor::RemoteCommandRequest& request) { BSONObjBuilder resBob; - CommandHelpers::appendCommandStatus(resBob, Status(code, "dummy error")); + CommandHelpers::appendCommandStatusNoThrow(resBob, Status(code, "dummy error")); return resBob.obj(); }); } diff --git a/src/mongo/s/sharding_router_test_fixture.cpp b/src/mongo/s/sharding_router_test_fixture.cpp index 69cbd4b4ac1..78cda1a545a 100644 --- a/src/mongo/s/sharding_router_test_fixture.cpp +++ b/src/mongo/s/sharding_router_test_fixture.cpp @@ -522,7 +522,7 @@ void ShardingTestFixture::expectCount(const HostAndPort& configHost, checkReadConcern(request.cmdObj, Timestamp(0, 0), repl::OpTime::kUninitializedTerm); BSONObjBuilder responseBuilder; - CommandHelpers::appendCommandStatus(responseBuilder, response.getStatus()); + CommandHelpers::appendCommandStatusNoThrow(responseBuilder, response.getStatus()); return responseBuilder.obj(); }); } diff --git a/src/mongo/s/write_ops/batch_write_exec_test.cpp b/src/mongo/s/write_ops/batch_write_exec_test.cpp index e39bc0bc930..d563fa8a11f 100644 --- a/src/mongo/s/write_ops/batch_write_exec_test.cpp +++ b/src/mongo/s/write_ops/batch_write_exec_test.cpp @@ -30,6 +30,7 @@ #include "mongo/client/remote_command_targeter_factory_mock.h" #include "mongo/client/remote_command_targeter_mock.h" +#include "mongo/db/commands.h" #include "mongo/db/logical_session_id.h" #include "mongo/s/catalog/type_shard.h" #include "mongo/s/client/shard_registry.h" @@ -159,23 +160,30 @@ public: void expectInsertsReturnError(const std::vector<BSONObj>& expected, const BatchedCommandResponse& errResponse) { onCommandForPoolExecutor([&](const executor::RemoteCommandRequest& request) { - ASSERT_EQUALS(nss.db(), request.dbname); + try { + ASSERT_EQUALS(nss.db(), request.dbname); - const auto opMsgRequest(OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj)); - const auto actualBatchedInsert(BatchedCommandRequest::parseInsert(opMsgRequest)); - ASSERT_EQUALS(nss.toString(), actualBatchedInsert.getNS().ns()); + const auto opMsgRequest( + OpMsgRequest::fromDBAndBody(request.dbname, request.cmdObj)); + const auto actualBatchedInsert(BatchedCommandRequest::parseInsert(opMsgRequest)); + ASSERT_EQUALS(nss.toString(), actualBatchedInsert.getNS().ns()); - const auto& inserted = actualBatchedInsert.getInsertRequest().getDocuments(); - ASSERT_EQUALS(expected.size(), inserted.size()); + const auto& inserted = actualBatchedInsert.getInsertRequest().getDocuments(); + ASSERT_EQUALS(expected.size(), inserted.size()); - auto itInserted = inserted.begin(); - auto itExpected = expected.begin(); + auto itInserted = inserted.begin(); + auto itExpected = expected.begin(); - for (; itInserted != inserted.end(); itInserted++, itExpected++) { - ASSERT_BSONOBJ_EQ(*itExpected, *itInserted); - } + for (; itInserted != inserted.end(); itInserted++, itExpected++) { + ASSERT_BSONOBJ_EQ(*itExpected, *itInserted); + } - return errResponse.toBSON(); + return errResponse.toBSON(); + } catch (const DBException& ex) { + BSONObjBuilder bb; + CommandHelpers::appendCommandStatusNoThrow(bb, ex.toStatus()); + return bb.obj(); + } }); } diff --git a/src/mongo/s/write_ops/batched_command_response_test.cpp b/src/mongo/s/write_ops/batched_command_response_test.cpp index f4195cff20d..8f37da45cb2 100644 --- a/src/mongo/s/write_ops/batched_command_response_test.cpp +++ b/src/mongo/s/write_ops/batched_command_response_test.cpp @@ -60,19 +60,12 @@ TEST(BatchedCommandResponse, Basic) { << "errInfo" << BSON("a" << 1))); - BSONObj origResponseObj = BSON("ok" << 0.0 << "errmsg" - << "this batch didn't work" - << "code" - << ErrorCodes::BadValue - << "codeName" - << "BadValue" - << BatchedCommandResponse::n(0) - << "opTime" - << mongo::Timestamp(1ULL) - << BatchedCommandResponse::writeErrors() - << writeErrorsArray - << BatchedCommandResponse::writeConcernError() - << writeConcernError); + BSONObj origResponseObj = + BSON("ok" << 1.0 << BatchedCommandResponse::n(0) << "opTime" << mongo::Timestamp(1ULL) + << BatchedCommandResponse::writeErrors() + << writeErrorsArray + << BatchedCommandResponse::writeConcernError() + << writeConcernError); string errMsg; BatchedCommandResponse response; |