fix: Handle invalid UTF-8 sequences gracefully when converting structured log events to JSON. #52
Conversation
WalkthroughThe pull request introduces a new function, Changes
Sequence DiagramsequenceDiagram
participant Reader as StructuredIrStreamReader
participant JSON as JSON Serializer
Reader->>JSON: Prepare JSON serialization
JSON-->>Reader: Generate JSON string with updated parameters
Note over JSON: Parameters:
Note over JSON: - No specific indentation (-1)
Note over JSON: - Space as indentation
Note over JSON: - Error replacement mode
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| @@ -132,7 +133,8 @@ auto StructuredIrStreamReader::decode_range(size_t begin_idx, size_t end_idx, bo | |||
| ); | |||
| json_str = std::string(cEmptyJsonStr); | |||
| } else { | |||
| json_str = json_result.value().dump(); | |||
| json_str = json_result.value() | |||
| .dump(-1, ' ', false, nlohmann::detail::error_handler_t::replace); | |||
There was a problem hiding this comment.
Since this is in their example: https://json.nlohmann.me/api/basic_json/error_handler_t/
shall we use the nlohmann::json::error_handler_t::replace alias instead of the one from the detail namespace?
| @@ -132,7 +133,8 @@ auto StructuredIrStreamReader::decode_range(size_t begin_idx, size_t end_idx, bo | |||
| ); | |||
| json_str = std::string(cEmptyJsonStr); | |||
| } else { | |||
| json_str = json_result.value().dump(); | |||
| json_str = json_result.value() | |||
There was a problem hiding this comment.
The magic values seem a bit odd. Shall we consider creating a helper for this? e.g.,
Add this to the anonymous namespace:
constexpr auto cDumpWithReplace = [](auto const& json_obj) {
return json_obj.dump(-1, ' ', false, nlohmann::json::error_handler_t::replace);
};
There was a problem hiding this comment.
As discussed offline, the code should clear enough and there's no need to make a helper. Please ignore this comment.
There was a problem hiding this comment.
See helper I added. I can remove if you'd like. lmk
| */ | ||
| auto json_dump_with_replace(nlohmann::json const& json) -> std::string; | ||
|
|
||
| auto json_dump_with_replace(nlohmann::json const& json) -> std::string { |
There was a problem hiding this comment.
I'll probably call this dump_json_with_replace but feel free to object
|
|
||
| /** | ||
| * @see nlohmann::basic_json::dump | ||
| * Dumps a JSON value with error handling set to replace invalid UTF-8 characters rather than |
There was a problem hiding this comment.
Would it be more clear if we say
"Serializes a JSON value into a string with invalid UTF-8 sequences replaced rather than ..."
|
Okay I tested still works with newer changes. @junhaoliao please approve again and i will merge |
davemarco
changed the title
Description
Fixes issue where some files with invalid UTF-8 could not be opened. Now chars are just replaced.
Validation performed
Tested file could now be opened. Tested older IRV2 test file still worked
Summary by CodeRabbit
These changes provide a more flexible approach to converting structured data to JSON, ensuring better compatibility and error management during serialization.