header: getting rid of exception-throwing behaviors in header files […

…the rest] (#12611) getting rid of exception-throwing behaviors in header files Signed-off-by: Yifan Yang <[email protected]>
envoyproxy · Aug 14, 2020 · 5f3a35a · 5f3a35a
1 parent bd2b989
commit 5f3a35a
Show file tree

Hide file tree

Showing 10 changed files with 84 additions and 28 deletions.
diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc
@@ -251,6 +251,16 @@ ProtoValidationException::ProtoValidationException(const std::string& validation
   ENVOY_LOG_MISC(debug, "Proto validation error; throwing {}", what());
 }
 
+void ProtoExceptionUtil::throwMissingFieldException(const std::string& field_name,
+                                                    const Protobuf::Message& message) {
+  throw MissingFieldException(field_name, message);
+}
+
+void ProtoExceptionUtil::throwProtoValidationException(const std::string& validation_error,
+                                                       const Protobuf::Message& message) {
+  throw ProtoValidationException(validation_error, message);
+}
+
 size_t MessageUtil::hash(const Protobuf::Message& message) {
   std::string text_format;
 

diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h
@@ -23,10 +23,14 @@
 
 // Obtain the value of a wrapped field (e.g. google.protobuf.UInt32Value) if set. Otherwise, throw
 // a MissingFieldException.
-#define PROTOBUF_GET_WRAPPED_REQUIRED(message, field_name)                                         \
-  ((message).has_##field_name() ? (message).field_name().value()                                   \
-                                : throw MissingFieldException(#field_name, (message)))
 
+#define PROTOBUF_GET_WRAPPED_REQUIRED(message, field_name)                                         \
+  ([](const auto& msg) {                                                                           \
+    if (!msg.has_##field_name()) {                                                                 \
+      ::Envoy::ProtoExceptionUtil::throwMissingFieldException(#field_name, msg);                   \
+    }                                                                                              \
+    return msg.field_name().value();                                                               \
+  }((message)))
 // Obtain the milliseconds value of a google.protobuf.Duration field if set. Otherwise, return the
 // default value.
 #define PROTOBUF_GET_MS_OR_DEFAULT(message, field_name, default_value)                             \
@@ -48,14 +52,22 @@
 // Obtain the milliseconds value of a google.protobuf.Duration field if set. Otherwise, throw a
 // MissingFieldException.
 #define PROTOBUF_GET_MS_REQUIRED(message, field_name)                                              \
-  ((message).has_##field_name() ? DurationUtil::durationToMilliseconds((message).field_name())     \
-                                : throw MissingFieldException(#field_name, (message)))
+  ([](const auto& msg) {                                                                           \
+    if (!msg.has_##field_name()) {                                                                 \
+      ::Envoy::ProtoExceptionUtil::throwMissingFieldException(#field_name, msg);                   \
+    }                                                                                              \
+    return DurationUtil::durationToMilliseconds(msg.field_name());                                 \
+  }((message)))
 
 // Obtain the seconds value of a google.protobuf.Duration field if set. Otherwise, throw a
 // MissingFieldException.
 #define PROTOBUF_GET_SECONDS_REQUIRED(message, field_name)                                         \
-  ((message).has_##field_name() ? DurationUtil::durationToSeconds((message).field_name())          \
-                                : throw MissingFieldException(#field_name, (message)))
+  ([](const auto& msg) {                                                                           \
+    if (!msg.has_##field_name()) {                                                                 \
+      ::Envoy::ProtoExceptionUtil::throwMissingFieldException(#field_name, msg);                   \
+    }                                                                                              \
+    return DurationUtil::durationToSeconds(msg.field_name());                                      \
+  }((message)))
 
 namespace Envoy {
 namespace ProtobufPercentHelper {
@@ -90,10 +102,13 @@ uint64_t fractionalPercentDenominatorToInt(
 // @param field_name supplies the field name in the message.
 // @param default_value supplies the default if the field is not present.
 #define PROTOBUF_PERCENT_TO_DOUBLE_OR_DEFAULT(message, field_name, default_value)                  \
-  (!std::isnan((message).field_name().value())                                                     \
-       ? (message).has_##field_name() ? (message).field_name().value() : default_value             \
-       : throw EnvoyException(fmt::format("Value not in the range of 0..100 range.")))
-
+  ([](const auto& msg) -> double {                                                                 \
+    if (std::isnan(msg.field_name().value())) {                                                    \
+      ::Envoy::ExceptionUtil::throwEnvoyException(                                                 \
+          fmt::format("Value not in the range of 0..100 range."));                                 \
+    }                                                                                              \
+    return (msg).has_##field_name() ? (msg).field_name().value() : default_value;                  \
+  }((message)))
 // Convert an envoy::type::v3::Percent to a rounded integer or a default.
 // @param message supplies the proto message containing the field.
 // @param field_name supplies the field name in the message.
@@ -104,11 +119,15 @@ uint64_t fractionalPercentDenominatorToInt(
 // Issue: https://github.com/envoyproxy/protoc-gen-validate/issues/85
 #define PROTOBUF_PERCENT_TO_ROUNDED_INTEGER_OR_DEFAULT(message, field_name, max_value,             \
                                                        default_value)                              \
-  (!std::isnan((message).field_name().value())                                                     \
-       ? (message).has_##field_name()                                                              \
-             ? ProtobufPercentHelper::convertPercent((message).field_name().value(), max_value)    \
-             : ProtobufPercentHelper::checkAndReturnDefault(default_value, max_value)              \
-       : throw EnvoyException(fmt::format("Value not in the range of 0..100 range.")))
+  ([](const auto& msg) {                                                                           \
+    if (std::isnan(msg.field_name().value())) {                                                    \
+      ::Envoy::ExceptionUtil::throwEnvoyException(                                                 \
+          fmt::format("Value not in the range of 0..100 range."));                                 \
+    }                                                                                              \
+    return (msg).has_##field_name()                                                                \
+               ? ProtobufPercentHelper::convertPercent((msg).field_name().value(), max_value)      \
+               : ProtobufPercentHelper::checkAndReturnDefault(default_value, max_value);           \
+  }((message)))
 
 namespace Envoy {
 
@@ -185,6 +204,17 @@ class ProtoValidationException : public EnvoyException {
   ProtoValidationException(const std::string& validation_error, const Protobuf::Message& message);
 };
 
+/**
+ * utility functions to call when throwing exceptions in header files
+ */
+class ProtoExceptionUtil {
+public:
+  static void throwMissingFieldException(const std::string& field_name,
+                                         const Protobuf::Message& message);
+  static void throwProtoValidationException(const std::string& validation_error,
+                                            const Protobuf::Message& message);
+};
+
 class MessageUtil {
 public:
   // std::hash
@@ -256,7 +286,7 @@ class MessageUtil {
 
     std::string err;
     if (!Validate(message, &err)) {
-      throw ProtoValidationException(err, API_RECOVER_ORIGINAL(message));
+      ProtoExceptionUtil::throwProtoValidationException(err, API_RECOVER_ORIGINAL(message));
     }
   }
 

diff --git a/source/extensions/common/BUILD b/source/extensions/common/BUILD
@@ -17,5 +17,6 @@ envoy_cc_library(
         "//include/envoy/runtime:runtime_interface",
         "//source/common/common:documentation_url_lib",
         "//source/common/common:minimal_logger_lib",
+        "//source/common/common:utility_lib",
     ],
 )
diff --git a/source/extensions/common/utility.h b/source/extensions/common/utility.h
@@ -5,6 +5,7 @@
 
 #include "common/common/documentation_url.h"
 #include "common/common/logger.h"
+#include "common/common/utility.h"
 
 namespace Envoy {
 namespace Extensions {
@@ -92,7 +93,8 @@ class ExtensionNameUtil {
       return;
     }
 
-    throw EnvoyException(fatalMessage(extension_type, deprecated_name, canonical_name));
+    ExceptionUtil::throwEnvoyException(
+        fatalMessage(extension_type, deprecated_name, canonical_name));
   }
 
 private:

diff --git a/source/extensions/filters/network/common/BUILD b/source/extensions/filters/network/common/BUILD
@@ -15,6 +15,7 @@ envoy_cc_library(
     visibility = ["//visibility:public"],
     deps = [
         "//include/envoy/server:filter_config_interface",
+        "//source/extensions/common:utility_lib",
     ],
 )
 

diff --git a/source/extensions/filters/network/common/factory_base.h b/source/extensions/filters/network/common/factory_base.h
@@ -4,6 +4,8 @@
 #include "envoy/server/transport_socket_config.h"
 #include "envoy/upstream/upstream.h"
 
+#include "common/common/utility.h"
+
 namespace Envoy {
 namespace Extensions {
 namespace NetworkFilters {
@@ -56,7 +58,8 @@ class FactoryBase : public Server::Configuration::NamedNetworkFilterConfigFactor
   virtual Upstream::ProtocolOptionsConfigConstSharedPtr
   createProtocolOptionsTyped(const ProtocolOptionsProto&,
                              Server::Configuration::ProtocolOptionsFactoryContext&) {
-    throw EnvoyException(fmt::format("filter {} does not support protocol options", name_));
+    ExceptionUtil::throwEnvoyException(
+        fmt::format("filter {} does not support protocol options", name_));
   }
 
   const std::string name_;

diff --git a/source/extensions/filters/network/dubbo_proxy/app_exception.h b/source/extensions/filters/network/dubbo_proxy/app_exception.h
@@ -2,6 +2,8 @@
 
 #include "envoy/common/exception.h"
 
+#include "common/common/utility.h"
+
 #include "extensions/filters/network/dubbo_proxy/filters/filter.h"
 #include "extensions/filters/network/dubbo_proxy/metadata.h"
 #include "extensions/filters/network/dubbo_proxy/protocol.h"
@@ -32,7 +34,7 @@ struct AppExceptionBase : public EnvoyException,
     metadata.setResponseStatus<T>(status_);
     metadata.setMessageType(MessageType::Response);
     if (!protocol.encode(buffer, metadata, what(), response_type_)) {
-      throw EnvoyException("Failed to encode local reply message");
+      ExceptionUtil::throwEnvoyException("Failed to encode local reply message");
     }
 
     return ResponseType::Exception;

diff --git a/source/extensions/filters/network/kafka/serialization.h b/source/extensions/filters/network/kafka/serialization.h
@@ -11,6 +11,7 @@
 
 #include "common/common/byte_order.h"
 #include "common/common/fmt.h"
+#include "common/common/utility.h"
 
 #include "extensions/filters/network/kafka/kafka_types.h"
 
@@ -206,7 +207,8 @@ class VarUInt32Deserializer : public Deserializer<uint32_t> {
         offset_ += 7;
         // Valid input can have at most 5 bytes.
         if (offset_ >= 5 * 7) {
-          throw EnvoyException("VarUInt32 is too long (5th byte has highest bit set)");
+          ExceptionUtil::throwEnvoyException(
+              "VarUInt32 is too long (5th byte has highest bit set)");
         }
       }
     }
@@ -463,7 +465,7 @@ class ArrayDeserializer : public Deserializer<std::vector<typename DeserializerT
       if (required_ >= 0) {
         children_ = std::vector<DeserializerType>(required_);
       } else {
-        throw EnvoyException(absl::StrCat("invalid ARRAY length: ", required_));
+        ExceptionUtil::throwEnvoyException(absl::StrCat("invalid ARRAY length: ", required_));
       }
       length_consumed_ = true;
     }
@@ -542,7 +544,8 @@ class CompactArrayDeserializer
       if (required >= 1) {
         children_ = std::vector<DeserializerType>(required - 1);
       } else {
-        throw EnvoyException(absl::StrCat("invalid COMPACT_ARRAY length: ", required));
+        ExceptionUtil::throwEnvoyException(
+            absl::StrCat("invalid COMPACT_ARRAY length: ", required));
       }
       length_consumed_ = true;
     }
@@ -625,7 +628,8 @@ class NullableArrayDeserializer
         ready_ = true;
       }
       if (required_ < NULL_ARRAY_LENGTH) {
-        throw EnvoyException(fmt::format("invalid NULLABLE_ARRAY length: {}", required_));
+        ExceptionUtil::throwEnvoyException(
+            fmt::format("invalid NULLABLE_ARRAY length: {}", required_));
       }
 
       length_consumed_ = true;

diff --git a/source/extensions/filters/network/mongo_proxy/bson_impl.h b/source/extensions/filters/network/mongo_proxy/bson_impl.h
@@ -8,6 +8,7 @@
 #include "envoy/common/exception.h"
 
 #include "common/common/logger.h"
+#include "common/common/utility.h"
 
 #include "extensions/filters/network/mongo_proxy/bson.h"
 
@@ -153,7 +154,7 @@ class FieldImpl : public Field {
 private:
   void checkType(Type type) const {
     if (type_ != type) {
-      throw EnvoyException("invalid BSON field type cast");
+      ExceptionUtil::throwEnvoyException("invalid BSON field type cast");
     }
   }
 

diff --git a/source/extensions/filters/network/thrift_proxy/thrift_object.h b/source/extensions/filters/network/thrift_proxy/thrift_object.h
@@ -6,6 +6,8 @@
 #include "envoy/buffer/buffer.h"
 #include "envoy/common/exception.h"
 
+#include "common/common/utility.h"
+
 #include "extensions/filters/network/thrift_proxy/thrift.h"
 
 namespace Envoy {
@@ -36,9 +38,9 @@ class ThriftValue {
     // and throw if the value's type doesn't match.
     FieldType expected_field_type = Traits<T>::getFieldType();
     if (expected_field_type != type()) {
-      throw EnvoyException(fmt::format("expected field type {}, got {}",
-                                       static_cast<int>(expected_field_type),
-                                       static_cast<int>(type())));
+      ExceptionUtil::throwEnvoyException(fmt::format("expected field type {}, got {}",
+                                                     static_cast<int>(expected_field_type),
+                                                     static_cast<int>(type())));
     }
 
     return *static_cast<const T*>(getValue());