Re: [PR] chore: better error handling of nlohmann json lib apis [iceberg-cpp]

[via GitHub]

wgtmac commented on code in PR #95:
URL: https://github.com/apache/iceberg-cpp/pull/95#discussion_r2083558800


##########
src/iceberg/json_internal.cc:
##########
@@ -1210,11 +1216,11 @@ Result<std::unique_ptr<TableMetadata>> 
TableMetadataFromJson(const nlohmann::jso
 }
 
 Result<nlohmann::json> FromJsonString(const std::string& json_string) {
-  try {
-    return nlohmann::json::parse(json_string);
-  } catch (const std::exception& e) {
-    return JsonParseError("Failed to parse JSON string: {}", e.what());
+  auto json = nlohmann::json::parse(json_string, nullptr, false);
+  if (json.is_discarded()) {

Review Comment:
   ```suggestion
     if (json.is_discarded()) [[unlikely]] {
   ```



##########
src/iceberg/json_internal.cc:
##########
@@ -1210,11 +1216,11 @@ Result<std::unique_ptr<TableMetadata>> 
TableMetadataFromJson(const nlohmann::jso
 }
 
 Result<nlohmann::json> FromJsonString(const std::string& json_string) {
-  try {
-    return nlohmann::json::parse(json_string);
-  } catch (const std::exception& e) {
-    return JsonParseError("Failed to parse JSON string: {}", e.what());
+  auto json = nlohmann::json::parse(json_string, nullptr, false);

Review Comment:
   ```suggestion
     auto json = nlohmann::json::parse(json_string, /*cb=*/nullptr, 
/*allow_exceptions=*/false);
   ```



##########
src/iceberg/json_internal.cc:
##########
@@ -173,12 +171,17 @@ void SetOptionalField(nlohmann::json& json, 
std::string_view key,
   }
 }
 
+std::string DumpJsonNoExcept(const nlohmann::json& json) {
+  return json.dump(-1, ' ', false, nlohmann::detail::error_handler_t::ignore);

Review Comment:
   What about renaming it to `SafeDumpJson`?



##########
src/iceberg/json_internal.cc:
##########
@@ -173,12 +171,17 @@ void SetOptionalField(nlohmann::json& json, 
std::string_view key,
   }
 }
 
+std::string DumpJsonNoExcept(const nlohmann::json& json) {
+  return json.dump(-1, ' ', false, nlohmann::detail::error_handler_t::ignore);

Review Comment:
   ```suggestion
     return json.dump(/*indent=*/-1, /*indent_char=*/' ', 
/*ensure_ascii=*/false, nlohmann::detail::error_handler_t::ignore);
   ```
   
   For better readability



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org