Skip to content

chore: better error handling of nlohmann json lib apis #95

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

Merged
merged 2 commits into from
May 12, 2025
Merged

chore: better error handling of nlohmann json lib apis #95

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46d386f
chore: better error handling of nlohmann json lib apis
zhjwpku May 11, 2025
0611f83
resolve review comments
zhjwpku May 12, 2025
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
47 changes: 27 additions & 20 deletions src/iceberg/json_internal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <algorithm>
#include <cstdint>
#include <format>
#include <ranges>
#include <regex>
#include <type_traits>
#include <unordered_set>
Expand All @@ -37,7 +36,6 @@
#include "iceberg/snapshot.h"
#include "iceberg/sort_order.h"
#include "iceberg/statistics_file.h"
#include "iceberg/table.h"
#include "iceberg/table_metadata.h"
#include "iceberg/transform.h"
#include "iceberg/type.h"
Expand Down Expand Up @@ -173,12 +171,18 @@ void SetOptionalField(nlohmann::json& json, std::string_view key,
}
}

std::string SafeDumpJson(const nlohmann::json& json) {
return json.dump(/*indent=*/-1, /*indent_char=*/' ', /*ensure_ascii=*/false,
nlohmann::detail::error_handler_t::ignore);
}

template <typename T>
Result<T> GetJsonValueImpl(const nlohmann::json& json, std::string_view key) {
try {
return json.at(key).get<T>();
} catch (const std::exception& ex) {
return JsonParseError("Failed to parse key '{}' in {}", key, json.dump());
return JsonParseError("Failed to parse '{}' from {}: {}", key, SafeDumpJson(json),
ex.what());
}
}

Expand All @@ -194,7 +198,7 @@ Result<std::optional<T>> GetJsonValueOptional(const nlohmann::json& json,
template <typename T>
Result<T> GetJsonValue(const nlohmann::json& json, std::string_view key) {
if (!json.contains(key)) {
return JsonParseError("Missing '{}' in {}", key, json.dump());
return JsonParseError("Missing '{}' in {}", key, SafeDumpJson(json));
}
return GetJsonValueImpl<T>(json, key);
}
Expand Down Expand Up @@ -245,7 +249,7 @@ Result<std::vector<T>> FromJsonList(
ICEBERG_ASSIGN_OR_RAISE(auto list_json, GetJsonValue<nlohmann::json>(json, key));
if (!list_json.is_array()) {
return JsonParseError("Cannot parse '{}' from non-array: {}", key,
list_json.dump());
SafeDumpJson(list_json));
}
for (const auto& entry_json : list_json) {
ICEBERG_ASSIGN_OR_RAISE(auto entry, from_json(entry_json));
Expand All @@ -270,7 +274,7 @@ Result<std::vector<std::shared_ptr<T>>> FromJsonList(
ICEBERG_ASSIGN_OR_RAISE(auto list_json, GetJsonValue<nlohmann::json>(json, key));
if (!list_json.is_array()) {
return JsonParseError("Cannot parse '{}' from non-array: {}", key,
list_json.dump());
SafeDumpJson(list_json));
}
for (const auto& entry_json : list_json) {
ICEBERG_ASSIGN_OR_RAISE(auto entry, from_json(entry_json));
Expand Down Expand Up @@ -319,7 +323,7 @@ Result<std::unordered_map<std::string, T>> FromJsonMap(
try {
return json.get<std::string>();
} catch (const std::exception& ex) {
return JsonParseError("Cannot parse {} to a string value: {}", json.dump(),
return JsonParseError("Cannot parse {} to a string value: {}", SafeDumpJson(json),
ex.what());
}
}) {
Expand All @@ -328,7 +332,7 @@ Result<std::unordered_map<std::string, T>> FromJsonMap(
ICEBERG_ASSIGN_OR_RAISE(auto map_json, GetJsonValue<nlohmann::json>(json, key));
if (!map_json.is_object()) {
return JsonParseError("Cannot parse '{}' from non-object: {}", key,
map_json.dump());
SafeDumpJson(map_json));
}
for (const auto& [key, value] : map_json.items()) {
ICEBERG_ASSIGN_OR_RAISE(auto entry, from_json(value));
Expand Down Expand Up @@ -633,7 +637,7 @@ Result<std::unique_ptr<Schema>> SchemaFromJson(const nlohmann::json& json) {
ICEBERG_ASSIGN_OR_RAISE(auto type, TypeFromJson(json));

if (type->type_id() != TypeId::kStruct) [[unlikely]] {
return JsonParseError("Schema must be a struct type, but got {}", json.dump());
return JsonParseError("Schema must be a struct type, but got {}", SafeDumpJson(json));
}

auto& struct_type = static_cast<StructType&>(*type);
Expand Down Expand Up @@ -741,11 +745,12 @@ Result<std::unique_ptr<Snapshot>> SnapshotFromJson(const nlohmann::json& json) {
return JsonParseError("Invalid snapshot summary field: {}", key);
}
if (!value.is_string()) {
return JsonParseError("Invalid snapshot summary field value: {}", value.dump());
return JsonParseError("Invalid snapshot summary field value: {}",
SafeDumpJson(value));
}
if (key == SnapshotSummaryFields::kOperation &&
!kValidDataOperation.contains(value.get<std::string>())) {
return JsonParseError("Invalid snapshot operation: {}", value.dump());
return JsonParseError("Invalid snapshot operation: {}", SafeDumpJson(value));
}
summary[key] = value.get<std::string>();
}
Expand Down Expand Up @@ -975,7 +980,7 @@ Result<std::shared_ptr<Schema>> ParseSchemas(
GetJsonValue<nlohmann::json>(json, kSchemas));
if (!schema_array.is_array()) {
return JsonParseError("Cannot parse schemas from non-array: {}",
schema_array.dump());
SafeDumpJson(schema_array));
}

ICEBERG_ASSIGN_OR_RAISE(current_schema_id,
Expand All @@ -990,7 +995,7 @@ Result<std::shared_ptr<Schema>> ParseSchemas(
}
if (!current_schema) {
return JsonParseError("Cannot find schema with {}={} from {}", kCurrentSchemaId,
current_schema_id.value(), schema_array.dump());
current_schema_id.value(), SafeDumpJson(schema_array));
}
} else {
if (format_version != 1) {
Expand Down Expand Up @@ -1021,7 +1026,7 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version,
GetJsonValue<nlohmann::json>(json, kPartitionSpecs));
if (!spec_array.is_array()) {
return JsonParseError("Cannot parse partition specs from non-array: {}",
spec_array.dump());
SafeDumpJson(spec_array));
}
ICEBERG_ASSIGN_OR_RAISE(default_spec_id, GetJsonValue<int32_t>(json, kDefaultSpecId));

Expand All @@ -1040,7 +1045,7 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version,
GetJsonValue<nlohmann::json>(json, kPartitionSpec));
if (!partition_spec_json.is_array()) {
return JsonParseError("Cannot parse v1 partition spec from non-array: {}",
partition_spec_json.dump());
SafeDumpJson(partition_spec_json));
}

int32_t next_partition_field_id = PartitionSpec::kLegacyPartitionDataIdStart;
Expand Down Expand Up @@ -1098,7 +1103,8 @@ Status ParseSortOrders(const nlohmann::json& json, int8_t format_version,

Result<std::unique_ptr<TableMetadata>> TableMetadataFromJson(const nlohmann::json& json) {
if (!json.is_object()) {
return JsonParseError("Cannot parse metadata from a non-object: {}", json.dump());
return JsonParseError("Cannot parse metadata from a non-object: {}",
SafeDumpJson(json));
}

auto table_metadata = std::make_unique<TableMetadata>();
Expand Down Expand Up @@ -1210,11 +1216,12 @@ 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, /*cb=*/nullptr, /*allow_exceptions=*/false);
if (json.is_discarded()) [[unlikely]] {
return JsonParseError("Failed to parse JSON string: {}", json_string);
}
return json;
}

Result<std::string> ToJsonString(const nlohmann::json& json) {
Expand Down
Loading