fix server: fix last error reply (#3728)

fix 1: in multi command squasher error message was not set therefore it was not printed to log on the relevant command only on exec, fixed by setting the last error in CapturingReplyBuilder::SendError fix 2: non clearing cached error replies before the command is Invoked --------- Signed-off-by: adi_holden <[email protected]> Co-authored-by: kostas <[email protected]>
dragonflydb · Sep 23, 2024 · 7df95df · 7df95df
1 parent 45ffc60
commit 7df95df
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 0 deletions.
diff --git a/src/facade/reply_capture.cc b/src/facade/reply_capture.cc
@@ -29,6 +29,9 @@ void CapturingReplyBuilder::SendMGetResponse(MGetResponse resp) {
 }
 
 void CapturingReplyBuilder::SendError(OpStatus status) {
+  if (status != OpStatus::OK) {
+    last_error_ = StatusToMsg(status);
+  }
   SKIP_LESS(ReplyMode::ONLY_ERR);
   Capture(status);
 }
@@ -215,6 +218,8 @@ void CapturingReplyBuilder::Apply(Payload&& pl, RedisReplyBuilder* rb) {
 
   CaptureVisitor cv{rb};
   visit(cv, std::move(pl));
+  // Consumed and printed by InvokeCmd. We just send the actual error here
+  std::ignore = rb->ConsumeLastError();
 }
 
 void CapturingReplyBuilder::SetReplyMode(ReplyMode mode) {

diff --git a/src/server/main_service.cc b/src/server/main_service.cc
@@ -1216,6 +1216,8 @@ std::optional<ErrorReply> Service::VerifyCommandState(const CommandId* cid, CmdA
 }
 
 void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx) {
+  absl::Cleanup clear_last_error(
+      [cntx]() { std::ignore = cntx->reply_builder()->ConsumeLastError(); });
   CHECK(!args.empty());
   DCHECK_NE(0u, shard_set->size()) << "Init was not called";
 
@@ -1419,6 +1421,8 @@ bool Service::InvokeCmd(const CommandId* cid, CmdArgList tail_args, ConnectionCo
   ReplyGuard reply_guard(cntx, cid->name());
 #endif
   uint64_t invoke_time_usec = 0;
+  auto last_error = cntx->reply_builder()->ConsumeLastError();
+  DCHECK(last_error.empty());
   try {
     invoke_time_usec = cid->Invoke(tail_args, cntx);
   } catch (std::exception& e) {

diff --git a/src/server/multi_command_squasher.cc b/src/server/multi_command_squasher.cc
@@ -121,6 +121,7 @@ bool MultiCommandSquasher::ExecuteStandalone(StoredCmd* cmd) {
   if (verify_commands_) {
     if (auto err = service_->VerifyCommandState(cmd->Cid(), args, *cntx_); err) {
       cntx_->SendError(std::move(*err));
+      std::ignore = cntx_->reply_builder()->ConsumeLastError();
       return !error_abort_;
     }
   }