Clear key after matching on it

src/commit_request.cpp

@@ -116,7 +116,7 @@ void CommitRequest::on_begin_object(void *userdata) {
     return;
 
   if (ctx.state_stack.empty()) {
-    ctx.parse_error = true;
+    ctx.parse_error = "Unexpected object start - empty state stack";
     return;
   }
 
@@ -135,7 +135,7 @@ void CommitRequest::on_begin_object(void *userdata) {
     ctx.current_operation = OperationParseState{};
     break;
   default:
-    ctx.parse_error = true;
+    ctx.parse_error = "Unexpected object in invalid parse state";
     break;
   }
 }
@@ -145,7 +145,9 @@ void CommitRequest::on_end_object(void *userdata) {
   auto &ctx = self->parser_context_;
 
   if (ctx.parse_error || ctx.state_stack.empty()) {
-    ctx.parse_error = true;
+    if (!ctx.parse_error) {
+      ctx.parse_error = "Unexpected object end - empty state stack";
+    }
     return;
   }
 
@@ -162,7 +164,8 @@ void CommitRequest::on_end_object(void *userdata) {
     switch (ctx.current_precondition.type) {
     case Precondition::Type::PointRead:
       if (!ctx.current_precondition.key.has_value()) {
-        ctx.parse_error = true;
+        ctx.parse_error =
+            "point_read precondition missing required 'key' field";
       } else {
         self->preconditions_.push_back(
             Precondition{ctx.current_precondition.type,
@@ -174,7 +177,8 @@ void CommitRequest::on_end_object(void *userdata) {
     case Precondition::Type::RangeRead:
       if (!ctx.current_precondition.begin.has_value() ||
           !ctx.current_precondition.end.has_value()) {
-        ctx.parse_error = true;
+        ctx.parse_error = "range_read precondition missing required 'begin' "
+                          "and/or 'end' fields";
       } else {
         self->preconditions_.push_back(
             Precondition{ctx.current_precondition.type,
@@ -190,7 +194,8 @@ void CommitRequest::on_end_object(void *userdata) {
     case Operation::Type::Write:
       if (!ctx.current_operation.key.has_value() ||
           !ctx.current_operation.value.has_value()) {
-        ctx.parse_error = true;
+        ctx.parse_error =
+            "write operation missing required 'key' and/or 'value' fields";
       } else {
         self->operations_.push_back(Operation{
             ctx.current_operation.type, ctx.current_operation.key.value(),
@@ -199,7 +204,7 @@ void CommitRequest::on_end_object(void *userdata) {
       break;
     case Operation::Type::Delete:
       if (!ctx.current_operation.key.has_value()) {
-        ctx.parse_error = true;
+        ctx.parse_error = "delete operation missing required 'key' field";
       } else {
         self->operations_.push_back(Operation{
             ctx.current_operation.type, ctx.current_operation.key.value(), {}});
@@ -208,7 +213,8 @@ void CommitRequest::on_end_object(void *userdata) {
     case Operation::Type::RangeDelete:
       if (!ctx.current_operation.begin.has_value() ||
           !ctx.current_operation.end.has_value()) {
-        ctx.parse_error = true;
+        ctx.parse_error = "range_delete operation missing required 'begin' "
+                          "and/or 'end' fields";
       } else {
         self->operations_.push_back(Operation{
             ctx.current_operation.type, ctx.current_operation.begin.value(),
@@ -235,7 +241,6 @@ void CommitRequest::on_string_data(void *userdata, const char *buf, int len,
   if (done) {
     self->handle_completed_string();
     ctx.current_string.clear();
-    ctx.current_key.clear(); // Clear key after processing value
   }
 }
 
@@ -263,11 +268,14 @@ void CommitRequest::on_begin_array(void *userdata) {
     return;
 
   if (ctx.current_key == "preconditions") {
+    ctx.current_key.clear();
     ctx.state_stack.push(ParseState::PreconditionsArray);
   } else if (ctx.current_key == "operations") {
+    ctx.current_key.clear();
     ctx.state_stack.push(ParseState::OperationsArray);
   } else {
-    ctx.parse_error = true;
+    ctx.parse_error = "Invalid array field - only 'preconditions' and "
+                      "'operations' arrays are allowed";
   }
 }
 
@@ -276,7 +284,9 @@ void CommitRequest::on_end_array(void *userdata) {
   auto &ctx = self->parser_context_;
 
   if (ctx.parse_error || ctx.state_stack.empty()) {
-    ctx.parse_error = true;
+    if (!ctx.parse_error) {
+      ctx.parse_error = "Unexpected array end - empty state stack";
+    }
     return;
   }
 
@@ -296,19 +306,18 @@ void CommitRequest::on_number_data(void *userdata, const char *buf, int len,
   if (done) {
     self->handle_completed_number();
     ctx.current_number.clear();
-    ctx.current_key.clear(); // Clear key after processing value
   }
 }
 
-void CommitRequest::on_true_literal(void *userdata) {
+void CommitRequest::on_true_literal(void *) {
   // Not used in this API
 }
 
-void CommitRequest::on_false_literal(void *userdata) {
+void CommitRequest::on_false_literal(void *) {
   // Not used in this API
 }
 
-void CommitRequest::on_null_literal(void *userdata) {
+void CommitRequest::on_null_literal(void *) {
   // Not used in this API
 }
 
@@ -316,7 +325,7 @@ void CommitRequest::handle_completed_string() {
   auto &ctx = parser_context_;
 
   if (ctx.state_stack.empty()) {
-    ctx.parse_error = true;
+    ctx.parse_error = "String value received with empty state stack";
     return;
   }
 
@@ -325,33 +334,42 @@ void CommitRequest::handle_completed_string() {
   switch (current_state) {
   case ParseState::Root:
     if (ctx.current_key == "request_id") {
+      ctx.current_key.clear();
       request_id_ = store_string(ctx.current_string);
     } else if (ctx.current_key == "leader_id") {
+      ctx.current_key.clear();
       leader_id_ = store_string(ctx.current_string);
     } else if (ctx.current_key == "read_version") {
+      ctx.current_key.clear();
       // read_version should be a number, not a string
-      ctx.parse_error = true;
+      ctx.parse_error = "read_version field must be a number, not a string";
     }
     break;
   case ParseState::PreconditionObject:
     if (ctx.current_key == "type") {
+      ctx.current_key.clear();
       if (ctx.current_string == "point_read") {
         ctx.current_precondition.type = Precondition::Type::PointRead;
       } else if (ctx.current_string == "range_read") {
         ctx.current_precondition.type = Precondition::Type::RangeRead;
       } else {
-        ctx.parse_error = true;
+        ctx.parse_error =
+            "Invalid precondition type - must be 'point_read' or 'range_read'";
       }
     } else if (ctx.current_key == "key") {
+      ctx.current_key.clear();
       ctx.current_precondition.key = decode_base64(ctx.current_string);
     } else if (ctx.current_key == "begin") {
+      ctx.current_key.clear();
       ctx.current_precondition.begin = decode_base64(ctx.current_string);
     } else if (ctx.current_key == "end") {
+      ctx.current_key.clear();
       ctx.current_precondition.end = decode_base64(ctx.current_string);
     }
     break;
   case ParseState::OperationObject:
     if (ctx.current_key == "type") {
+      ctx.current_key.clear();
       if (ctx.current_string == "write") {
         ctx.current_operation.type = Operation::Type::Write;
       } else if (ctx.current_string == "delete") {
@@ -359,15 +377,20 @@ void CommitRequest::handle_completed_string() {
       } else if (ctx.current_string == "range_delete") {
         ctx.current_operation.type = Operation::Type::RangeDelete;
       } else {
-        ctx.parse_error = true;
+        ctx.parse_error = "Invalid operation type - must be 'write', 'delete', "
+                          "or 'range_delete'";
       }
     } else if (ctx.current_key == "key") {
+      ctx.current_key.clear();
       ctx.current_operation.key = decode_base64(ctx.current_string);
     } else if (ctx.current_key == "value") {
+      ctx.current_key.clear();
       ctx.current_operation.value = decode_base64(ctx.current_string);
     } else if (ctx.current_key == "begin") {
+      ctx.current_key.clear();
       ctx.current_operation.begin = decode_base64(ctx.current_string);
     } else if (ctx.current_key == "end") {
+      ctx.current_key.clear();
       ctx.current_operation.end = decode_base64(ctx.current_string);
     }
     break;
@@ -380,7 +403,7 @@ void CommitRequest::handle_completed_number() {
   auto &ctx = parser_context_;
 
   if (ctx.state_stack.empty()) {
-    ctx.parse_error = true;
+    ctx.parse_error = "Number value received with empty state stack";
     return;
   }
 
@@ -389,6 +412,7 @@ void CommitRequest::handle_completed_number() {
   switch (current_state) {
   case ParseState::Root:
     if (ctx.current_key == "read_version") {
+      ctx.current_key.clear();
       uint64_t version;
       auto result = std::from_chars(
           ctx.current_number.data(),
@@ -397,12 +421,13 @@ void CommitRequest::handle_completed_number() {
         read_version_ = version;
         has_read_version_been_set_ = true;
       } else {
-        ctx.parse_error = true;
+        ctx.parse_error = "Invalid number format for read_version field";
       }
     }
     break;
   case ParseState::PreconditionObject:
     if (ctx.current_key == "version") {
+      ctx.current_key.clear();
       uint64_t version;
       auto result = std::from_chars(
           ctx.current_number.data(),
@@ -410,7 +435,8 @@ void CommitRequest::handle_completed_number() {
       if (result.ec == std::errc{}) {
         ctx.current_precondition.version = version;
       } else {
-        ctx.parse_error = true;
+        ctx.parse_error =
+            "Invalid number format for precondition version field";
       }
     }
     break;
@@ -462,7 +488,8 @@ CommitRequest::ParseStatus CommitRequest::parse_chunk(char *data, size_t len) {
   case WeaselJson_REJECT:
   case WeaselJson_OVERFLOW:
   default:
-    parser_context_.parse_error = true;
+    parser_context_.parse_error =
+        "JSON parsing failed - invalid or oversized JSON";
     return ParseStatus::Error;
   }
 }
@@ -483,7 +510,8 @@ CommitRequest::ParseStatus CommitRequest::finish_streaming_parse() {
       !parser_context_.parse_error) {
     return ParseStatus::Complete;
   } else {
-    parser_context_.parse_error = true;
+    parser_context_.parse_error =
+        "JSON parsing incomplete or failed during finalization";
     return ParseStatus::Error;
   }
 }

src/commit_request.hpp

@@ -89,7 +89,7 @@ public:
     ArenaString current_string;
     ArenaString current_number;
     bool in_key = false;
-    bool parse_error = false;
+    const char *parse_error = nullptr;
     bool parse_complete = false;
 
     // Current objects being parsed
@@ -222,7 +222,15 @@ public:
    * @brief Check if there was a parse error.
    * @return true if there was a parse error
    */
-  bool has_parse_error() const { return parser_context_.parse_error; }
+  bool has_parse_error() const {
+    return parser_context_.parse_error != nullptr;
+  }
+
+  /**
+   * @brief Get the parse error message if there was an error.
+   * @return Error message string, or nullptr if no error
+   */
+  const char *get_parse_error() const { return parser_context_.parse_error; }
 
   /**
    * @brief Get the request ID if present.

tests/test_commit_request.cpp

@@ -195,8 +195,8 @@ TEST_CASE("CommitRequest precondition and operation validation") {
                 {
                     "type": "range_read",
                     "version": 12340,
-                    "begin": "",
+                    "begin": "dGVzdA==",
-                    "end": ""
+                    "end": "dGVzdFo="
                 }
             ]
         })";
@@ -206,6 +206,9 @@ TEST_CASE("CommitRequest precondition and operation validation") {
     INFO("Parse result: " << parse_result);
     INFO("Parse complete: " << request.is_parse_complete());
     INFO("Parse error: " << request.has_parse_error());
+    const char *error_msg = request.get_parse_error();
+    INFO("Parse error message: " << (error_msg ? std::string(error_msg)
+                                               : "none"));
     INFO("Leader ID: '" << request.leader_id() << "'");
     INFO("Read version: " << request.read_version());