Skip to content

Commit 46d386f

Browse files
committed
chore: better error handling of nlohmann json lib apis
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 b839b3b commit 46d386f

File tree

1 file changed

+27
-21
lines changed

1 file changed

+27
-21
lines changed

src/iceberg/json_internal.cc

Lines changed: 27 additions & 21 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,17 @@ void SetOptionalField(nlohmann::json& json, std::string_view key,
173171
}
174172
}
175173

174+
std::string DumpJsonNoExcept(const nlohmann::json& json) {
175+
return json.dump(-1, ' ', false, nlohmann::detail::error_handler_t::ignore);
176+
}
177+
176178
template <typename T>
177179
Result<T> GetJsonValueImpl(const nlohmann::json& json, std::string_view key) {
178180
try {
179181
return json.at(key).get<T>();
180182
} catch (const std::exception& ex) {
181-
return JsonParseError("Failed to parse key '{}' in {}", key, json.dump());
183+
return JsonParseError("Failed to parse '{}' from {}: {}", key, DumpJsonNoExcept(json),
184+
ex.what());
182185
}
183186
}
184187

@@ -194,7 +197,7 @@ Result<std::optional<T>> GetJsonValueOptional(const nlohmann::json& json,
194197
template <typename T>
195198
Result<T> GetJsonValue(const nlohmann::json& json, std::string_view key) {
196199
if (!json.contains(key)) {
197-
return JsonParseError("Missing '{}' in {}", key, json.dump());
200+
return JsonParseError("Missing '{}' in {}", key, DumpJsonNoExcept(json));
198201
}
199202
return GetJsonValueImpl<T>(json, key);
200203
}
@@ -245,7 +248,7 @@ Result<std::vector<T>> FromJsonList(
245248
ICEBERG_ASSIGN_OR_RAISE(auto list_json, GetJsonValue<nlohmann::json>(json, key));
246249
if (!list_json.is_array()) {
247250
return JsonParseError("Cannot parse '{}' from non-array: {}", key,
248-
list_json.dump());
251+
DumpJsonNoExcept(list_json));
249252
}
250253
for (const auto& entry_json : list_json) {
251254
ICEBERG_ASSIGN_OR_RAISE(auto entry, from_json(entry_json));
@@ -270,7 +273,7 @@ Result<std::vector<std::shared_ptr<T>>> FromJsonList(
270273
ICEBERG_ASSIGN_OR_RAISE(auto list_json, GetJsonValue<nlohmann::json>(json, key));
271274
if (!list_json.is_array()) {
272275
return JsonParseError("Cannot parse '{}' from non-array: {}", key,
273-
list_json.dump());
276+
DumpJsonNoExcept(list_json));
274277
}
275278
for (const auto& entry_json : list_json) {
276279
ICEBERG_ASSIGN_OR_RAISE(auto entry, from_json(entry_json));
@@ -319,16 +322,16 @@ Result<std::unordered_map<std::string, T>> FromJsonMap(
319322
try {
320323
return json.get<std::string>();
321324
} catch (const std::exception& ex) {
322-
return JsonParseError("Cannot parse {} to a string value: {}", json.dump(),
323-
ex.what());
325+
return JsonParseError("Cannot parse {} to a string value: {}",
326+
DumpJsonNoExcept(json), ex.what());
324327
}
325328
}) {
326329
std::unordered_map<std::string, T> map{};
327330
if (json.contains(key)) {
328331
ICEBERG_ASSIGN_OR_RAISE(auto map_json, GetJsonValue<nlohmann::json>(json, key));
329332
if (!map_json.is_object()) {
330333
return JsonParseError("Cannot parse '{}' from non-object: {}", key,
331-
map_json.dump());
334+
DumpJsonNoExcept(map_json));
332335
}
333336
for (const auto& [key, value] : map_json.items()) {
334337
ICEBERG_ASSIGN_OR_RAISE(auto entry, from_json(value));
@@ -633,7 +636,8 @@ Result<std::unique_ptr<Schema>> SchemaFromJson(const nlohmann::json& json) {
633636
ICEBERG_ASSIGN_OR_RAISE(auto type, TypeFromJson(json));
634637

635638
if (type->type_id() != TypeId::kStruct) [[unlikely]] {
636-
return JsonParseError("Schema must be a struct type, but got {}", json.dump());
639+
return JsonParseError("Schema must be a struct type, but got {}",
640+
DumpJsonNoExcept(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+
DumpJsonNoExcept(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: {}", DumpJsonNoExcept(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+
DumpJsonNoExcept(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(), DumpJsonNoExcept(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+
DumpJsonNoExcept(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+
DumpJsonNoExcept(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+
DumpJsonNoExcept(json));
11021108
}
11031109

11041110
auto table_metadata = std::make_unique<TableMetadata>();
@@ -1210,11 +1216,11 @@ 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 = nlohmann::json::parse(json_string, nullptr, false);
1220+
if (json.is_discarded()) {
1221+
return JsonParseError("Failed to parse JSON string: {}", json_string);
12171222
}
1223+
return json;
12181224
}
12191225

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

0 commit comments

Comments
 (0)