watched: removing exceptions (#37461)

Risk Level: low Testing: updated tests Docs Changes: n/a Release Notes: n/a envoyproxy/envoy-mobile#176 Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
envoyproxy · Dec 5, 2024 · d822e8a · d822e8a
1 parent 9494a9d
commit d822e8a
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 16 deletions.
diff --git a/source/common/config/watched_directory.cc b/source/common/config/watched_directory.cc
@@ -3,12 +3,23 @@
 namespace Envoy {
 namespace Config {
 
+absl::StatusOr<std::unique_ptr<WatchedDirectory>>
+WatchedDirectory::create(const envoy::config::core::v3::WatchedDirectory& config,
+                         Event::Dispatcher& dispatcher) {
+  absl::Status creation_status = absl::OkStatus();
+  auto ret =
+      std::unique_ptr<WatchedDirectory>(new WatchedDirectory(config, dispatcher, creation_status));
+  RETURN_IF_NOT_OK_REF(creation_status);
+  return ret;
+}
+
 WatchedDirectory::WatchedDirectory(const envoy::config::core::v3::WatchedDirectory& config,
-                                   Event::Dispatcher& dispatcher) {
+                                   Event::Dispatcher& dispatcher, absl::Status& creation_status) {
   watcher_ = dispatcher.createFilesystemWatcher();
-  THROW_IF_NOT_OK(watcher_->addWatch(absl::StrCat(config.path(), "/"),
-                                     Filesystem::Watcher::Events::MovedTo,
-                                     [this](uint32_t) { return cb_(); }));
+  SET_AND_RETURN_IF_NOT_OK(watcher_->addWatch(absl::StrCat(config.path(), "/"),
+                                              Filesystem::Watcher::Events::MovedTo,
+                                              [this](uint32_t) { return cb_(); }),
+                           creation_status);
 }
 
 } // namespace Config

diff --git a/source/common/config/watched_directory.h b/source/common/config/watched_directory.h
@@ -10,13 +10,17 @@ namespace Config {
 // Implement the common functionality of envoy::config::core::v3::WatchedDirectory.
 class WatchedDirectory {
 public:
-  using Callback = std::function<absl::Status()>;
+  static absl::StatusOr<std::unique_ptr<WatchedDirectory>>
+  create(const envoy::config::core::v3::WatchedDirectory& config, Event::Dispatcher& dispatcher);
 
-  WatchedDirectory(const envoy::config::core::v3::WatchedDirectory& config,
-                   Event::Dispatcher& dispatcher);
+  using Callback = std::function<absl::Status()>;
 
   void setCallback(Callback cb) { cb_ = cb; }
 
+protected:
+  WatchedDirectory(const envoy::config::core::v3::WatchedDirectory& config,
+                   Event::Dispatcher& dispatcher, absl::Status& creation_status);
+
 private:
   std::unique_ptr<Filesystem::Watcher> watcher_;
   Callback cb_;

diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc
@@ -270,8 +270,9 @@ void TlsCertificateSdsApi::setSecret(
           secret.tls_certificate());
   resolved_tls_certificate_secrets_ = nullptr;
   if (secret.tls_certificate().has_watched_directory()) {
-    watched_directory_ = std::make_unique<Config::WatchedDirectory>(
-        secret.tls_certificate().watched_directory(), dispatcher_);
+    watched_directory_ = THROW_OR_RETURN_VALUE(
+        Config::WatchedDirectory::create(secret.tls_certificate().watched_directory(), dispatcher_),
+        std::unique_ptr<Config::WatchedDirectory>);
   } else {
     watched_directory_.reset();
   }
@@ -324,8 +325,10 @@ void CertificateValidationContextSdsApi::setSecret(
           secret.validation_context());
   resolved_certificate_validation_context_secrets_ = nullptr;
   if (secret.validation_context().has_watched_directory()) {
-    watched_directory_ = std::make_unique<Config::WatchedDirectory>(
-        secret.validation_context().watched_directory(), dispatcher_);
+    watched_directory_ =
+        THROW_OR_RETURN_VALUE(Config::WatchedDirectory::create(
+                                  secret.validation_context().watched_directory(), dispatcher_),
+                              std::unique_ptr<Config::WatchedDirectory>);
   } else {
     watched_directory_.reset();
   }

diff --git a/source/extensions/config_subscription/filesystem/filesystem_subscription_impl.cc b/source/extensions/config_subscription/filesystem/filesystem_subscription_impl.cc
@@ -35,8 +35,9 @@ FilesystemSubscriptionImpl::FilesystemSubscriptionImpl(
           return absl::OkStatus();
         }));
   } else {
-    directory_watcher_ =
-        std::make_unique<WatchedDirectory>(path_config_source.watched_directory(), dispatcher);
+    directory_watcher_ = THROW_OR_RETURN_VALUE(
+        WatchedDirectory::create(path_config_source.watched_directory(), dispatcher),
+        std::unique_ptr<WatchedDirectory>);
     directory_watcher_->setCallback([this]() {
       if (started_) {
         refresh();

diff --git a/test/common/config/watched_directory_test.cc b/test/common/config/watched_directory_test.cc
@@ -23,9 +23,9 @@ TEST(WatchedDirectory, All) {
   Filesystem::Watcher::OnChangedCb cb;
   EXPECT_CALL(*watcher, addWatch("foo/bar/", Filesystem::Watcher::Events::MovedTo, _))
       .WillOnce(DoAll(SaveArg<2>(&cb), Return(absl::OkStatus())));
-  WatchedDirectory wd(config, dispatcher);
+  auto wd = *WatchedDirectory::create(config, dispatcher);
   bool called = false;
-  wd.setCallback([&called] {
+  wd->setCallback([&called] {
     called = true;
     return absl::OkStatus();
   });

diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml
@@ -140,7 +140,6 @@ paths:
     - source/common/grpc/async_client_impl.cc
     - source/common/grpc/google_grpc_creds_impl.cc
     - source/common/local_reply/local_reply.cc
-    - source/common/config/watched_directory.cc
     - source/server/drain_manager_impl.cc
     - source/common/router/rds_impl.cc
     - source/server/hot_restarting_parent.cc