Skip to content

Commit 74b79e6

Browse files
drkannandrkannan
committed
Resolves FoundationDB#120: Cleaned code to use generic function for reporting error messages
1 parent f6e30d2 commit 74b79e6

File tree

4 files changed

+32
-50
lines changed

4 files changed

+32
-50
lines changed

‎src/ExtCmd.actor.cpp

Lines changed: 17 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doDropDatabase(Reference<ExtConnecti
124124
return reply;
125125
} catch (Error& e) {
126126
// clang-format off
127-
reply->addDocument(BSON("ok" << 1.0 <<
128-
"err" << e.what() <<
129-
"code" << e.code()));
127+
reply->setError(e);
130128
// clang-format on
131129
return reply;
132130
}
@@ -187,16 +185,17 @@ struct GetlasterrorCmd {
187185
try {
188186
WriteResult res = wait(lastWrite);
189187
bob.appendNumber("n", (long long)res.n); //< FIXME: ???
190-
bob << "err" << BSONNULL << "ok" << 1.0;
188+
bob << "$err" << BSONNULL << "ok" << 1.0;
191189
if (!res.upsertedOIDList.empty()) {
192190
bob.appendElements(DataValue::decode_key_part(res.upsertedOIDList[0]).wrap("upserted"));
193191
}
194192
if (res.type == WriteType::UPDATE) {
195193
bob << "updatedExisting" << (res.nModified > 0 && res.upsertedOIDList.empty());
196194
}
197195
} catch (Error& e) {
198-
bob.append("err", e.what());
196+
bob.append("$err", e.name());
199197
bob.append("code", e.code());
198+
bob.append("errmsg", e.what());
200199
bob.appendNumber("n", (long long)0);
201200
bob.append("ok", 1.0);
202201
}
@@ -226,13 +225,10 @@ struct GetLogCmd {
226225
static Future<Reference<ExtMsgReply>> call(Reference<ExtConnection> nmc,
227226
Reference<ExtMsgQuery> query,
228227
Reference<ExtMsgReply> reply) {
229-
bson::BSONObjBuilder bob;
230228

231229
if (query->ns.first != "admin") {
232230
// clang-format off
233-
reply->addDocument((bob << "ok" << 0.0 <<
234-
"errmsg" << "access denied; use admin db" <<
235-
"$err" << "access denied; use admin db").obj());
231+
reply->setError(access_denied_use_admin_db());
236232
// clang-format on
237233
return reply;
238234
}
@@ -283,15 +279,9 @@ struct ReplSetGetStatusCmd {
283279
static Future<Reference<ExtMsgReply>> call(Reference<ExtConnection> nmc,
284280
Reference<ExtMsgQuery> query,
285281
Reference<ExtMsgReply> reply) {
286-
bson::BSONObjBuilder bob;
287282

288283
// FIXME: what do we really want to report here?
289-
bob.append("ok", 0.0);
290-
bob.append("errmsg", "not really talking to mongodb");
291-
bob.append("$err", "not really talking to mongodb");
292-
293-
reply->addDocument(bob.obj());
294-
284+
reply->setError(not_really_talking_to_mongodb());
295285
return reply;
296286
}
297287
};
@@ -391,7 +381,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doDropCollection(Reference<ExtConnec
391381
reply->addDocument(BSON("ok" << 1.0));
392382
return reply;
393383
} catch (Error& e) {
394-
reply->addDocument(BSON("ok" << 1.0 << "err" << e.what() << "code" << e.code()));
384+
reply->setError(e);
395385
return reply;
396386
}
397387
}
@@ -483,7 +473,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doRenameCollection(Reference<ExtConn
483473
reply->addDocument(BSON("ok" << 1.0));
484474
return reply;
485475
} catch (Error& e) {
486-
reply->addDocument(BSON("ok" << 0.0 << "errmsg" << e.what() << "code" << e.code()));
476+
reply->setError(e);
487477
return reply;
488478
}
489479
}
@@ -518,7 +508,7 @@ ACTOR static Future<Reference<ExtMsgReply>> getStreamCount(Reference<ExtConnecti
518508
reply->addDocument(BSON("n" << (double)std::max<int64_t>(count - skip, 0) << "ok" << 1.0));
519509
return reply;
520510
} catch (Error& e) {
521-
reply->addDocument(BSON("$err" << e.what() << "code" << e.code() << "ok" << 1.0));
511+
reply->setError(e);
522512
reply->setResponseFlags(2);
523513
return reply;
524514
}
@@ -632,10 +622,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doFindAndModify(Reference<ExtConnect
632622
reply->addDocument(replyObj);
633623
} catch (Error& e) {
634624
// clang-format off
635-
reply->addDocument(BSON("errmsg" << e.what() <<
636-
"$err" << e.what() <<
637-
"code" << e.code() <<
638-
"ok" << 1.0));
625+
reply->setError(e);
639626
// clang-format on
640627
}
641628

@@ -702,9 +689,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doDropIndexesActor(Reference<ExtConn
702689
return reply;
703690
} else {
704691
// clang-format off
705-
reply->addDocument(BSON("ok" << 0.0 <<
706-
"$err" << "'index' must be a string or an object" <<
707-
"errmsg" << "'index' must be a string or an object"));
692+
reply->setError(index_must_be_a_string_or_an_object());
708693
// clang-format on
709694
return reply;
710695
}
@@ -724,9 +709,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doDropIndexesActor(Reference<ExtConn
724709
}
725710
} catch (Error& e) {
726711
// clang-format off
727-
reply->addDocument(BSON("ok" << 0.0 <<
728-
"err" << e.what() <<
729-
"code" << e.code()));
712+
reply->setError(e);
730713
// clang-format on
731714
return reply;
732715
}
@@ -767,10 +750,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doCreateIndexes(Reference<ExtConnect
767750
reply->addDocument(BSON("ok" << 1.0));
768751
} catch (Error& e) {
769752
// clang-format off
770-
reply->addDocument(BSON("ok" << 0.0 <<
771-
"$err" << e.what() <<
772-
"errmsg" << e.what() <<
773-
"code" << e.code()));
753+
reply->setError(e);
774754
// clang-format on
775755
}
776756
return reply;
@@ -917,7 +897,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doCreateCollection(Reference<ExtConn
917897
reply->addDocument(BSON("ok" << 1.0));
918898
return reply;
919899
} catch (Error& e) {
920-
reply->addDocument(BSON("ok" << 1.0 << "err" << e.what() << "code" << e.code()));
900+
reply->setError(e);
921901
return reply;
922902
}
923903
}
@@ -945,11 +925,7 @@ ACTOR static Future<Reference<ExtMsgReply>> doGetKVStatusActor(Reference<ExtConn
945925
<< vv.encode_value().toString()) /*bson::fromjson(vv.encode_value().c_str())*/);
946926
}
947927
} catch (Error& e) {
948-
reply->addDocument(
949-
BSON("ok" << 1.0 << "err"
950-
<< "This command is supported only with version 3.0 and above of the KV Store, if you are using "
951-
"an older FDB version please use the fdbcli utility to check its status."
952-
<< "code" << e.code()));
928+
reply->setError(unsupported_fdb_version_for_kvstatus());
953929
}
954930

955931
return reply;
@@ -1067,8 +1043,8 @@ ACTOR static Future<Reference<ExtMsgReply>> insertAndReply(Reference<ExtConnecti
10671043
for (int i = 0; i < docs.size(); i++) {
10681044
// clang-format off
10691045
arrayBuilder << BSON("index" << i <<
1046+
"$err" << e.name() <<
10701047
"code" << e.code() <<
1071-
"$err" << e.what() <<
10721048
"errmsg" << e.what());
10731049
// clang-format on
10741050
}
@@ -1379,7 +1355,7 @@ ACTOR static Future<Reference<ExtMsgReply>> getStreamDistinct(Reference<ExtConne
13791355
// clang-format on
13801356
return reply;
13811357
} catch (Error& e) {
1382-
reply->addDocument(BSON("$err" << e.what() << "code" << e.code() << "ok" << 1.0));
1358+
reply->setError(e);
13831359
reply->setResponseFlags(2 /*0b0010*/);
13841360
return reply;
13851361
}

‎src/ExtMsg.actor.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ ACTOR static Future<Void> runQuery(Reference<ExtConnection> ec,
487487
} catch (Error& e) {
488488
if (e.code() != error_code_end_of_stream) {
489489
reply = Reference<ExtMsgReply>(new ExtMsgReply(msg->header, msg->query));
490-
reply->addDocument(BSON("$err" << e.what() << "code" << e.code() << "ok" << 1.0));
490+
reply->setError(e);
491491
reply->setResponseFlags(2 /*0b0010*/);
492492
replyStream.send(reply);
493493
}
@@ -1115,8 +1115,8 @@ ACTOR Future<WriteCmdResult> doUpdateCmd(Namespace ns,
11151115
TraceEvent(SevError, "ExtMsgUpdateFailure").error(e);
11161116
// clang-format off
11171117
cmdResult.writeErrors.push_back(BSON("index" << idx <<
1118+
"$err" << e.name() <<
11181119
"code" << e.code() <<
1119-
"$err" << e.what() <<
11201120
"errmsg" << e.what()));
11211121
// clang-format on
11221122
if (ordered)
@@ -1182,6 +1182,7 @@ ACTOR static Future<Void> doGetMoreRun(Reference<ExtMsgGetMore> getMore, Referen
11821182
cursor->refresh();
11831183
} catch (Error& e) {
11841184
reply->setError(e);
1185+
reply->setResponseFlags(2 /*0b0010*/);
11851186
}
11861187
} else {
11871188
reply->addResponseFlag(1 /*0b0001*/);
@@ -1269,8 +1270,8 @@ ACTOR Future<WriteCmdResult> doDeleteCmd(Namespace ns,
12691270
TraceEvent(SevError, "ExtMsgDeleteFailure").error(e);
12701271
// clang-format off
12711272
writeErrors.push_back(BSON("index" << idx <<
1273+
"$err" << e.name() <<
12721274
"code" << e.code() <<
1273-
"$err" << e.what() <<
12741275
"errmsg" << e.what()));
12751276
// clang-format on
12761277
if (ordered)

‎src/ExtMsg.actor.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,22 +113,20 @@ struct ExtMsgReply : ExtMsg, FastAllocated<ExtMsgReply> {
113113
replyHeader.documentCount++;
114114
}
115115

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

121-
// Set query failure flag
122-
addResponseFlag(2);
123-
124121
// clang-format off
125122
addDocument(BSON("ok" << 0 <<
126123
"$err" << msg <<
127-
"code" << code));
124+
"code" << code <<
125+
"errmsg" << errmsg));
128126
// clang-format on
129127
}
130128

131-
void setError(Error e) { setError(e.what(), e.code()); }
129+
void setError(Error e) { setError(e.name(), e.code(), e.what()); }
132130

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

‎src/error_definitions.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,13 @@ DOCLAYER_ERROR(wire_protocol_mismatch, 29966, "Wire protocol mismatch. Bad messa
8585
DOCLAYER_ERROR(no_index_name, 29967, "No index name specified");
8686
DOCLAYER_ERROR(unsupported_index_type, 29969, "Document Layer does not support this index type, yet.");
8787

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

0 commit comments

Comments
 (0)