diff --git a/envoy/common/exception.h b/envoy/common/exception.h index 745936ee26bc..9c82af1573ad 100644 --- a/envoy/common/exception.h +++ b/envoy/common/exception.h @@ -69,7 +69,7 @@ class EnvoyException : public std::runtime_error { template Type returnOrThrow(absl::StatusOr type_or_error) { THROW_IF_STATUS_NOT_OK(type_or_error, throw); - return type_or_error.value(); + return std::move(type_or_error.value()); } #define THROW_OR_RETURN_VALUE(expression, type) returnOrThrow(expression) diff --git a/source/common/stats/tag_extractor_impl.cc b/source/common/stats/tag_extractor_impl.cc index c48370880519..059d79305176 100644 --- a/source/common/stats/tag_extractor_impl.cc +++ b/source/common/stats/tag_extractor_impl.cc @@ -66,17 +66,16 @@ std::string TagExtractorImplBase::extractRegexPrefix(absl::string_view regex) { return prefix; } -TagExtractorPtr TagExtractorImplBase::createTagExtractor(absl::string_view name, - absl::string_view regex, - absl::string_view substr, - absl::string_view negative_match, - Regex::Type re_type) { +absl::StatusOr +TagExtractorImplBase::createTagExtractor(absl::string_view name, absl::string_view regex, + absl::string_view substr, absl::string_view negative_match, + Regex::Type re_type) { if (name.empty()) { - throwEnvoyExceptionOrPanic("tag_name cannot be empty"); + return absl::InvalidArgumentError("tag_name cannot be empty"); } if (regex.empty()) { - throwEnvoyExceptionOrPanic(fmt::format( + return absl::InvalidArgumentError(fmt::format( "No regex specified for tag specifier and no default regex for name: '{}'", name)); } switch (re_type) { diff --git a/source/common/stats/tag_extractor_impl.h b/source/common/stats/tag_extractor_impl.h index bf2d62b32362..0a3981a41cd8 100644 --- a/source/common/stats/tag_extractor_impl.h +++ b/source/common/stats/tag_extractor_impl.h @@ -64,12 +64,12 @@ class TagExtractorImplBase : public TagExtractor { * in order to match the regex. This is an optional performance tweak * to avoid large numbers of failed regex lookups. * @param re_type the regular expression syntax used (Regex::Type::StdRegex or Regex::Type::Re2). - * @return TagExtractorPtr newly constructed TagExtractor. + * @return TagExtractorPtr newly constructed TagExtractor or a failure status. */ - static TagExtractorPtr createTagExtractor(absl::string_view name, absl::string_view regex, - absl::string_view substr = "", - absl::string_view negative_match = "", - Regex::Type re_type = Regex::Type::StdRegex); + static absl::StatusOr + createTagExtractor(absl::string_view name, absl::string_view regex, absl::string_view substr = "", + absl::string_view negative_match = "", + Regex::Type re_type = Regex::Type::StdRegex); TagExtractorImplBase(absl::string_view name, absl::string_view regex, absl::string_view substr = ""); diff --git a/source/common/stats/tag_producer_impl.cc b/source/common/stats/tag_producer_impl.cc index 44e6c862e3ae..621d3e001934 100644 --- a/source/common/stats/tag_producer_impl.cc +++ b/source/common/stats/tag_producer_impl.cc @@ -39,7 +39,9 @@ TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v3::StatsConfig& "No regex specified for tag specifier and no default regex for name: '{}'", name)); } } else { - addExtractor(TagExtractorImplBase::createTagExtractor(name, tag_specifier.regex())); + addExtractor(THROW_OR_RETURN_VALUE( + TagExtractorImplBase::createTagExtractor(name, tag_specifier.regex()), + TagExtractorPtr)); } } else if (tag_specifier.tag_value_case() == envoy::config::metrics::v3::TagSpecifier::TagValueCase::kFixedValue) { @@ -53,8 +55,10 @@ int TagProducerImpl::addExtractorsMatching(absl::string_view name) { int num_found = 0; for (const auto& desc : Config::TagNames::get().descriptorVec()) { if (desc.name_ == name) { - addExtractor(TagExtractorImplBase::createTagExtractor(desc.name_, desc.regex_, desc.substr_, - desc.negative_match_, desc.re_type_)); + addExtractor(THROW_OR_RETURN_VALUE( + TagExtractorImplBase::createTagExtractor(desc.name_, desc.regex_, desc.substr_, + desc.negative_match_, desc.re_type_), + TagExtractorPtr)); ++num_found; } } @@ -135,8 +139,10 @@ void TagProducerImpl::reserveResources(const envoy::config::metrics::v3::StatsCo void TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v3::StatsConfig& config) { if (!config.has_use_all_default_tags() || config.use_all_default_tags().value()) { for (const auto& desc : Config::TagNames::get().descriptorVec()) { - addExtractor(TagExtractorImplBase::createTagExtractor(desc.name_, desc.regex_, desc.substr_, - desc.negative_match_, desc.re_type_)); + addExtractor(THROW_OR_RETURN_VALUE( + TagExtractorImplBase::createTagExtractor(desc.name_, desc.regex_, desc.substr_, + desc.negative_match_, desc.re_type_), + TagExtractorPtr)); } for (const auto& desc : Config::TagNames::get().tokenizedDescriptorVec()) { addExtractor(std::make_unique(desc.name_, desc.pattern_)); diff --git a/test/common/stats/tag_extractor_impl_test.cc b/test/common/stats/tag_extractor_impl_test.cc index 747f9daca0c1..a2bf3b928282 100644 --- a/test/common/stats/tag_extractor_impl_test.cc +++ b/test/common/stats/tag_extractor_impl_test.cc @@ -74,14 +74,15 @@ TEST(TagExtractorTest, noSubstrMismatch) { } TEST(TagExtractorTest, EmptyName) { - EXPECT_THROW_WITH_MESSAGE( - TagExtractorStdRegexImpl::createTagExtractor("", "^listener\\.(\\d+?\\.)"), EnvoyException, + EXPECT_EQ( + TagExtractorStdRegexImpl::createTagExtractor("", "^listener\\.(\\d+?\\.)").status().message(), "tag_name cannot be empty"); } TEST(TagExtractorTest, BadRegex) { - EXPECT_THROW_WITH_REGEX(TagExtractorStdRegexImpl::createTagExtractor("cluster_name", "+invalid"), - EnvoyException, "Invalid regex '\\+invalid':"); + EXPECT_THROW_WITH_REGEX( + TagExtractorStdRegexImpl::createTagExtractor("cluster_name", "+invalid").IgnoreError(), + EnvoyException, "Invalid regex '\\+invalid':"); } class DefaultTagRegexTester { @@ -489,7 +490,7 @@ TEST(TagExtractorTest, ExtAuthzTagExtractors) { TEST(TagExtractorTest, ExtractRegexPrefix) { TagExtractorPtr tag_extractor; // Keep tag_extractor in this scope to prolong prefix lifetime. auto extractRegexPrefix = [&tag_extractor](const std::string& regex) -> absl::string_view { - tag_extractor = TagExtractorStdRegexImpl::createTagExtractor("foo", regex); + tag_extractor = TagExtractorStdRegexImpl::createTagExtractor("foo", regex).value(); return tag_extractor->prefixToken(); }; @@ -504,8 +505,9 @@ TEST(TagExtractorTest, ExtractRegexPrefix) { } TEST(TagExtractorTest, CreateTagExtractorNoRegex) { - EXPECT_THROW_WITH_REGEX(TagExtractorStdRegexImpl::createTagExtractor("no such default tag", ""), - EnvoyException, "^No regex specified for tag specifier and no default"); + EXPECT_THAT( + TagExtractorStdRegexImpl::createTagExtractor("no such default tag", "").status().message(), + testing::ContainsRegex("^No regex specified for tag specifier and no default")); } class TagExtractorTokensTest : public testing::Test {