Skip to content

Resolves #120: Cleaned code to use generic function for reporting error messages #207

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Resolves #120: Cleaned code to use generic function for reporting error messages #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 17 additions & 41 deletions src/ExtCmd.actor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doDropDatabase(Reference<ExtConnecti
return reply;
} catch (Error& e) {
// clang-format off
reply->addDocument(BSON("ok" << 1.0 <<
"err" << e.what() <<
"code" << e.code()));
reply->setError(e);
// clang-format on
return reply;
}
Expand Down Expand Up @@ -187,16 +185,17 @@ struct GetlasterrorCmd {
try {
WriteResult res = wait(lastWrite);
bob.appendNumber("n", (long long)res.n); //< FIXME: ???
bob << "err" << BSONNULL << "ok" << 1.0;
bob << "$err" << BSONNULL << "ok" << 1.0;
if (!res.upsertedOIDList.empty()) {
bob.appendElements(DataValue::decode_key_part(res.upsertedOIDList[0]).wrap("upserted"));
}
if (res.type == WriteType::UPDATE) {
bob << "updatedExisting" << (res.nModified > 0 && res.upsertedOIDList.empty());
}
} catch (Error& e) {
bob.append("err", e.what());
bob.append("$err", e.name());
bob.append("code", e.code());
bob.append("errmsg", e.what());
bob.appendNumber("n", (long long)0);
bob.append("ok", 1.0);
}
Expand Down Expand Up @@ -226,13 +225,10 @@ struct GetLogCmd {
static Future<Reference<ExtMsgReply>> call(Reference<ExtConnection> nmc,
Reference<ExtMsgQuery> query,
Reference<ExtMsgReply> reply) {
bson::BSONObjBuilder bob;

if (query->ns.first != "admin") {
// clang-format off
reply->addDocument((bob << "ok" << 0.0 <<
"errmsg" << "access denied; use admin db" <<
"$err" << "access denied; use admin db").obj());
reply->setError(access_denied_use_admin_db());
// clang-format on
return reply;
}
Expand Down Expand Up @@ -283,15 +279,9 @@ struct ReplSetGetStatusCmd {
static Future<Reference<ExtMsgReply>> call(Reference<ExtConnection> nmc,
Reference<ExtMsgQuery> query,
Reference<ExtMsgReply> reply) {
bson::BSONObjBuilder bob;

// FIXME: what do we really want to report here?
bob.append("ok", 0.0);
bob.append("errmsg", "not really talking to mongodb");
bob.append("$err", "not really talking to mongodb");

reply->addDocument(bob.obj());

reply->setError(not_really_talking_to_mongodb());
return reply;
}
};
Expand Down Expand Up @@ -391,7 +381,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doDropCollection(Reference<ExtConnec
reply->addDocument(BSON("ok" << 1.0));
return reply;
} catch (Error& e) {
reply->addDocument(BSON("ok" << 1.0 << "err" << e.what() << "code" << e.code()));
reply->setError(e);
return reply;
}
}
Expand Down Expand Up @@ -483,7 +473,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doRenameCollection(Reference<ExtConn
reply->addDocument(BSON("ok" << 1.0));
return reply;
} catch (Error& e) {
reply->addDocument(BSON("ok" << 0.0 << "errmsg" << e.what() << "code" << e.code()));
reply->setError(e);
return reply;
}
}
Expand Down Expand Up @@ -518,7 +508,7 @@ ACTOR static Future<Reference<ExtMsgReply>> getStreamCount(Reference<ExtConnecti
reply->addDocument(BSON("n" << (double)std::max<int64_t>(count - skip, 0) << "ok" << 1.0));
return reply;
} catch (Error& e) {
reply->addDocument(BSON("$err" << e.what() << "code" << e.code() << "ok" << 1.0));
reply->setError(e);
reply->setResponseFlags(2);
return reply;
}
Expand Down Expand Up @@ -632,10 +622,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doFindAndModify(Reference<ExtConnect
reply->addDocument(replyObj);
} catch (Error& e) {
// clang-format off
reply->addDocument(BSON("errmsg" << e.what() <<
"$err" << e.what() <<
"code" << e.code() <<
"ok" << 1.0));
reply->setError(e);
// clang-format on
}

Expand Down Expand Up @@ -702,9 +689,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doDropIndexesActor(Reference<ExtConn
return reply;
} else {
// clang-format off
reply->addDocument(BSON("ok" << 0.0 <<
"$err" << "'index' must be a string or an object" <<
"errmsg" << "'index' must be a string or an object"));
reply->setError(index_must_be_a_string_or_an_object());
// clang-format on
return reply;
}
Expand All @@ -724,9 +709,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doDropIndexesActor(Reference<ExtConn
}
} catch (Error& e) {
// clang-format off
reply->addDocument(BSON("ok" << 0.0 <<
"err" << e.what() <<
"code" << e.code()));
reply->setError(e);
// clang-format on
return reply;
}
Expand Down Expand Up @@ -767,10 +750,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doCreateIndexes(Reference<ExtConnect
reply->addDocument(BSON("ok" << 1.0));
} catch (Error& e) {
// clang-format off
reply->addDocument(BSON("ok" << 0.0 <<
"$err" << e.what() <<
"errmsg" << e.what() <<
"code" << e.code()));
reply->setError(e);
// clang-format on
}
return reply;
Expand Down Expand Up @@ -917,7 +897,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doCreateCollection(Reference<ExtConn
reply->addDocument(BSON("ok" << 1.0));
return reply;
} catch (Error& e) {
reply->addDocument(BSON("ok" << 1.0 << "err" << e.what() << "code" << e.code()));
reply->setError(e);
return reply;
}
}
Expand Down Expand Up @@ -945,11 +925,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doGetKVStatusActor(Reference<ExtConn
<< vv.encode_value().toString()) /*bson::fromjson(vv.encode_value().c_str())*/);
}
} catch (Error& e) {
reply->addDocument(
BSON("ok" << 1.0 << "err"
<< "This command is supported only with version 3.0 and above of the KV Store, if you are using "
"an older FDB version please use the fdbcli utility to check its status."
<< "code" << e.code()));
reply->setError(unsupported_fdb_version_for_kvstatus());
}

return reply;
Expand Down Expand Up @@ -1067,8 +1043,8 @@ ACTOR static Future<Reference<ExtMsgReply>> insertAndReply(Reference<ExtConnecti
for (int i = 0; i < docs.size(); i++) {
// clang-format off
arrayBuilder << BSON("index" << i <<
"$err" << e.name() <<
"code" << e.code() <<
"$err" << e.what() <<
"errmsg" << e.what());
// clang-format on
}
Expand Down Expand Up @@ -1379,7 +1355,7 @@ ACTOR static Future<Reference<ExtMsgReply>> getStreamDistinct(Reference<ExtConne
// clang-format on
return reply;
} catch (Error& e) {
reply->addDocument(BSON("$err" << e.what() << "code" << e.code() << "ok" << 1.0));
reply->setError(e);
reply->setResponseFlags(2 /*0b0010*/);
return reply;
}
Expand Down
7 changes: 4 additions & 3 deletions src/ExtMsg.actor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ ACTOR static Future<Void> runQuery(Reference<ExtConnection> ec,
} catch (Error& e) {
if (e.code() != error_code_end_of_stream) {
reply = Reference<ExtMsgReply>(new ExtMsgReply(msg->header, msg->query));
reply->addDocument(BSON("$err" << e.what() << "code" << e.code() << "ok" << 1.0));
reply->setError(e);
reply->setResponseFlags(2 /*0b0010*/);
replyStream.send(reply);
}
Expand Down Expand Up @@ -1115,8 +1115,8 @@ ACTOR Future<WriteCmdResult> doUpdateCmd(Namespace ns,
TraceEvent(SevError, "ExtMsgUpdateFailure").error(e);
// clang-format off
cmdResult.writeErrors.push_back(BSON("index" << idx <<
"$err" << e.name() <<
"code" << e.code() <<
"$err" << e.what() <<
"errmsg" << e.what()));
// clang-format on
if (ordered)
Expand Down Expand Up @@ -1182,6 +1182,7 @@ ACTOR static Future<Void> doGetMoreRun(Reference<ExtMsgGetMore> getMore, Referen
cursor->refresh();
} catch (Error& e) {
reply->setError(e);
reply->setResponseFlags(2 /*0b0010*/);
}
} else {
reply->addResponseFlag(1 /*0b0001*/);
Expand Down Expand Up @@ -1269,8 +1270,8 @@ ACTOR Future<WriteCmdResult> doDeleteCmd(Namespace ns,
TraceEvent(SevError, "ExtMsgDeleteFailure").error(e);
// clang-format off
writeErrors.push_back(BSON("index" << idx <<
"$err" << e.name() <<
"code" << e.code() <<
"$err" << e.what() <<
"errmsg" << e.what()));
// clang-format on
if (ordered)
Expand Down
10 changes: 4 additions & 6 deletions src/ExtMsg.actor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,22 +113,20 @@ struct ExtMsgReply : ExtMsg, FastAllocated<ExtMsgReply> {
replyHeader.documentCount++;
}

void setError(std::string msg, int code) {
void setError(std::string msg, int code, std::string errmsg) {
// Clear if response contains any outstanding documents
documents.clear();
replyHeader.documentCount = 0;

// Set query failure flag
addResponseFlag(2);

// clang-format off
addDocument(BSON("ok" << 0 <<
"$err" << msg <<
"code" << code));
"code" << code <<
"errmsg" << errmsg));
// clang-format on
}

void setError(Error e) { setError(e.what(), e.code()); }
void setError(Error e) { setError(e.name(), e.code(), e.what()); }

void setResponseFlags(int32_t flags) { replyHeader.responseFlags = flags; }

Expand Down
7 changes: 7 additions & 0 deletions src/error_definitions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ DOCLAYER_ERROR(wire_protocol_mismatch, 29966, "Wire protocol mismatch. Bad messa
DOCLAYER_ERROR(no_index_name, 29967, "No index name specified");
DOCLAYER_ERROR(unsupported_index_type, 29969, "Document Layer does not support this index type, yet.");

DOCLAYER_ERROR(access_denied_use_admin_db, 29972, "access denied; use admin db");
DOCLAYER_ERROR(not_really_talking_to_mongodb, 29973, "not really talking to mongodb");
DOCLAYER_ERROR(index_must_be_a_string_or_an_object, 29974, "'index' must be a string or an object");
DOCLAYER_ERROR(unsupported_fdb_version_for_kvstatus,
29975,
"This command is supported only with version 3.0 and above of the KV Store, if you are using an older "
"FDB version please use the fdbcli utility to check its status.");
DOCLAYER_ERROR(collection_name_does_not_exist, 29976, "Collection name does not exist.");
DOCLAYER_ERROR(collection_name_already_exist, 29977, "Collection name already exist.");
DOCLAYER_ERROR(old_and_new_collection_name_cannot_be_same, 29978, "Old and New collection name cannot be same.");
Expand Down