Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions score/json/json_serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ inline void JsonDeserializeStructImpl(DeserializeAsJson& visitor, Field& field,
}
else
{
score::cpp::ignore = visitor.error.emplace(std::move(field_content).error());
// Annotate with the failing field name; field_name is a static const char* from struct_visitable.
score::cpp::ignore = visitor.error.emplace(field_content.error().WithUserMessage(field_name));
}
}
else
Expand All @@ -220,7 +221,7 @@ inline void JsonDeserializeStructImpl(DeserializeAsJson& visitor, Field& field,
}
else
{
score::cpp::ignore = visitor.error.emplace(Error::kKeyNotFound, "Missing mandatory field in JSON object");
score::cpp::ignore = visitor.error.emplace(MakeError(Error::kKeyNotFound, field_name));
}
}
}
Expand Down
85 changes: 85 additions & 0 deletions score/json/json_serializer_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -719,5 +719,90 @@ TEST(JsonSerializerTest, UseCustomSerializationOnVisitableStruct)
EXPECT_EQ(serialized.As<Object>()->get().count("foo"), 1);
}

TEST(JsonSerializerTest, TypeMismatchErrorContainsFieldName)
{
RecordProperty("TestType", "interface-test");
RecordProperty("Verifies", "::score::json::FromJsonAny");
RecordProperty("Description",
"Verify that a type-mismatch deserialization error carries the name of the failing field in its "
"UserMessage");
RecordProperty("ASIL", "QM");
RecordProperty("Priority", "3");
RecordProperty("DerivationTechnique", "error-guessing");

// Given a JSON where string_val holds a number instead of a string
auto source = R"({
"integer_val": 42,
"string_val": 999,
"nested_val": {
"nested_int": 43,
"nested_bool": true,
"nested_array": [44, 45]
}
} )"_json;

// When deserializing into the struct
auto unit{FromJsonAny<TypeToSerialize>(std::move(source))};

// Then the error code is WrongType and the UserMessage names the offending field
ASSERT_FALSE(unit.has_value());
EXPECT_EQ(unit.error(), Error::kWrongType);
EXPECT_EQ(unit.error().UserMessage(), "string_val");
}

TEST(JsonSerializerTest, MissingMandatoryFieldErrorContainsFieldName)
{
RecordProperty("TestType", "interface-test");
RecordProperty("Verifies", "::score::json::FromJsonAny");
RecordProperty("Description",
"Verify that a missing-mandatory-field deserialization error carries the name of the absent field "
"in its UserMessage");
RecordProperty("ASIL", "QM");
RecordProperty("Priority", "3");
RecordProperty("DerivationTechnique", "error-guessing");

// Given a JSON where mandatory integer_val is absent
auto source = R"({"string_val": "hello"})"_json;

// When deserializing into the struct
auto unit{FromJsonAny<TypeToSerialize>(std::move(source))};

// Then the error code is KeyNotFound and the UserMessage names the missing field
ASSERT_FALSE(unit.has_value());
EXPECT_EQ(unit.error(), Error::kKeyNotFound);
EXPECT_EQ(unit.error().UserMessage(), "integer_val");
}

TEST(JsonSerializerTest, NestedTypeMismatchErrorContainsOutermostFieldName)
{
RecordProperty("TestType", "control-flow-analysis"); // data flow
RecordProperty("Verifies", "::score::json::FromJsonAny");
RecordProperty("Description",
"Verify that a type-mismatch inside a nested struct is reported under the parent field name, "
"because each deserialization level re-stamps the error with its own field key");
RecordProperty("ASIL", "QM");
RecordProperty("Priority", "3");
RecordProperty("DerivationTechnique", "error-guessing");

// Given a JSON where nested_val.nested_bool holds a string instead of a bool
auto source = R"({
"integer_val": 42,
"string_val": "ok",
"nested_val": {
"nested_int": 43,
"nested_bool": "not-a-bool",
"nested_array": [44, 45]
}
} )"_json;

// 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

ASSERT_FALSE(unit.has_value());
EXPECT_EQ(unit.error(), Error::kWrongType);
EXPECT_EQ(unit.error().UserMessage(), "nested_val");
}

} // namespace
} // namespace score::json::test
7 changes: 7 additions & 0 deletions score/result/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ class Error final
return user_messages_;
}

/// \brief Returns a copy of this error with an updated user message.
/// \return Copy preserving code and domain with the provided user message
[[nodiscard]] constexpr Error WithUserMessage(const std::string_view user_message) const noexcept
{
return Error{code_, *domain_, user_message};
}

private:
score::result::ErrorCode code_;
const score::result::ErrorDomain* domain_;
Expand Down
28 changes: 28 additions & 0 deletions score/result/error_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,34 @@ TEST(Error, CanLogCustomMessageToOstream)
EXPECT_EQ(stream.str(), "Error Second Error! occurred with message Foo");
}

TEST(Error, WithUserMessagePreservesCodeAndDomain)
{
// Given an error with a known code/domain
const score::result::Error original{MyErrorCode::kSecondError, "original message"};

// When creating a copy with a new user message
const score::result::Error updated = original.WithUserMessage("updated message");

// Then code/domain are preserved while the user message is replaced
EXPECT_EQ(updated, original);
EXPECT_EQ(*updated, *original);
EXPECT_EQ(updated.Message(), original.Message());
EXPECT_EQ(updated.UserMessage(), "updated message");
}

TEST(Error, WithUserMessageCanClearUserMessage)
{
// Given an error with an existing user message
const score::result::Error original{MyErrorCode::kFirstError, "has message"};

// When creating a copy with an empty user message
const score::result::Error updated = original.WithUserMessage("");

// Then the user message is cleared
EXPECT_EQ(updated, original);
EXPECT_TRUE(updated.UserMessage().empty());
}

} // namespace
} // namespace result
} // namespace score
Loading