local reply: removing exceptions

source/common/local_reply/local_reply.cc

@@ -20,11 +20,18 @@ class BodyFormatter {
 public:
   BodyFormatter() : content_type_(Http::Headers::get().ContentTypeValues.Text) {}
 
+  static absl::StatusOr<std::unique_ptr<BodyFormatter>>
+  create(const envoy::config::core::v3::SubstitutionFormatString& config,
+         Server::Configuration::GenericFactoryContext& context) {
+    auto formatter_or_error =
+        Formatter::SubstitutionFormatStringUtils::fromProtoConfig(config, context);
+    RETURN_IF_NOT_OK_REF(formatter_or_error.status());
+    return std::make_unique<BodyFormatter>(config, std::move(*formatter_or_error));
+  }
+
   BodyFormatter(const envoy::config::core::v3::SubstitutionFormatString& config,
-                Server::Configuration::GenericFactoryContext& context)
-      : formatter_(THROW_OR_RETURN_VALUE(
-            Formatter::SubstitutionFormatStringUtils::fromProtoConfig(config, context),
-            Formatter::FormatterBasePtr<Formatter::HttpFormatterContext>)),
+                Formatter::FormatterPtr&& formatter)
+      : formatter_(std::move(formatter)),
         content_type_(
             !config.content_type().empty() ? config.content_type()
             : config.format_case() ==
@@ -57,26 +64,40 @@ using HeaderParserPtr = std::unique_ptr<Envoy::Router::HeaderParser>;
 
 class ResponseMapper {
 public:
+  static absl::StatusOr<std::unique_ptr<ResponseMapper>>
+  create(const envoy::extensions::filters::network::http_connection_manager::v3::ResponseMapper&
+             config,
+         Server::Configuration::FactoryContext& context) {
+    absl::Status creation_status = absl::OkStatus();
+    auto ret = std::make_unique<ResponseMapper>(config, context, creation_status);
+    RETURN_IF_NOT_OK(creation_status);
+    return ret;
+  }
+
   ResponseMapper(
       const envoy::extensions::filters::network::http_connection_manager::v3::ResponseMapper&
           config,
-      Server::Configuration::FactoryContext& context)
+      Server::Configuration::FactoryContext& context, absl::Status& creation_status)
       : filter_(AccessLog::FilterFactory::fromProto(config.filter(), context)) {
     if (config.has_status_code()) {
       status_code_ = static_cast<Http::Code>(config.status_code().value());
     }
     if (config.has_body()) {
-      body_ = THROW_OR_RETURN_VALUE(
-          Config::DataSource::read(config.body(), true, context.serverFactoryContext().api()),
-          std::string);
+      auto body_or_error =
+          Config::DataSource::read(config.body(), true, context.serverFactoryContext().api());
+      SET_AND_RETURN_IF_NOT_OK(body_or_error.status(), creation_status);
+      body_ = *body_or_error;
     }
 
     if (config.has_body_format_override()) {
-      body_formatter_ = std::make_unique<BodyFormatter>(config.body_format_override(), context);
+      auto formatter_or_error = BodyFormatter::create(config.body_format_override(), context);
+      SET_AND_RETURN_IF_NOT_OK(formatter_or_error.status(), creation_status);
+      body_formatter_ = std::move(*formatter_or_error);
     }
 
-    header_parser_ = THROW_OR_RETURN_VALUE(
-        Envoy::Router::HeaderParser::configure(config.headers_to_add()), HeaderParserPtr);
+    auto parser_or_error = Envoy::Router::HeaderParser::configure(config.headers_to_add());
+    SET_AND_RETURN_IF_NOT_OK(parser_or_error.status(), creation_status);
+    header_parser_ = std::move(*parser_or_error);
   }
 
   bool matchAndRewrite(const Http::RequestHeaderMap& request_headers,
@@ -124,15 +145,32 @@ class LocalReplyImpl : public LocalReply {
 public:
   LocalReplyImpl() : body_formatter_(std::make_unique<BodyFormatter>()) {}
 
+  static absl::StatusOr<std::unique_ptr<LocalReplyImpl>>
+  create(const envoy::extensions::filters::network::http_connection_manager::v3::LocalReplyConfig&
+             config,
+         Server::Configuration::FactoryContext& context) {
+    absl::Status creation_status = absl::OkStatus();
+    auto ret = std::make_unique<LocalReplyImpl>(config, context, creation_status);
+    RETURN_IF_NOT_OK(creation_status);
+    return ret;
+  }
+
   LocalReplyImpl(
       const envoy::extensions::filters::network::http_connection_manager::v3::LocalReplyConfig&
           config,
-      Server::Configuration::FactoryContext& context)
-      : body_formatter_(config.has_body_format()
-                            ? std::make_unique<BodyFormatter>(config.body_format(), context)
-                            : std::make_unique<BodyFormatter>()) {
+      Server::Configuration::FactoryContext& context, absl::Status& creation_status) {
+    if (!config.has_body_format()) {
+      body_formatter_ = std::make_unique<BodyFormatter>();
+    } else {
+      auto formatter_or_error = BodyFormatter::create(config.body_format(), context);
+      SET_AND_RETURN_IF_NOT_OK(formatter_or_error.status(), creation_status);
+      body_formatter_ = std::move(*formatter_or_error);
+    }
+
     for (const auto& mapper : config.mappers()) {
-      mappers_.emplace_back(std::make_unique<ResponseMapper>(mapper, context));
+      auto mapper_or_error = ResponseMapper::create(mapper, context);
+      SET_AND_RETURN_IF_NOT_OK(mapper_or_error.status(), creation_status);
+      mappers_.emplace_back(std::move(*mapper_or_error));
     }
   }
 
@@ -169,16 +207,16 @@ class LocalReplyImpl : public LocalReply {
 
 private:
   std::list<ResponseMapperPtr> mappers_;
-  const BodyFormatterPtr body_formatter_;
+  BodyFormatterPtr body_formatter_;
 };
 
 LocalReplyPtr Factory::createDefault() { return std::make_unique<LocalReplyImpl>(); }
 
-LocalReplyPtr Factory::create(
+absl::StatusOr<LocalReplyPtr> Factory::create(
     const envoy::extensions::filters::network::http_connection_manager::v3::LocalReplyConfig&
         config,
     Server::Configuration::FactoryContext& context) {
-  return std::make_unique<LocalReplyImpl>(config, context);
+  return LocalReplyImpl::create(config, context);
 }
 
 } // namespace LocalReply

source/common/local_reply/local_reply.h

@@ -38,7 +38,7 @@ class Factory {
   /**
    * Create a LocalReply object from ProtoConfig
    */
-  static LocalReplyPtr
+  static absl::StatusOr<LocalReplyPtr>
   create(const envoy::extensions::filters::network::http_connection_manager::v3::LocalReplyConfig&
              config,
          Server::Configuration::FactoryContext& context);

source/extensions/filters/network/http_connection_manager/config.cc

@@ -417,7 +417,6 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
       merge_slashes_(config.merge_slashes()),
       headers_with_underscores_action_(
           config.common_http_protocol_options().headers_with_underscores_action()),
-      local_reply_(LocalReply::Factory::create(config.local_reply_config(), context)),
       path_with_escaped_slashes_action_(getPathWithEscapedSlashesAction(config, context)),
       strip_trailing_host_dot_(config.strip_trailing_host_dot()),
       max_requests_per_connection_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(
@@ -434,6 +433,9 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
   if (!creation_status.ok()) {
     return;
   }
+  auto local_reply_or_error = LocalReply::Factory::create(config.local_reply_config(), context);
+  SET_AND_RETURN_IF_NOT_OK(local_reply_or_error.status(), creation_status);
+  local_reply_ = std::move(*local_reply_or_error);
 
   auto options_or_error = Http2::Utility::initializeAndValidateOptions(
       config.http2_protocol_options(), config.has_stream_error_on_invalid_http_message(),

source/extensions/filters/network/http_connection_manager/config.h

@@ -351,7 +351,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   Http::StripPortType strip_port_type_;
   const envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
       headers_with_underscores_action_;
-  const LocalReply::LocalReplyPtr local_reply_;
+  LocalReply::LocalReplyPtr local_reply_;
   std::vector<Http::OriginalIPDetectionSharedPtr> original_ip_detection_extensions_{};
   std::vector<Http::EarlyHeaderMutationPtr> early_header_mutation_extensions_{};
 

test/common/local_reply/local_reply_test.cc

@@ -51,7 +51,7 @@ class LocalReplyTest : public testing::Test {
 
 TEST_F(LocalReplyTest, TestEmptyConfig) {
   // Empty LocalReply config.
-  auto local = Factory::create(config_, context_);
+  auto local = *Factory::create(config_, context_);
 
   local->rewrite(nullptr, response_headers_, stream_info_, code_, body_, content_type_);
   EXPECT_EQ(code_, TestInitCode);
@@ -114,7 +114,7 @@ TEST_F(LocalReplyTest, TestDefaultTextFormatter) {
        inline_string: "%LOCAL_REPLY_BODY% %RESPONSE_CODE%"
 )";
   TestUtility::loadFromYaml(yaml, config_);
-  auto local = Factory::create(config_, context_);
+  auto local = *Factory::create(config_, context_);
 
   local->rewrite(nullptr, response_headers_, stream_info_, code_, body_, content_type_);
   EXPECT_EQ(code_, TestInitCode);
@@ -136,7 +136,7 @@ TEST_F(LocalReplyTest, TestDefaultJsonFormatter) {
       body: "%LOCAL_REPLY_BODY%"
 )";
   TestUtility::loadFromYaml(yaml, config_);
-  auto local = Factory::create(config_, context_);
+  auto local = *Factory::create(config_, context_);
 
   local->rewrite(&request_headers_, response_headers_, stream_info_, code_, body_, content_type_);
   EXPECT_EQ(code_, TestInitCode);
@@ -203,7 +203,7 @@ TEST_F(LocalReplyTest, TestMapperRewrite) {
               runtime_key: key_b
 )";
   TestUtility::loadFromYaml(yaml, config_);
-  auto local = Factory::create(config_, context_);
+  auto local = *Factory::create(config_, context_);
 
   // code=400 matches the first filter; rewrite code and body
   resetData(400);
@@ -270,7 +270,7 @@ TEST_F(LocalReplyTest, DEPRECATED_FEATURE_TEST(TestMapperRewriteDeprecatedTextFo
         text_format: ""
 )";
   TestUtility::loadFromYaml(yaml, config_);
-  auto local = Factory::create(config_, context_);
+  auto local = *Factory::create(config_, context_);
 
   // code=404 matches the only filter; does not rewrite code, sets an empty body and content_type.
   resetData(404);
@@ -318,7 +318,7 @@ TEST_F(LocalReplyTest, TestMapperFormat) {
         inline_string: "%LOCAL_REPLY_BODY% %RESPONSE_CODE% default formatter"
 )";
   TestUtility::loadFromYaml(yaml, config_);
-  auto local = Factory::create(config_, context_);
+  auto local = *Factory::create(config_, context_);
 
   // code=400 matches the first filter; rewrite code and body
   // has its own formatter
@@ -377,7 +377,7 @@ TEST_F(LocalReplyTest, TestHeaderAddition) {
             value: '%REQ(req-id)%'
 )";
   TestUtility::loadFromYaml(yaml, config_);
-  auto local = Factory::create(config_, context_);
+  auto local = *Factory::create(config_, context_);
 
   response_headers_.addCopy("foo-2", "bar2");
   response_headers_.addCopy("foo-3", "bar3");
@@ -444,7 +444,7 @@ TEST_F(LocalReplyTest, TestMapperWithContentType) {
       content_type: "text/html; charset=UTF-8"
 )";
   TestUtility::loadFromYaml(yaml, config_);
-  auto local = Factory::create(config_, context_);
+  auto local = *Factory::create(config_, context_);
 
   // code=400 matches the first filter; rewrite code and body
   // has its own formatter.

tools/code_format/config.yaml

@@ -138,7 +138,6 @@ paths:
     - source/common/secret/sds_api.cc
     - source/common/config/config_provider_impl.h
     - source/common/grpc/google_grpc_creds_impl.cc
-    - source/common/local_reply/local_reply.cc
     - source/common/router/rds_impl.cc
     - source/server/hot_restarting_parent.cc
     - source/common/io/io_uring_worker_impl.cc