Skip to content

Commit

Permalink
Reorder proto validation error message (envoyproxy#38089)
Browse files Browse the repository at this point in the history
Commit Message: Reorder proto validation error message
Additional Description: As discussed in envoyproxy#38064, the error message is
currently often hard to interpret due to the ordering being "high-level
low-level high-level" rather than the usual high-to-low ordering error
messages have, leading to confusing adjacencies. This is a minimal
reordering that doesn't break tests and makes the error message easier
to read, for example

`Proto constraint validation failed
(HttpProtocolOptionsValidationError.ExplicitHttpConfig: embedded message
failed validation | caused by field: "protocol_config", reason: is
required): explicit_http_config`

becomes

`explicit_http_config: Proto constraint validation failed
(HttpProtocolOptionsValidationError.ExplicitHttpConfig: embedded message
failed validation | caused by field: "protocol_config", reason: is
required)`

Risk Level: Very low it's just an error string.
Testing: Existing tests still pass.
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

---------

Signed-off-by: Raven Black <[email protected]>
  • Loading branch information
ravenblackx authored and bazmurphy committed Jan 29, 2025
1 parent 91cdac5 commit a493fde
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
8 changes: 4 additions & 4 deletions source/common/protobuf/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ void ProtoExceptionUtil::throwMissingFieldException(const std::string& field_nam

void ProtoExceptionUtil::throwProtoValidationException(const std::string& validation_error,
const Protobuf::Message& message) {
std::string error = fmt::format("Proto constraint validation failed ({}): {}", validation_error,
message.DebugString());
std::string error = fmt::format("{}: Proto constraint validation failed ({})",
message.DebugString(), validation_error);
throwEnvoyExceptionOrPanic(error);
}

Expand Down Expand Up @@ -429,8 +429,8 @@ class PgvCheckVisitor : public ProtobufMessage::ConstProtoVisitor {
// at which PGV would have stopped because it does not itself check within Any messages.
if (was_any_or_top_level &&
!pgv::BaseValidator::AbstractCheckMessage(*reflectable_message, &err)) {
std::string error = fmt::format("Proto constraint validation failed ({}): {}", err,
reflectable_message->DebugString());
std::string error = fmt::format("{}: Proto constraint validation failed ({})",
reflectable_message->DebugString(), err);
return absl::InvalidArgumentError(error);
}
return absl::OkStatus();
Expand Down
4 changes: 2 additions & 2 deletions test/extensions/string_matcher/lua/lua_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ TEST(LuaStringMatcher, NoCode) {

LuaStringMatcherFactory factory;
::envoy::extensions::string_matcher::lua::v3::Lua empty_config;
EXPECT_THROW_WITH_MESSAGE(
EXPECT_THROW_WITH_REGEX(
factory.createStringMatcher(empty_config, context), EnvoyException,
"Proto constraint validation failed (LuaValidationError.SourceCode: value is required): ");
"Proto constraint validation failed.*LuaValidationError.SourceCode: value is required");

empty_config.mutable_source_code()->set_inline_string("");
EXPECT_THROW_WITH_MESSAGE(factory.createStringMatcher(empty_config, context), EnvoyException,
Expand Down

0 comments on commit a493fde

Please sign in to comment.