Skip to content

Commit 5aed8f5

Browse files
authored
chore: better error handling of nlohmann json lib apis (#95)
1. switch off exceptions for nlohmann::json::parse 2. add a no except wrapper around json.dump, since we don't handle exceptions in Error messages. This closes issue #87 References: - https://json.nlohmann.me/features/parsing/parse_exceptions/ - https://json.nlohmann.me/api/basic_json/dump/ --------- Signed-off-by: Junwang Zhao <[email protected]>
1 parent 11667a9 commit 5aed8f5

File tree

1 file changed

+27
-20
lines changed

1 file changed

+27
-20
lines changed

src/iceberg/json_internal.cc

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include <algorithm>
2323
#include <cstdint>
2424
#include <format>
25-
#include <ranges>
2625
#include <regex>
2726
#include <type_traits>
2827
#include <unordered_set>
@@ -37,7 +36,6 @@
3736
#include "iceberg/snapshot.h"
3837
#include "iceberg/sort_order.h"
3938
#include "iceberg/statistics_file.h"
40-
#include "iceberg/table.h"
4139
#include "iceberg/table_metadata.h"
4240
#include "iceberg/transform.h"
4341
#include "iceberg/type.h"
@@ -173,12 +171,18 @@ void SetOptionalField(nlohmann::json& json, std::string_view key,
173171
}
174172
}
175173

174+
std::string SafeDumpJson(const nlohmann::json& json) {
175+
return json.dump(/*indent=*/-1, /*indent_char=*/' ', /*ensure_ascii=*/false,
176+
nlohmann::detail::error_handler_t::ignore);
177+
}
178+
176179
template <typename T>
177180
Result<T> GetJsonValueImpl(const nlohmann::json& json, std::string_view key) {
178181
try {
179182
return json.at(key).get<T>();
180183
} catch (const std::exception& ex) {
181-
return JsonParseError("Failed to parse key '{}' in {}", key, json.dump());
184+
return JsonParseError("Failed to parse '{}' from {}: {}", key, SafeDumpJson(json),
185+
ex.what());
182186
}
183187
}
184188

@@ -194,7 +198,7 @@ Result<std::optional<T>> GetJsonValueOptional(const nlohmann::json& json,
194198
template <typename T>
195199
Result<T> GetJsonValue(const nlohmann::json& json, std::string_view key) {
196200
if (!json.contains(key)) {
197-
return JsonParseError("Missing '{}' in {}", key, json.dump());
201+
return JsonParseError("Missing '{}' in {}", key, SafeDumpJson(json));
198202
}
199203
return GetJsonValueImpl<T>(json, key);
200204
}
@@ -245,7 +249,7 @@ Result<std::vector<T>> FromJsonList(
245249
ICEBERG_ASSIGN_OR_RAISE(auto list_json, GetJsonValue<nlohmann::json>(json, key));
246250
if (!list_json.is_array()) {
247251
return JsonParseError("Cannot parse '{}' from non-array: {}", key,
248-
list_json.dump());
252+
SafeDumpJson(list_json));
249253
}
250254
for (const auto& entry_json : list_json) {
251255
ICEBERG_ASSIGN_OR_RAISE(auto entry, from_json(entry_json));
@@ -270,7 +274,7 @@ Result<std::vector<std::shared_ptr<T>>> FromJsonList(
270274
ICEBERG_ASSIGN_OR_RAISE(auto list_json, GetJsonValue<nlohmann::json>(json, key));
271275
if (!list_json.is_array()) {
272276
return JsonParseError("Cannot parse '{}' from non-array: {}", key,
273-
list_json.dump());
277+
SafeDumpJson(list_json));
274278
}
275279
for (const auto& entry_json : list_json) {
276280
ICEBERG_ASSIGN_OR_RAISE(auto entry, from_json(entry_json));
@@ -319,7 +323,7 @@ Result<std::unordered_map<std::string, T>> FromJsonMap(
319323
try {
320324
return json.get<std::string>();
321325
} catch (const std::exception& ex) {
322-
return JsonParseError("Cannot parse {} to a string value: {}", json.dump(),
326+
return JsonParseError("Cannot parse {} to a string value: {}", SafeDumpJson(json),
323327
ex.what());
324328
}
325329
}) {
@@ -328,7 +332,7 @@ Result<std::unordered_map<std::string, T>> FromJsonMap(
328332
ICEBERG_ASSIGN_OR_RAISE(auto map_json, GetJsonValue<nlohmann::json>(json, key));
329333
if (!map_json.is_object()) {
330334
return JsonParseError("Cannot parse '{}' from non-object: {}", key,
331-
map_json.dump());
335+
SafeDumpJson(map_json));
332336
}
333337
for (const auto& [key, value] : map_json.items()) {
334338
ICEBERG_ASSIGN_OR_RAISE(auto entry, from_json(value));
@@ -633,7 +637,7 @@ Result<std::unique_ptr<Schema>> SchemaFromJson(const nlohmann::json& json) {
633637
ICEBERG_ASSIGN_OR_RAISE(auto type, TypeFromJson(json));
634638

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

639643
auto& struct_type = static_cast<StructType&>(*type);
@@ -741,11 +745,12 @@ Result<std::unique_ptr<Snapshot>> SnapshotFromJson(const nlohmann::json& json) {
741745
return JsonParseError("Invalid snapshot summary field: {}", key);
742746
}
743747
if (!value.is_string()) {
744-
return JsonParseError("Invalid snapshot summary field value: {}", value.dump());
748+
return JsonParseError("Invalid snapshot summary field value: {}",
749+
SafeDumpJson(value));
745750
}
746751
if (key == SnapshotSummaryFields::kOperation &&
747752
!kValidDataOperation.contains(value.get<std::string>())) {
748-
return JsonParseError("Invalid snapshot operation: {}", value.dump());
753+
return JsonParseError("Invalid snapshot operation: {}", SafeDumpJson(value));
749754
}
750755
summary[key] = value.get<std::string>();
751756
}
@@ -975,7 +980,7 @@ Result<std::shared_ptr<Schema>> ParseSchemas(
975980
GetJsonValue<nlohmann::json>(json, kSchemas));
976981
if (!schema_array.is_array()) {
977982
return JsonParseError("Cannot parse schemas from non-array: {}",
978-
schema_array.dump());
983+
SafeDumpJson(schema_array));
979984
}
980985

981986
ICEBERG_ASSIGN_OR_RAISE(current_schema_id,
@@ -990,7 +995,7 @@ Result<std::shared_ptr<Schema>> ParseSchemas(
990995
}
991996
if (!current_schema) {
992997
return JsonParseError("Cannot find schema with {}={} from {}", kCurrentSchemaId,
993-
current_schema_id.value(), schema_array.dump());
998+
current_schema_id.value(), SafeDumpJson(schema_array));
994999
}
9951000
} else {
9961001
if (format_version != 1) {
@@ -1021,7 +1026,7 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version,
10211026
GetJsonValue<nlohmann::json>(json, kPartitionSpecs));
10221027
if (!spec_array.is_array()) {
10231028
return JsonParseError("Cannot parse partition specs from non-array: {}",
1024-
spec_array.dump());
1029+
SafeDumpJson(spec_array));
10251030
}
10261031
ICEBERG_ASSIGN_OR_RAISE(default_spec_id, GetJsonValue<int32_t>(json, kDefaultSpecId));
10271032

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

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

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

11041110
auto table_metadata = std::make_unique<TableMetadata>();
@@ -1210,11 +1216,12 @@ Result<std::unique_ptr<TableMetadata>> TableMetadataFromJson(const nlohmann::jso
12101216
}
12111217

12121218
Result<nlohmann::json> FromJsonString(const std::string& json_string) {
1213-
try {
1214-
return nlohmann::json::parse(json_string);
1215-
} catch (const std::exception& e) {
1216-
return JsonParseError("Failed to parse JSON string: {}", e.what());
1219+
auto json =
1220+
nlohmann::json::parse(json_string, /*cb=*/nullptr, /*allow_exceptions=*/false);
1221+
if (json.is_discarded()) [[unlikely]] {
1222+
return JsonParseError("Failed to parse JSON string: {}", json_string);
12171223
}
1224+
return json;
12181225
}
12191226

12201227
Result<std::string> ToJsonString(const nlohmann::json& json) {

0 commit comments

Comments
 (0)