feat(json): propagate field name in FromJsonAny deserialization errors - #467
Conversation
|
The created documentation from the pull request is available at: docu-html |
There was a problem hiding this comment.
Pull request overview
Adds struct field names to JSON deserialization error messages.
Changes:
- Annotates type and missing-field errors with field names.
- Adds tests for direct, missing, and nested-field failures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
score/json/json_serializer.h |
Adds field-name error annotation. |
score/json/json_serializer_test.cpp |
Tests field-name propagation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| score::cpp::ignore = visitor.error.emplace( | ||
| MakeError(static_cast<Error>(*field_content.error()), field_name)); |
There was a problem hiding this comment.
Fixed. Added Error::WithUserMessage() to error.h which returns a copy preserving the original numeric code and ErrorDomain*, replacing only user_messages_. JsonDeserializeStructImpl now calls field_content.error().WithUserMessage(field_name) instead of MakeError(static_castjson::Error(...))json::Error(...)), so a TypedHash field failure carrying score::hash::ErrorCode::kInvalidParameters propagates with its hash domain intact.
bb0a5d1 to
17d5a89
Compare
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Waiting
|
Documentation preview for this pull request is available at: |
17d5a89 to
756d896
Compare
JsonDeserializeStructImpl now annotates every deserialization failure with the name of the struct field that caused it, stored in the error's UserMessage(). Previously, field identity was lost and only a generic error code was returned. - Type mismatch: UserMessage() = failing field name (e.g. "pdu_id") - Missing mandatory field: UserMessage() = absent field name - Nested struct failure: UserMessage() = outermost containing field name field_name is a static const char* from struct_visitable, so no heap allocation occurs and no string_view lifetime issue is introduced. Add three tests covering type mismatch, missing key, and nested struct error propagation cases.
756d896 to
7fa9914
Compare
| // When deserializing into the struct | ||
| auto unit{FromJsonAny<TypeToSerialize>(std::move(source))}; | ||
|
|
||
| // Then the error code is WrongType and the UserMessage names the parent struct field, not the inner field |
There was a problem hiding this comment.
Shouldn't we add error for specific error field in nested struct, ie. "nested_bool" , "next_int" etc.
instead of "nested_value" which is again kind of generic error ?
There was a problem hiding this comment.
Hi @Nikhil2206 thanks for the review, for my side I have added some points.
The serializer re-stamps the error with the current field name at each struct level (see json_serializer.h line 209: field_content.error().WithUserMessage(field_name)), so callers always get the top-level failing field of the struct they directly deserialized.
Two reasons we keep it this way:
-
Stable error contract - if callers inspect UserMessage() for error handling or logging, exposing leaf names means renaming a field inside a nested type becomes a silent breaking change for all error consumers.
-
Encapsulation - reporting nested_val.nested_bool leaks the internal shape of NestedType as externally observable behavior.
let me know if you want to discuss more?
There was a problem hiding this comment.
Alright for now, however we can check the consumer of these logs and make sure it's not breaking then we can have opportunity to get that more detailed error message (the original intent of this work).
|
Hi @fbaeuerle and @4og could you please review this PR when you have a moment? Your feedback would be greatly appreciated. |
github-project-automation
Bot
moved this from In Progress
to On Hold
in BAS - Baselibs FT
JsonDeserializeStructImpl now annotates every deserialization failure with the name of the struct field that caused it, stored in the error's UserMessage(). Previously, field identity was lost and only a generic error code was returned.
field_name is a static const char* from struct_visitable, so no heap allocation occurs and no string_view lifetime issue is introduced.
Add three tests covering type mismatch, missing key, and nested struct error propagation cases.