substitution_format_utility: exception free

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk · Apr 3, 2024 · feb28a4 · feb28a4
1 parent 3a26f5a
commit feb28a4
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 17 deletions.
diff --git a/source/common/formatter/http_specific_formatter.cc b/source/common/formatter/http_specific_formatter.cc
@@ -397,7 +397,8 @@ FormatterProviderPtr HttpBuiltInCommandParser::parse(const std::string& command,
   }
 
   // Check flags for the command.
-  CommandSyntaxChecker::verifySyntax((*it).second.first, command, subcommand, max_length);
+  THROW_IF_NOT_OK(
+      CommandSyntaxChecker::verifySyntax((*it).second.first, command, subcommand, max_length));
 
   // Create a pointer to the formatter by calling a function
   // associated with formatter's name.

diff --git a/source/common/formatter/stream_info_formatter.h b/source/common/formatter/stream_info_formatter.h
@@ -278,7 +278,8 @@ class CommonStreamInfoFormatterBase : public FormatterProviderBase<FormatterCont
     }
 
     // Check flags for the command.
-    CommandSyntaxChecker::verifySyntax((*it).second.first, command, sub_command, max_length);
+    THROW_IF_NOT_OK(
+        CommandSyntaxChecker::verifySyntax((*it).second.first, command, sub_command, max_length));
 
     // Create a pointer to the formatter by calling a function
     // associated with formatter's name.

diff --git a/source/common/formatter/substitution_format_utility.cc b/source/common/formatter/substitution_format_utility.cc
@@ -12,20 +12,24 @@ namespace Formatter {
 
 static const std::string DefaultUnspecifiedValueString = "-";
 
-void CommandSyntaxChecker::verifySyntax(CommandSyntaxFlags flags, const std::string& command,
-                                        const std::string& subcommand,
-                                        const absl::optional<size_t>& length) {
+absl::Status CommandSyntaxChecker::verifySyntax(CommandSyntaxFlags flags,
+                                                const std::string& command,
+                                                const std::string& subcommand,
+                                                const absl::optional<size_t>& length) {
   if ((flags == COMMAND_ONLY) && ((subcommand.length() != 0) || length.has_value())) {
-    throwEnvoyExceptionOrPanic(fmt::format("{} does not take any parameters or length", command));
+    return absl::InvalidArgumentError(
+        fmt::format("{} does not take any parameters or length", command));
   }
 
   if ((flags & PARAMS_REQUIRED).any() && (subcommand.length() == 0)) {
-    throwEnvoyExceptionOrPanic(fmt::format("{} requires parameters", command));
+    return absl::InvalidArgumentError(fmt::format("{} requires parameters", command));
   }
 
   if ((flags & LENGTH_ALLOWED).none() && length.has_value()) {
-    throwEnvoyExceptionOrPanic(fmt::format("{} does not allow length to be specified.", command));
+    return absl::InvalidArgumentError(
+        fmt::format("{} does not allow length to be specified.", command));
   }
+  return absl::OkStatus();
 }
 
 const absl::optional<std::reference_wrapper<const std::string>>

diff --git a/source/common/formatter/substitution_format_utility.h b/source/common/formatter/substitution_format_utility.h
@@ -23,9 +23,9 @@ class CommandSyntaxChecker {
   static constexpr CommandSyntaxFlags PARAMS_OPTIONAL = 1 << 1;
   static constexpr CommandSyntaxFlags LENGTH_ALLOWED = 1 << 2;
 
-  static void verifySyntax(CommandSyntaxChecker::CommandSyntaxFlags flags,
-                           const std::string& command, const std::string& subcommand,
-                           const absl::optional<size_t>& length);
+  static absl::Status verifySyntax(CommandSyntaxChecker::CommandSyntaxFlags flags,
+                                   const std::string& command, const std::string& subcommand,
+                                   const absl::optional<size_t>& length);
 };
 
 /**

diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc
@@ -4225,9 +4225,9 @@ TEST(SubstitutionFormatParser, SyntaxVerifierFail) {
       };
 
   for (const auto& test_case : test_cases) {
-    EXPECT_THROW(CommandSyntaxChecker::verifySyntax(std::get<0>(test_case), "TEST_TOKEN",
-                                                    std::get<1>(test_case), std::get<2>(test_case)),
-                 EnvoyException);
+    EXPECT_FALSE(CommandSyntaxChecker::verifySyntax(std::get<0>(test_case), "TEST_TOKEN",
+                                                    std::get<1>(test_case), std::get<2>(test_case))
+                     .ok());
   }
 }
 
@@ -4256,8 +4256,9 @@ TEST(SubstitutionFormatParser, SyntaxVerifierPass) {
            absl::nullopt}};
 
   for (const auto& test_case : test_cases) {
-    EXPECT_NO_THROW(CommandSyntaxChecker::verifySyntax(
-        std::get<0>(test_case), "TEST_TOKEN", std::get<1>(test_case), std::get<2>(test_case)));
+    EXPECT_TRUE(CommandSyntaxChecker::verifySyntax(std::get<0>(test_case), "TEST_TOKEN",
+                                                   std::get<1>(test_case), std::get<2>(test_case))
+                    .ok());
   }
 }
 } // namespace

diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml
@@ -114,7 +114,6 @@ paths:
     - source/common/formatter/stream_info_formatter.h
     - source/common/formatter/stream_info_formatter.cc
     - source/common/formatter/substitution_formatter.h
-    - source/common/formatter/substitution_format_utility.cc
     - source/common/formatter/substitution_format_string.h
     - source/common/stats/tag_extractor_impl.cc
     - source/common/stats/tag_producer_impl.cc