diff --git a/BUILD b/BUILD index 436ebb07180..00a775248f0 100644 --- a/BUILD +++ b/BUILD @@ -3093,6 +3093,8 @@ grpc_cc_library( ], external_deps = [ "absl/memory", + "absl/status", + "absl/status:statusor", "absl/strings", ], language = "c++", @@ -3119,11 +3121,14 @@ grpc_cc_library( hdrs = [ "src/core/lib/service_config/service_config_parser.h", ], - external_deps = ["absl/strings"], + external_deps = [ + "absl/status", + "absl/status:statusor", + "absl/strings", + ], language = "c++", deps = [ "channel_args", - "error", "gpr_base", "json", ], @@ -3543,6 +3548,8 @@ grpc_cc_library( ], external_deps = [ "absl/memory", + "absl/status", + "absl/status:statusor", "absl/strings", "absl/strings:str_format", "absl/types:optional", @@ -6453,6 +6460,8 @@ grpc_cc_library( hdrs = GRPCXX_HDRS, external_deps = [ "absl/base:core_headers", + "absl/status", + "absl/status:statusor", "absl/strings", "absl/synchronization", "absl/memory", @@ -6497,6 +6506,8 @@ grpc_cc_library( hdrs = GRPCXX_HDRS, external_deps = [ "absl/base:core_headers", + "absl/status", + "absl/status:statusor", "absl/strings", "absl/synchronization", "absl/memory", diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 30c96331012..9e705f572aa 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -999,12 +999,13 @@ ClientChannel::ClientChannel(grpc_channel_element_args* args, channel_args_.GetString(GRPC_ARG_SERVICE_CONFIG); if (!service_config_json.has_value()) service_config_json = "{}"; *error = GRPC_ERROR_NONE; - default_service_config_ = - ServiceConfigImpl::Create(channel_args_, *service_config_json, error); - if (!GRPC_ERROR_IS_NONE(*error)) { - default_service_config_.reset(); + auto service_config = + ServiceConfigImpl::Create(channel_args_, *service_config_json); + if (!service_config.ok()) { + *error = absl_status_to_grpc_error(service_config.status()); return; } + default_service_config_ = std::move(*service_config); // Get URI to resolve, using proxy mapper if needed. absl::optional server_uri = channel_args_.GetOwnedString(GRPC_ARG_SERVER_URI); diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index 2ed8d6f4ed3..af536a6ee0a 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -407,22 +407,21 @@ AresClientChannelDNSResolver::AresRequestWrapper::OnResolvedLocked( grpc_error_handle service_config_error = GRPC_ERROR_NONE; std::string service_config_string = ChooseServiceConfig(service_config_json_, &service_config_error); - RefCountedPtr service_config; - if (GRPC_ERROR_IS_NONE(service_config_error) && - !service_config_string.empty()) { - GRPC_CARES_TRACE_LOG("resolver:%p selected service config choice: %s", - this, service_config_string.c_str()); - service_config = ServiceConfigImpl::Create(resolver_->channel_args(), - service_config_string, - &service_config_error); - } if (!GRPC_ERROR_IS_NONE(service_config_error)) { result.service_config = absl::UnavailableError( absl::StrCat("failed to parse service config: ", grpc_error_std_string(service_config_error))); GRPC_ERROR_UNREF(service_config_error); - } else { - result.service_config = std::move(service_config); + } else if (!service_config_string.empty()) { + GRPC_CARES_TRACE_LOG("resolver:%p selected service config choice: %s", + this, service_config_string.c_str()); + result.service_config = ServiceConfigImpl::Create( + resolver_->channel_args(), service_config_string); + if (!result.service_config.ok()) { + result.service_config = absl::UnavailableError( + absl::StrCat("failed to parse service config: ", + result.service_config.status().message())); + } } } if (balancer_addresses_ != nullptr) { diff --git a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc index 283fdc76638..5fe88c40d5d 100644 --- a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc @@ -616,8 +616,13 @@ grpc_error_handle XdsResolver::XdsConfigSelector::CreateMethodConfig( absl::StrJoin(fields, ",\n"), "\n } ]\n" "}"); - *method_config = - ServiceConfigImpl::Create(result.args, json.c_str(), &error); + auto method_config_or = + ServiceConfigImpl::Create(result.args, json.c_str()); + if (!method_config_or.ok()) { + error = absl_status_to_grpc_error(method_config_or.status()); + } else { + *method_config = std::move(*method_config_or); + } } return error; } @@ -993,9 +998,8 @@ void XdsResolver::OnResourceDoesNotExist(std::string context) { current_virtual_host_.routes.clear(); Result result; result.addresses.emplace(); - grpc_error_handle error = GRPC_ERROR_NONE; - result.service_config = ServiceConfigImpl::Create(args_, "{}", &error); - GPR_ASSERT(*result.service_config != nullptr); + result.service_config = ServiceConfigImpl::Create(args_, "{}"); + GPR_ASSERT(result.service_config.ok()); result.resolution_note = std::move(context); result.args = args_; result_handler_->ReportResult(std::move(result)); @@ -1039,14 +1043,7 @@ XdsResolver::CreateServiceConfig() { " ]\n" "}"); std::string json = absl::StrJoin(config_parts, ""); - grpc_error_handle error = GRPC_ERROR_NONE; - absl::StatusOr> result = - ServiceConfigImpl::Create(args_, json.c_str(), &error); - if (!GRPC_ERROR_IS_NONE(error)) { - result = grpc_error_to_absl_status(error); - GRPC_ERROR_UNREF(error); - } - return result; + return ServiceConfigImpl::Create(args_, json.c_str()); } void XdsResolver::GenerateResult() { diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.cc b/src/core/ext/filters/client_channel/resolver_result_parsing.cc index 1e042424f7c..054ef0d4215 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.cc +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.cc @@ -33,6 +33,7 @@ #include #include "src/core/ext/filters/client_channel/lb_policy_registry.h" +#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json_util.h" // As per the retry design, we do not allow more than 5 retry attempts. @@ -80,11 +81,9 @@ absl::optional ParseHealthCheckConfig(const Json& field, } // namespace -std::unique_ptr +absl::StatusOr> ClientChannelServiceConfigParser::ParseGlobalParams(const ChannelArgs& /*args*/, - const Json& json, - grpc_error_handle* error) { - GPR_DEBUG_ASSERT(error != nullptr && GRPC_ERROR_IS_NONE(*error)); + const Json& json) { std::vector error_list; // Parse LB config. RefCountedPtr parsed_lb_config; @@ -135,20 +134,23 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const ChannelArgs& /*args*/, error_list.push_back(parsing_error); } } - *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel global parser", - &error_list); - if (GRPC_ERROR_IS_NONE(*error)) { - return absl::make_unique( - std::move(parsed_lb_config), std::move(lb_policy_name), - std::move(health_check_service_name)); + if (!error_list.empty()) { + grpc_error_handle error = GRPC_ERROR_CREATE_FROM_VECTOR( + "Client channel global parser", &error_list); + absl::Status status = absl::InvalidArgumentError( + absl::StrCat("error parsing client channel global parameters: ", + grpc_error_std_string(error))); + GRPC_ERROR_UNREF(error); + return status; } - return nullptr; + return absl::make_unique( + std::move(parsed_lb_config), std::move(lb_policy_name), + std::move(health_check_service_name)); } -std::unique_ptr +absl::StatusOr> ClientChannelServiceConfigParser::ParsePerMethodParams( - const ChannelArgs& /*args*/, const Json& json, grpc_error_handle* error) { - GPR_DEBUG_ASSERT(error != nullptr && GRPC_ERROR_IS_NONE(*error)); + const ChannelArgs& /*args*/, const Json& json) { std::vector error_list; // Parse waitForReady. absl::optional wait_for_ready; @@ -168,12 +170,17 @@ ClientChannelServiceConfigParser::ParsePerMethodParams( ParseJsonObjectFieldAsDuration(json.object_value(), "timeout", &timeout, &error_list, false); // Return result. - *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel parser", &error_list); - if (GRPC_ERROR_IS_NONE(*error)) { - return absl::make_unique(timeout, - wait_for_ready); + if (!error_list.empty()) { + grpc_error_handle error = + GRPC_ERROR_CREATE_FROM_VECTOR("Client channel parser", &error_list); + absl::Status status = absl::InvalidArgumentError( + absl::StrCat("error parsing client channel method parameters: ", + grpc_error_std_string(error))); + GRPC_ERROR_UNREF(error); + return status; } - return nullptr; + return absl::make_unique(timeout, + wait_for_ready); } } // namespace internal diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.h b/src/core/ext/filters/client_channel/resolver_result_parsing.h index 6ee7a072bf1..70dd7dad96b 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.h +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.h @@ -25,6 +25,7 @@ #include #include +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -33,7 +34,6 @@ #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/time.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json.h" #include "src/core/lib/service_config/service_config_parser.h" @@ -89,13 +89,11 @@ class ClientChannelServiceConfigParser : public ServiceConfigParser::Parser { public: absl::string_view name() const override { return parser_name(); } - std::unique_ptr ParseGlobalParams( - const ChannelArgs& /*args*/, const Json& json, - grpc_error_handle* error) override; + absl::StatusOr> + ParseGlobalParams(const ChannelArgs& /*args*/, const Json& json) override; - std::unique_ptr ParsePerMethodParams( - const ChannelArgs& /*args*/, const Json& json, - grpc_error_handle* error) override; + absl::StatusOr> + ParsePerMethodParams(const ChannelArgs& /*args*/, const Json& json) override; static size_t ParserIndex(); static void Register(CoreConfiguration::Builder* builder); diff --git a/src/core/ext/filters/client_channel/retry_service_config.cc b/src/core/ext/filters/client_channel/retry_service_config.cc index 52cfeb131ad..a0c7706591b 100644 --- a/src/core/ext/filters/client_channel/retry_service_config.cc +++ b/src/core/ext/filters/client_channel/retry_service_config.cc @@ -28,6 +28,8 @@ #include #include "absl/memory/memory.h" +#include "absl/status/status.h" +#include "absl/strings/str_cat.h" #include "absl/types/optional.h" #include @@ -38,6 +40,7 @@ #include "src/core/lib/channel/status_util.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gpr/string.h" +#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json_util.h" // As per the retry design, we do not allow more than 5 retry attempts. @@ -138,18 +141,22 @@ grpc_error_handle ParseRetryThrottling(const Json& json, } // namespace -std::unique_ptr +absl::StatusOr> RetryServiceConfigParser::ParseGlobalParams(const ChannelArgs& /*args*/, - const Json& json, - grpc_error_handle* error) { - GPR_DEBUG_ASSERT(error != nullptr && GRPC_ERROR_IS_NONE(*error)); + const Json& json) { auto it = json.object_value().find("retryThrottling"); if (it == json.object_value().end()) return nullptr; intptr_t max_milli_tokens = 0; intptr_t milli_token_ratio = 0; - *error = + grpc_error_handle error = ParseRetryThrottling(it->second, &max_milli_tokens, &milli_token_ratio); - if (!GRPC_ERROR_IS_NONE(*error)) return nullptr; + if (!GRPC_ERROR_IS_NONE(error)) { + absl::Status status = absl::InvalidArgumentError( + absl::StrCat("error parsing retry global parameters: ", + grpc_error_std_string(error))); + GRPC_ERROR_UNREF(error); + return status; + } return absl::make_unique(max_milli_tokens, milli_token_ratio); } @@ -286,11 +293,9 @@ grpc_error_handle ParseRetryPolicy( } // namespace -std::unique_ptr +absl::StatusOr> RetryServiceConfigParser::ParsePerMethodParams(const ChannelArgs& args, - const Json& json, - grpc_error_handle* error) { - GPR_DEBUG_ASSERT(error != nullptr && GRPC_ERROR_IS_NONE(*error)); + const Json& json) { // Parse retry policy. auto it = json.object_value().find("retryPolicy"); if (it == json.object_value().end()) return nullptr; @@ -300,10 +305,16 @@ RetryServiceConfigParser::ParsePerMethodParams(const ChannelArgs& args, float backoff_multiplier = 0; StatusCodeSet retryable_status_codes; absl::optional per_attempt_recv_timeout; - *error = ParseRetryPolicy(args, it->second, &max_attempts, &initial_backoff, - &max_backoff, &backoff_multiplier, - &retryable_status_codes, &per_attempt_recv_timeout); - if (!GRPC_ERROR_IS_NONE(*error)) return nullptr; + grpc_error_handle error = ParseRetryPolicy( + args, it->second, &max_attempts, &initial_backoff, &max_backoff, + &backoff_multiplier, &retryable_status_codes, &per_attempt_recv_timeout); + if (!GRPC_ERROR_IS_NONE(error)) { + absl::Status status = absl::InvalidArgumentError( + absl::StrCat("error parsing retry method parameters: ", + grpc_error_std_string(error))); + GRPC_ERROR_UNREF(error); + return status; + } return absl::make_unique( max_attempts, initial_backoff, max_backoff, backoff_multiplier, retryable_status_codes, per_attempt_recv_timeout); diff --git a/src/core/ext/filters/client_channel/retry_service_config.h b/src/core/ext/filters/client_channel/retry_service_config.h index e55e08126f6..6fdcdb2d055 100644 --- a/src/core/ext/filters/client_channel/retry_service_config.h +++ b/src/core/ext/filters/client_channel/retry_service_config.h @@ -24,6 +24,7 @@ #include +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -31,7 +32,6 @@ #include "src/core/lib/channel/status_util.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gprpp/time.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json.h" #include "src/core/lib/service_config/service_config_parser.h" @@ -89,13 +89,11 @@ class RetryServiceConfigParser : public ServiceConfigParser::Parser { public: absl::string_view name() const override { return parser_name(); } - std::unique_ptr ParseGlobalParams( - const ChannelArgs& /*args*/, const Json& json, - grpc_error_handle* error) override; + absl::StatusOr> + ParseGlobalParams(const ChannelArgs& /*args*/, const Json& json) override; - std::unique_ptr ParsePerMethodParams( - const ChannelArgs& args, const Json& json, - grpc_error_handle* error) override; + absl::StatusOr> + ParsePerMethodParams(const ChannelArgs& args, const Json& json) override; static size_t ParserIndex(); static void Register(CoreConfiguration::Builder* builder); diff --git a/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc b/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc index e1097fa56e7..cd5559c63c4 100644 --- a/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc +++ b/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc @@ -23,6 +23,8 @@ #include #include +#include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/types/optional.h" #include @@ -55,17 +57,13 @@ class ServiceConfigChannelArgChannelData { const char* service_config_str = grpc_channel_args_find_string( args->channel_args, GRPC_ARG_SERVICE_CONFIG); if (service_config_str != nullptr) { - grpc_error_handle service_config_error = GRPC_ERROR_NONE; - auto service_config = - ServiceConfigImpl::Create(ChannelArgs::FromC(args->channel_args), - service_config_str, &service_config_error); - if (GRPC_ERROR_IS_NONE(service_config_error)) { - service_config_ = std::move(service_config); + auto service_config = ServiceConfigImpl::Create( + ChannelArgs::FromC(args->channel_args), service_config_str); + if (!service_config.ok()) { + gpr_log(GPR_ERROR, "%s", service_config.status().ToString().c_str()); } else { - gpr_log(GPR_ERROR, "%s", - grpc_error_std_string(service_config_error).c_str()); + service_config_ = std::move(*service_config); } - GRPC_ERROR_UNREF(service_config_error); } } diff --git a/src/core/ext/filters/fault_injection/service_config_parser.cc b/src/core/ext/filters/fault_injection/service_config_parser.cc index 236ace269e7..0a5ce06bd2a 100644 --- a/src/core/ext/filters/fault_injection/service_config_parser.cc +++ b/src/core/ext/filters/fault_injection/service_config_parser.cc @@ -22,14 +22,14 @@ #include #include "absl/memory/memory.h" +#include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/types/optional.h" -#include - #include "src/core/ext/filters/fault_injection/fault_injection_filter.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/status_util.h" +#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json_util.h" namespace grpc_core { @@ -141,10 +141,9 @@ ParseFaultInjectionPolicy(const Json::Array& policies_json_array, } // namespace -std::unique_ptr -FaultInjectionServiceConfigParser::ParsePerMethodParams( - const ChannelArgs& args, const Json& json, grpc_error_handle* error) { - GPR_DEBUG_ASSERT(error != nullptr && GRPC_ERROR_IS_NONE(*error)); +absl::StatusOr> +FaultInjectionServiceConfigParser::ParsePerMethodParams(const ChannelArgs& args, + const Json& json) { // Only parse fault injection policy if the following channel arg is present. if (!args.GetBool(GRPC_ARG_PARSE_FAULT_INJECTION_METHOD_CONFIG) .value_or(false)) { @@ -160,10 +159,16 @@ FaultInjectionServiceConfigParser::ParsePerMethodParams( fault_injection_policies = ParseFaultInjectionPolicy(*policies_json_array, &error_list); } - *error = GRPC_ERROR_CREATE_FROM_VECTOR("Fault injection parser", &error_list); - if (!GRPC_ERROR_IS_NONE(*error) || fault_injection_policies.empty()) { - return nullptr; + if (!error_list.empty()) { + grpc_error_handle error = + GRPC_ERROR_CREATE_FROM_VECTOR("Fault injection parser", &error_list); + absl::Status status = absl::InvalidArgumentError( + absl::StrCat("error parsing fault injection method parameters: ", + grpc_error_std_string(error))); + GRPC_ERROR_UNREF(error); + return status; } + if (fault_injection_policies.empty()) return nullptr; return absl::make_unique( std::move(fault_injection_policies)); } diff --git a/src/core/ext/filters/fault_injection/service_config_parser.h b/src/core/ext/filters/fault_injection/service_config_parser.h index c36c76a5b49..af266e0f1fd 100644 --- a/src/core/ext/filters/fault_injection/service_config_parser.h +++ b/src/core/ext/filters/fault_injection/service_config_parser.h @@ -28,6 +28,7 @@ #include #include +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include @@ -35,7 +36,6 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gprpp/time.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json.h" #include "src/core/lib/service_config/service_config_parser.h" @@ -88,9 +88,8 @@ class FaultInjectionServiceConfigParser final public: absl::string_view name() const override { return parser_name(); } // Parses the per-method service config for fault injection filter. - std::unique_ptr ParsePerMethodParams( - const ChannelArgs& args, const Json& json, - grpc_error_handle* error) override; + absl::StatusOr> + ParsePerMethodParams(const ChannelArgs& args, const Json& json) override; // Returns the parser index for FaultInjectionServiceConfigParser. static size_t ParserIndex(); // Registers FaultInjectionServiceConfigParser to ServiceConfigParser. diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index dac5c477eda..0479a6b57c9 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -26,6 +26,8 @@ #include #include "absl/memory/memory.h" +#include "absl/status/status.h" +#include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/types/optional.h" @@ -41,6 +43,7 @@ #include "src/core/lib/gprpp/debug_location.h" #include "src/core/lib/iomgr/call_combiner.h" #include "src/core/lib/iomgr/closure.h" +#include "src/core/lib/iomgr/error.h" #include "src/core/lib/service_config/service_config_call_data.h" #include "src/core/lib/slice/slice_buffer.h" #include "src/core/lib/surface/channel_init.h" @@ -72,11 +75,9 @@ const MessageSizeParsedConfig* MessageSizeParsedConfig::GetFromCallContext( // MessageSizeParser // -std::unique_ptr +absl::StatusOr> MessageSizeParser::ParsePerMethodParams(const ChannelArgs& /*args*/, - const Json& json, - grpc_error_handle* error) { - GPR_DEBUG_ASSERT(error != nullptr && GRPC_ERROR_IS_NONE(*error)); + const Json& json) { std::vector error_list; // Max request size. int max_request_message_bytes = -1; @@ -113,8 +114,13 @@ MessageSizeParser::ParsePerMethodParams(const ChannelArgs& /*args*/, } } if (!error_list.empty()) { - *error = GRPC_ERROR_CREATE_FROM_VECTOR("Message size parser", &error_list); - return nullptr; + grpc_error_handle error = + GRPC_ERROR_CREATE_FROM_VECTOR("Message size parser", &error_list); + absl::Status status = absl::InvalidArgumentError( + absl::StrCat("error parsing message size method parameters: ", + grpc_error_std_string(error))); + GRPC_ERROR_UNREF(error); + return status; } return absl::make_unique(max_request_message_bytes, max_response_message_bytes); diff --git a/src/core/ext/filters/message_size/message_size_filter.h b/src/core/ext/filters/message_size/message_size_filter.h index 0af9437aa04..4493b2acc2c 100644 --- a/src/core/ext/filters/message_size/message_size_filter.h +++ b/src/core/ext/filters/message_size/message_size_filter.h @@ -23,6 +23,7 @@ #include +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "src/core/lib/channel/channel_args.h" @@ -30,7 +31,6 @@ #include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/channel/context.h" #include "src/core/lib/config/core_configuration.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json.h" #include "src/core/lib/service_config/service_config_parser.h" @@ -64,9 +64,8 @@ class MessageSizeParser : public ServiceConfigParser::Parser { public: absl::string_view name() const override { return parser_name(); } - std::unique_ptr ParsePerMethodParams( - const ChannelArgs& /*args*/, const Json& json, - grpc_error_handle* error) override; + absl::StatusOr> + ParsePerMethodParams(const ChannelArgs& /*args*/, const Json& json) override; static void Register(CoreConfiguration::Builder* builder); diff --git a/src/core/ext/filters/rbac/rbac_service_config_parser.cc b/src/core/ext/filters/rbac/rbac_service_config_parser.cc index 8ae2bf330f6..1e98d55260c 100644 --- a/src/core/ext/filters/rbac/rbac_service_config_parser.cc +++ b/src/core/ext/filters/rbac/rbac_service_config_parser.cc @@ -26,12 +26,12 @@ #include "absl/memory/memory.h" #include "absl/status/status.h" #include "absl/status/statusor.h" +#include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/types/optional.h" -#include - #include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json_util.h" #include "src/core/lib/matchers/matchers.h" #include "src/core/lib/transport/error_utils.h" @@ -581,11 +581,9 @@ std::vector ParseRbacArray(const Json::Array& policies_json_array, } // namespace -std::unique_ptr +absl::StatusOr> RbacServiceConfigParser::ParsePerMethodParams(const ChannelArgs& args, - const Json& json, - grpc_error_handle* error) { - GPR_DEBUG_ASSERT(error != nullptr && GRPC_ERROR_IS_NONE(*error)); + const Json& json) { // Only parse rbac policy if the channel arg is present if (!args.GetBool(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG).value_or(false)) { return nullptr; @@ -597,10 +595,16 @@ RbacServiceConfigParser::ParsePerMethodParams(const ChannelArgs& args, &policies_json_array, &error_list)) { rbac_policies = ParseRbacArray(*policies_json_array, &error_list); } - *error = GRPC_ERROR_CREATE_FROM_VECTOR("Rbac parser", &error_list); - if (!GRPC_ERROR_IS_NONE(*error) || rbac_policies.empty()) { - return nullptr; + grpc_error_handle error = + GRPC_ERROR_CREATE_FROM_VECTOR("Rbac parser", &error_list); + if (!GRPC_ERROR_IS_NONE(error)) { + absl::Status status = absl::InvalidArgumentError( + absl::StrCat("error parsing RBAC method parameters: ", + grpc_error_std_string(error))); + GRPC_ERROR_UNREF(error); + return status; } + if (rbac_policies.empty()) return nullptr; return absl::make_unique(std::move(rbac_policies)); } diff --git a/src/core/ext/filters/rbac/rbac_service_config_parser.h b/src/core/ext/filters/rbac/rbac_service_config_parser.h index b36795d6cd0..06f83827b7b 100644 --- a/src/core/ext/filters/rbac/rbac_service_config_parser.h +++ b/src/core/ext/filters/rbac/rbac_service_config_parser.h @@ -26,11 +26,11 @@ #include #include +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/config/core_configuration.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json.h" #include "src/core/lib/security/authorization/grpc_authorization_engine.h" #include "src/core/lib/security/authorization/rbac_policy.h" @@ -69,9 +69,8 @@ class RbacServiceConfigParser : public ServiceConfigParser::Parser { public: absl::string_view name() const override { return parser_name(); } // Parses the per-method service config for rbac filter. - std::unique_ptr ParsePerMethodParams( - const ChannelArgs& args, const Json& json, - grpc_error_handle* error) override; + absl::StatusOr> + ParsePerMethodParams(const ChannelArgs& args, const Json& json) override; // Returns the parser index for RbacServiceConfigParser. static size_t ParserIndex(); // Registers RbacServiceConfigParser to ServiceConfigParser. diff --git a/src/core/ext/xds/xds_server_config_fetcher.cc b/src/core/ext/xds/xds_server_config_fetcher.cc index c26b73271ca..925cb8da0b3 100644 --- a/src/core/ext/xds/xds_server_config_fetcher.cc +++ b/src/core/ext/xds/xds_server_config_fetcher.cc @@ -1169,10 +1169,8 @@ XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager:: absl::StrJoin(fields, ",\n"), "\n } ]\n" "}"); - grpc_error_handle error = GRPC_ERROR_NONE; config_selector_route.method_config = - ServiceConfigImpl::Create(result.args, json.c_str(), &error); - GPR_ASSERT(GRPC_ERROR_IS_NONE(error)); + ServiceConfigImpl::Create(result.args, json.c_str()).value(); } } } diff --git a/src/core/lib/service_config/service_config_impl.cc b/src/core/lib/service_config/service_config_impl.cc index 25315ffc09d..29fc2f332be 100644 --- a/src/core/lib/service_config/service_config_impl.cc +++ b/src/core/lib/service_config/service_config_impl.cc @@ -26,11 +26,13 @@ #include "absl/memory/memory.h" #include "absl/strings/str_cat.h" +#include "absl/strings/str_join.h" #include #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gprpp/memory.h" +#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json.h" #include "src/core/lib/service_config/service_config_parser.h" #include "src/core/lib/slice/slice_internal.h" @@ -38,37 +40,46 @@ namespace grpc_core { -RefCountedPtr ServiceConfigImpl::Create( - const ChannelArgs& args, absl::string_view json_string, - grpc_error_handle* error) { - GPR_DEBUG_ASSERT(error != nullptr); - Json json = Json::Parse(json_string, error); - if (!GRPC_ERROR_IS_NONE(*error)) return nullptr; - return MakeRefCounted(args, std::string(json_string), - std::move(json), error); +absl::StatusOr> ServiceConfigImpl::Create( + const ChannelArgs& args, absl::string_view json_string) { + grpc_error_handle error = GRPC_ERROR_NONE; + Json json = Json::Parse(json_string, &error); + if (!GRPC_ERROR_IS_NONE(error)) { + absl::Status status = + absl::InvalidArgumentError(grpc_error_std_string(error)); + GRPC_ERROR_UNREF(error); + return status; + } + absl::Status status; + auto service_config = MakeRefCounted( + args, std::string(json_string), std::move(json), &status); + if (!status.ok()) return status; + return service_config; } ServiceConfigImpl::ServiceConfigImpl(const ChannelArgs& args, std::string json_string, Json json, - grpc_error_handle* error) + absl::Status* status) : json_string_(std::move(json_string)), json_(std::move(json)) { - GPR_DEBUG_ASSERT(error != nullptr); + GPR_DEBUG_ASSERT(status != nullptr); if (json_.type() != Json::Type::OBJECT) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON value is not an object"); + *status = absl::InvalidArgumentError("JSON value is not an object"); return; } - std::vector error_list; - grpc_error_handle global_error = GRPC_ERROR_NONE; - parsed_global_configs_ = + std::vector errors; + auto parsed_global_configs = CoreConfiguration::Get().service_config_parser().ParseGlobalParameters( - args, json_, &global_error); - if (!GRPC_ERROR_IS_NONE(global_error)) error_list.push_back(global_error); - grpc_error_handle local_error = ParsePerMethodParams(args); - if (!GRPC_ERROR_IS_NONE(local_error)) error_list.push_back(local_error); - if (!error_list.empty()) { - *error = GRPC_ERROR_CREATE_FROM_VECTOR("Service config parsing error", - &error_list); + args, json_); + if (!parsed_global_configs.ok()) { + errors.emplace_back(parsed_global_configs.status().message()); + } else { + parsed_global_configs_ = std::move(*parsed_global_configs); + } + absl::Status local_status = ParsePerMethodParams(args); + if (!local_status.ok()) errors.emplace_back(local_status.message()); + if (!errors.empty()) { + *status = absl::InvalidArgumentError(absl::StrCat( + "Service config parsing errors: [", absl::StrJoin(errors, "; "), "]")); } } @@ -78,98 +89,95 @@ ServiceConfigImpl::~ServiceConfigImpl() { } } -grpc_error_handle ServiceConfigImpl::ParseJsonMethodConfig( - const ChannelArgs& args, const Json& json) { - std::vector error_list; +absl::Status ServiceConfigImpl::ParseJsonMethodConfig(const ChannelArgs& args, + const Json& json, + size_t index) { + std::vector errors; + const ServiceConfigParser::ParsedConfigVector* vector_ptr = nullptr; // Parse method config with each registered parser. - auto parsed_configs = - absl::make_unique(); - grpc_error_handle parser_error = GRPC_ERROR_NONE; - *parsed_configs = + auto parsed_configs_or = CoreConfiguration::Get().service_config_parser().ParsePerMethodParameters( - args, json, &parser_error); - if (!GRPC_ERROR_IS_NONE(parser_error)) { - error_list.push_back(parser_error); + args, json); + if (!parsed_configs_or.ok()) { + errors.emplace_back(parsed_configs_or.status().message()); + } else { + auto parsed_configs = + absl::make_unique( + std::move(*parsed_configs_or)); + parsed_method_config_vectors_storage_.push_back(std::move(parsed_configs)); + vector_ptr = parsed_method_config_vectors_storage_.back().get(); } - parsed_method_config_vectors_storage_.push_back(std::move(parsed_configs)); - const auto* vector_ptr = parsed_method_config_vectors_storage_.back().get(); // Add an entry for each path. - bool found_name = false; auto it = json.object_value().find("name"); if (it != json.object_value().end()) { if (it->second.type() != Json::Type::ARRAY) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error:not of type Array")); - return GRPC_ERROR_CREATE_FROM_VECTOR("methodConfig", &error_list); - } - const Json::Array& name_array = it->second.array_value(); - for (const Json& name : name_array) { - grpc_error_handle parse_error = GRPC_ERROR_NONE; - std::string path = ParseJsonMethodName(name, &parse_error); - if (!GRPC_ERROR_IS_NONE(parse_error)) { - error_list.push_back(parse_error); - } else { - found_name = true; - if (path.empty()) { - if (default_method_config_vector_ != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error:multiple default method configs")); - } - default_method_config_vector_ = vector_ptr; + errors.emplace_back("field:name error:not of type Array"); + } else { + const Json::Array& name_array = it->second.array_value(); + for (const Json& name : name_array) { + absl::StatusOr path = ParseJsonMethodName(name); + if (!path.ok()) { + errors.emplace_back(path.status().message()); } else { - grpc_slice key = grpc_slice_from_copied_string(path.c_str()); - // If the key is not already present in the map, this will - // store a ref to the key in the map. - auto& value = parsed_method_configs_map_[key]; - if (value != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error:multiple method configs with same name")); - // The map entry already existed, so we need to unref the - // key we just created. - grpc_slice_unref_internal(key); + if (path->empty()) { + if (default_method_config_vector_ != nullptr) { + errors.emplace_back( + "field:name error:multiple default method configs"); + } + default_method_config_vector_ = vector_ptr; } else { - value = vector_ptr; + grpc_slice key = grpc_slice_from_cpp_string(std::move(*path)); + // If the key is not already present in the map, this will + // store a ref to the key in the map. + auto& value = parsed_method_configs_map_[key]; + if (value != nullptr) { + errors.emplace_back( + "field:name error:multiple method configs with same name"); + // The map entry already existed, so we need to unref the + // key we just created. + grpc_slice_unref_internal(key); + } else { + value = vector_ptr; + } } } } } } - if (!found_name) { - parsed_method_config_vectors_storage_.pop_back(); + if (!errors.empty()) { + return absl::InvalidArgumentError( + absl::StrCat("index ", index, ": [", absl::StrJoin(errors, "; "), "]")); } - return GRPC_ERROR_CREATE_FROM_VECTOR("methodConfig", &error_list); + return absl::OkStatus(); } -grpc_error_handle ServiceConfigImpl::ParsePerMethodParams( - const ChannelArgs& args) { - std::vector error_list; +absl::Status ServiceConfigImpl::ParsePerMethodParams(const ChannelArgs& args) { auto it = json_.object_value().find("methodConfig"); - if (it != json_.object_value().end()) { - if (it->second.type() != Json::Type::ARRAY) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:methodConfig error:not of type Array")); - } - for (const Json& method_config : it->second.array_value()) { - if (method_config.type() != Json::Type::OBJECT) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:methodConfig error:not of type Object")); - continue; - } - grpc_error_handle error = ParseJsonMethodConfig(args, method_config); - if (!GRPC_ERROR_IS_NONE(error)) { - error_list.push_back(error); - } + if (it == json_.object_value().end()) return absl::OkStatus(); + if (it->second.type() != Json::Type::ARRAY) { + return absl::InvalidArgumentError("field must be of type array"); + } + std::vector errors; + for (size_t i = 0; i < it->second.array_value().size(); ++i) { + const Json& method_config = it->second.array_value()[i]; + if (method_config.type() != Json::Type::OBJECT) { + errors.emplace_back(absl::StrCat("index ", i, ": not of type Object")); + } else { + absl::Status status = ParseJsonMethodConfig(args, method_config, i); + if (!status.ok()) errors.emplace_back(status.message()); } } - return GRPC_ERROR_CREATE_FROM_VECTOR("Method Params", &error_list); + if (!errors.empty()) { + return absl::InvalidArgumentError(absl::StrCat( + "errors parsing methodConfig: [", absl::StrJoin(errors, "; "), "]")); + } + return absl::OkStatus(); } -std::string ServiceConfigImpl::ParseJsonMethodName(const Json& json, - grpc_error_handle* error) { +absl::StatusOr ServiceConfigImpl::ParseJsonMethodName( + const Json& json) { if (json.type() != Json::Type::OBJECT) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "field:name error:type is not object"); - return ""; + return absl::InvalidArgumentError("field:name error:type is not object"); } // Find service name. const std::string* service_name = nullptr; @@ -177,9 +185,8 @@ std::string ServiceConfigImpl::ParseJsonMethodName(const Json& json, if (it != json.object_value().end() && it->second.type() != Json::Type::JSON_NULL) { if (it->second.type() != Json::Type::STRING) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( + return absl::InvalidArgumentError( "field:name error: field:service error:not of type string"); - return ""; } if (!it->second.string_value().empty()) { service_name = &it->second.string_value(); @@ -191,9 +198,8 @@ std::string ServiceConfigImpl::ParseJsonMethodName(const Json& json, if (it != json.object_value().end() && it->second.type() != Json::Type::JSON_NULL) { if (it->second.type() != Json::Type::STRING) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( + return absl::InvalidArgumentError( "field:name error: field:method error:not of type string"); - return ""; } if (!it->second.string_value().empty()) { method_name = &it->second.string_value(); @@ -203,7 +209,7 @@ std::string ServiceConfigImpl::ParseJsonMethodName(const Json& json, // Method name may not be specified without service name. if (service_name == nullptr) { if (method_name != nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( + return absl::InvalidArgumentError( "field:name error:method name populated without service name"); } return ""; diff --git a/src/core/lib/service_config/service_config_impl.h b/src/core/lib/service_config/service_config_impl.h index de10d160c20..6aa6f8bbc95 100644 --- a/src/core/lib/service_config/service_config_impl.h +++ b/src/core/lib/service_config/service_config_impl.h @@ -26,6 +26,8 @@ #include #include +#include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include @@ -33,7 +35,6 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json.h" #include "src/core/lib/service_config/service_config.h" #include "src/core/lib/service_config/service_config_parser.h" @@ -68,13 +69,11 @@ namespace grpc_core { class ServiceConfigImpl final : public ServiceConfig { public: /// Creates a new service config from parsing \a json_string. - /// Returns null on parse error. - static RefCountedPtr Create(const ChannelArgs& args, - absl::string_view json_string, - grpc_error_handle* error); + static absl::StatusOr> Create( + const ChannelArgs& args, absl::string_view json_string); ServiceConfigImpl(const ChannelArgs& args, std::string json_string, Json json, - grpc_error_handle* error); + absl::Status* status); ~ServiceConfigImpl() override; absl::string_view json_string() const override { return json_string_; } @@ -96,14 +95,13 @@ class ServiceConfigImpl final : public ServiceConfig { private: // Helper functions for parsing the method configs. - grpc_error_handle ParsePerMethodParams(const ChannelArgs& args); - grpc_error_handle ParseJsonMethodConfig(const ChannelArgs& args, - const Json& json); + absl::Status ParsePerMethodParams(const ChannelArgs& args); + absl::Status ParseJsonMethodConfig(const ChannelArgs& args, const Json& json, + size_t index); // Returns a path string for the JSON name object specified by json. // Sets *error on error. - static std::string ParseJsonMethodName(const Json& json, - grpc_error_handle* error); + static absl::StatusOr ParseJsonMethodName(const Json& json); std::string json_string_; Json json_; diff --git a/src/core/lib/service_config/service_config_parser.cc b/src/core/lib/service_config/service_config_parser.cc index 50688c05da1..0ec4be9fd79 100644 --- a/src/core/lib/service_config/service_config_parser.cc +++ b/src/core/lib/service_config/service_config_parser.cc @@ -22,7 +22,9 @@ #include +#include "absl/status/status.h" #include "absl/strings/str_cat.h" +#include "absl/strings/str_join.h" #include @@ -47,46 +49,43 @@ void ServiceConfigParser::Builder::RegisterParser( registered_parsers_.emplace_back(std::move(parser)); } -ServiceConfigParser::ParsedConfigVector +absl::StatusOr ServiceConfigParser::ParseGlobalParameters(const ChannelArgs& args, - const Json& json, - grpc_error_handle* error) const { + const Json& json) const { ParsedConfigVector parsed_global_configs; - std::vector error_list; + std::vector errors; for (size_t i = 0; i < registered_parsers_.size(); i++) { - grpc_error_handle parser_error = GRPC_ERROR_NONE; - auto parsed_config = - registered_parsers_[i]->ParseGlobalParams(args, json, &parser_error); - if (!GRPC_ERROR_IS_NONE(parser_error)) { - error_list.push_back(parser_error); + auto parsed_config = registered_parsers_[i]->ParseGlobalParams(args, json); + if (!parsed_config.ok()) { + errors.emplace_back(parsed_config.status().message()); + } else { + parsed_global_configs.push_back(std::move(*parsed_config)); } - parsed_global_configs.push_back(std::move(parsed_config)); } - if (!error_list.empty()) { - *error = GRPC_ERROR_CREATE_FROM_VECTOR("Global Params", &error_list); + if (!errors.empty()) { + return absl::InvalidArgumentError(absl::StrJoin(errors, "; ")); } - return parsed_global_configs; + return std::move(parsed_global_configs); } -ServiceConfigParser::ParsedConfigVector +absl::StatusOr ServiceConfigParser::ParsePerMethodParameters(const ChannelArgs& args, - const Json& json, - grpc_error_handle* error) const { + const Json& json) const { ParsedConfigVector parsed_method_configs; - std::vector error_list; + std::vector errors; for (size_t i = 0; i < registered_parsers_.size(); ++i) { - grpc_error_handle parser_error = GRPC_ERROR_NONE; auto parsed_config = - registered_parsers_[i]->ParsePerMethodParams(args, json, &parser_error); - if (!GRPC_ERROR_IS_NONE(parser_error)) { - error_list.push_back(parser_error); + registered_parsers_[i]->ParsePerMethodParams(args, json); + if (!parsed_config.ok()) { + errors.emplace_back(parsed_config.status().message()); + } else { + parsed_method_configs.push_back(std::move(*parsed_config)); } - parsed_method_configs.push_back(std::move(parsed_config)); } - if (!error_list.empty()) { - *error = GRPC_ERROR_CREATE_FROM_VECTOR("methodConfig", &error_list); + if (!errors.empty()) { + return absl::InvalidArgumentError(absl::StrJoin(errors, "; ")); } - return parsed_method_configs; + return std::move(parsed_method_configs); } size_t ServiceConfigParser::GetParserIndex(absl::string_view name) const { diff --git a/src/core/lib/service_config/service_config_parser.h b/src/core/lib/service_config/service_config_parser.h index a9a4361f6d6..600b1624028 100644 --- a/src/core/lib/service_config/service_config_parser.h +++ b/src/core/lib/service_config/service_config_parser.h @@ -26,12 +26,10 @@ #include #include +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" -#include - #include "src/core/lib/channel/channel_args.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/json/json.h" namespace grpc_core { @@ -54,19 +52,13 @@ class ServiceConfigParser { virtual absl::string_view name() const = 0; - virtual std::unique_ptr ParseGlobalParams( - const ChannelArgs&, const Json& /* json */, grpc_error_handle* error) { - // Avoid unused parameter warning on debug-only parameter - (void)error; - GPR_DEBUG_ASSERT(error != nullptr); + virtual absl::StatusOr> ParseGlobalParams( + const ChannelArgs& /*args*/, const Json& /*json*/) { return nullptr; } - virtual std::unique_ptr ParsePerMethodParams( - const ChannelArgs&, const Json& /* json */, grpc_error_handle* error) { - // Avoid unused parameter warning on debug-only parameter - (void)error; - GPR_DEBUG_ASSERT(error != nullptr); + virtual absl::StatusOr> ParsePerMethodParams( + const ChannelArgs& /*args*/, const Json& /*json*/) { return nullptr; } }; @@ -88,13 +80,11 @@ class ServiceConfigParser { ServiceConfigParserList registered_parsers_; }; - ParsedConfigVector ParseGlobalParameters(const ChannelArgs& args, - const Json& json, - grpc_error_handle* error) const; + absl::StatusOr ParseGlobalParameters( + const ChannelArgs& args, const Json& json) const; - ParsedConfigVector ParsePerMethodParameters(const ChannelArgs& args, - const Json& json, - grpc_error_handle* error) const; + absl::StatusOr ParsePerMethodParameters( + const ChannelArgs& args, const Json& json) const; // Return the index for a given registered parser. // If there is an error, return -1. diff --git a/src/cpp/common/validate_service_config.cc b/src/cpp/common/validate_service_config.cc index 362436aac53..a5a1a77b269 100644 --- a/src/cpp/common/validate_service_config.cc +++ b/src/cpp/common/validate_service_config.cc @@ -18,26 +18,24 @@ #include +#include "absl/status/status.h" +#include "absl/status/statusor.h" + #include #include #include #include "src/core/lib/channel/channel_args.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/service_config/service_config_impl.h" namespace grpc { namespace experimental { std::string ValidateServiceConfigJSON(const std::string& service_config_json) { grpc_init(); - grpc_error_handle error = GRPC_ERROR_NONE; - grpc_core::ServiceConfigImpl::Create(grpc_core::ChannelArgs(), - service_config_json.c_str(), &error); + auto service_config = grpc_core::ServiceConfigImpl::Create( + grpc_core::ChannelArgs(), service_config_json.c_str()); std::string return_value; - if (!GRPC_ERROR_IS_NONE(error)) { - return_value = grpc_error_std_string(error); - GRPC_ERROR_UNREF(error); - } + if (!service_config.ok()) return_value = service_config.status().ToString(); grpc_shutdown(); return return_value; } diff --git a/test/core/client_channel/rls_lb_config_parser_test.cc b/test/core/client_channel/rls_lb_config_parser_test.cc index a4b4645d34a..fe1e5aac561 100644 --- a/test/core/client_channel/rls_lb_config_parser_test.cc +++ b/test/core/client_channel/rls_lb_config_parser_test.cc @@ -63,11 +63,10 @@ TEST_F(RlsConfigParsingTest, ValidConfig) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); - EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - EXPECT_NE(service_config, nullptr); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + EXPECT_NE(*service_config, nullptr); } // @@ -82,17 +81,17 @@ TEST_F(RlsConfigParsingTest, TopLevelRequiredFieldsMissing) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig error:does not exist.*" "field:childPolicyConfigTargetFieldName error:does not exist.*" - "field:childPolicy error:does not exist")); - GRPC_ERROR_UNREF(error); + "field:childPolicy error:does not exist")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, TopLevelFieldsWrongTypes) { @@ -107,18 +106,18 @@ TEST_F(RlsConfigParsingTest, TopLevelFieldsWrongTypes) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig error:type should be OBJECT.*" "field:routeLookupChannelServiceConfig error:type should be OBJECT.*" "field:childPolicyConfigTargetFieldName error:type should be STRING.*" - "field:childPolicy error:type should be ARRAY")); - GRPC_ERROR_UNREF(error); + "field:childPolicy error:type should be ARRAY")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, TopLevelFieldsInvalidValues) { @@ -133,17 +132,17 @@ TEST_F(RlsConfigParsingTest, TopLevelFieldsInvalidValues) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:childPolicyConfigTargetFieldName error:must be non-empty.*" "field:childPolicy" CHILD_ERROR_TAG - "No known policies in list: unknown")); - GRPC_ERROR_UNREF(error); + "No known policies in list: unknown")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, InvalidChildPolicyConfig) { @@ -158,16 +157,16 @@ TEST_F(RlsConfigParsingTest, InvalidChildPolicyConfig) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:childPolicy" CHILD_ERROR_TAG "errors parsing grpclb LB policy config: \\[" - "error parsing childPolicy field: type should be array\\]")); - GRPC_ERROR_UNREF(error); + "error parsing childPolicy field: type should be array\\]")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, InvalidRlsChannelServiceConfig) { @@ -185,18 +184,18 @@ TEST_F(RlsConfigParsingTest, InvalidRlsChannelServiceConfig) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "errors parsing RLS LB policy config" CHILD_ERROR_TAG - "field:routeLookupChannelServiceConfig" CHILD_ERROR_TAG - "Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG - "Client channel global parser" CHILD_ERROR_TAG - "field:loadBalancingPolicy error:Unknown lb policy")); - GRPC_ERROR_UNREF(error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + std::string(service_config.status().message()), + ::testing::ContainsRegex( + "errors parsing RLS LB policy config" CHILD_ERROR_TAG + "field:routeLookupChannelServiceConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "error parsing client channel global parameters" CHILD_ERROR_TAG + "field:loadBalancingPolicy error:Unknown lb policy")) + << service_config.status(); } // @@ -213,16 +212,16 @@ TEST_F(RlsConfigParsingTest, RouteLookupConfigRequiredFieldsMissing) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig" CHILD_ERROR_TAG "field:grpcKeybuilders error:does not exist.*" - "field:lookupService error:does not exist")); - GRPC_ERROR_UNREF(error); + "field:lookupService error:does not exist")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, RouteLookupConfigFieldsWrongTypes) { @@ -243,10 +242,10 @@ TEST_F(RlsConfigParsingTest, RouteLookupConfigFieldsWrongTypes) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig" CHILD_ERROR_TAG @@ -255,8 +254,8 @@ TEST_F(RlsConfigParsingTest, RouteLookupConfigFieldsWrongTypes) { "field:maxAge error:type should be STRING.*" "field:staleAge error:type should be STRING.*" "field:cacheSizeBytes error:failed to parse.*" - "field:defaultTarget error:type should be STRING")); - GRPC_ERROR_UNREF(error); + "field:defaultTarget error:type should be STRING")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, RouteLookupConfigFieldsInvalidValues) { @@ -271,16 +270,16 @@ TEST_F(RlsConfigParsingTest, RouteLookupConfigFieldsInvalidValues) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig" CHILD_ERROR_TAG "field:lookupService error:must be valid gRPC target URI.*" - "field:cacheSizeBytes error:must be greater than 0")); - GRPC_ERROR_UNREF(error); + "field:cacheSizeBytes error:must be greater than 0")) + << service_config.status(); } // @@ -301,17 +300,16 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderRequiredFieldsMissing) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); - EXPECT_THAT( - grpc_error_std_string(error), - ::testing::ContainsRegex( - "errors parsing RLS LB policy config" CHILD_ERROR_TAG - "field:routeLookupConfig" CHILD_ERROR_TAG - "field:grpcKeybuilders" CHILD_ERROR_TAG "index:0" CHILD_ERROR_TAG - "field:names error:does not exist")); - GRPC_ERROR_UNREF(error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), + ::testing::ContainsRegex( + "errors parsing RLS LB policy config" CHILD_ERROR_TAG + "field:routeLookupConfig" CHILD_ERROR_TAG + "field:grpcKeybuilders" CHILD_ERROR_TAG + "index:0" CHILD_ERROR_TAG "field:names error:does not exist")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, GrpcKeybuilderWrongFieldTypes) { @@ -332,11 +330,11 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderWrongFieldTypes) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig" CHILD_ERROR_TAG @@ -344,8 +342,8 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderWrongFieldTypes) { "field:names error:type should be ARRAY.*" "field:headers error:type should be ARRAY.*" "field:extraKeys error:type should be OBJECT.*" - "field:constantKeys error:type should be OBJECT")); - GRPC_ERROR_UNREF(error); + "field:constantKeys error:type should be OBJECT")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, GrpcKeybuilderInvalidValues) { @@ -371,10 +369,10 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderInvalidValues) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig" CHILD_ERROR_TAG @@ -385,8 +383,8 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderInvalidValues) { "field:service error:type should be STRING.*" "field:method error:type should be STRING.*" "field:constantKeys" CHILD_ERROR_TAG - "field:key error:type should be STRING")); - GRPC_ERROR_UNREF(error); + "field:key error:type should be STRING")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, GrpcKeybuilderInvalidHeaders) { @@ -423,11 +421,11 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderInvalidHeaders) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig" CHILD_ERROR_TAG @@ -445,8 +443,8 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderInvalidHeaders) { "field:names index:1 error:header name must be non-empty.*" "field:extraKeys" CHILD_ERROR_TAG "field:host error:must be non-empty.*" - "field:constantKeys" CHILD_ERROR_TAG "error:keys must be non-empty")); - GRPC_ERROR_UNREF(error); + "field:constantKeys" CHILD_ERROR_TAG "error:keys must be non-empty")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, GrpcKeybuilderNameWrongFieldTypes) { @@ -470,11 +468,11 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderNameWrongFieldTypes) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig" CHILD_ERROR_TAG @@ -482,8 +480,8 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderNameWrongFieldTypes) { "field:names index:0 error:type should be OBJECT.*" "field:names index:1" CHILD_ERROR_TAG "field:service error:type should be STRING.*" - "field:method error:type should be STRING")); - GRPC_ERROR_UNREF(error); + "field:method error:type should be STRING")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, DuplicateMethodNamesInSameKeyBuilder) { @@ -510,17 +508,17 @@ TEST_F(RlsConfigParsingTest, DuplicateMethodNamesInSameKeyBuilder) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig" CHILD_ERROR_TAG "field:grpcKeybuilders" CHILD_ERROR_TAG "index:0" CHILD_ERROR_TAG - "field:names error:duplicate entry for /foo/bar")); - GRPC_ERROR_UNREF(error); + "field:names error:duplicate entry for /foo/bar")) + << service_config.status(); } TEST_F(RlsConfigParsingTest, DuplicateMethodNamesInDifferentKeyBuilders) { @@ -551,17 +549,17 @@ TEST_F(RlsConfigParsingTest, DuplicateMethodNamesInDifferentKeyBuilders) { " }\n" " }]\n" "}\n"; - grpc_error_handle error = GRPC_ERROR_NONE; auto service_config = - ServiceConfigImpl::Create(ChannelArgs(), service_config_json, &error); + ServiceConfigImpl::Create(ChannelArgs(), service_config_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "errors parsing RLS LB policy config" CHILD_ERROR_TAG "field:routeLookupConfig" CHILD_ERROR_TAG "field:grpcKeybuilders" CHILD_ERROR_TAG "index:1" CHILD_ERROR_TAG - "field:names error:duplicate entry for /foo/bar")); - GRPC_ERROR_UNREF(error); + "field:names error:duplicate entry for /foo/bar")) + << service_config.status(); } } // namespace diff --git a/test/core/client_channel/service_config_test.cc b/test/core/client_channel/service_config_test.cc index c630113f618..d51153376e7 100644 --- a/test/core/client_channel/service_config_test.cc +++ b/test/core/client_channel/service_config_test.cc @@ -63,25 +63,19 @@ class TestParser1 : public ServiceConfigParser::Parser { public: absl::string_view name() const override { return "test_parser_1"; } - std::unique_ptr ParseGlobalParams( - const ChannelArgs& args, const Json& json, - grpc_error_handle* error) override { - GPR_DEBUG_ASSERT(error != nullptr); + absl::StatusOr> + ParseGlobalParams(const ChannelArgs& args, const Json& json) override { if (args.GetBool(GRPC_ARG_DISABLE_PARSING).value_or(false)) { return nullptr; } auto it = json.object_value().find("global_param"); if (it != json.object_value().end()) { if (it->second.type() != Json::Type::NUMBER) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidTypeErrorMessage()); - return nullptr; + return absl::InvalidArgumentError(InvalidTypeErrorMessage()); } int value = gpr_parse_nonnegative_int(it->second.string_value().c_str()); if (value == -1) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); - return nullptr; + return absl::InvalidArgumentError(InvalidValueErrorMessage()); } return absl::make_unique(value); } @@ -101,25 +95,19 @@ class TestParser2 : public ServiceConfigParser::Parser { public: absl::string_view name() const override { return "test_parser_2"; } - std::unique_ptr ParsePerMethodParams( - const ChannelArgs& args, const Json& json, - grpc_error_handle* error) override { - GPR_DEBUG_ASSERT(error != nullptr); + absl::StatusOr> + ParsePerMethodParams(const ChannelArgs& args, const Json& json) override { if (args.GetBool(GRPC_ARG_DISABLE_PARSING).value_or(false)) { return nullptr; } auto it = json.object_value().find("method_param"); if (it != json.object_value().end()) { if (it->second.type() != Json::Type::NUMBER) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidTypeErrorMessage()); - return nullptr; + return absl::InvalidArgumentError(InvalidTypeErrorMessage()); } int value = gpr_parse_nonnegative_int(it->second.string_value().c_str()); if (value == -1) { - *error = - GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); - return nullptr; + return absl::InvalidArgumentError(InvalidValueErrorMessage()); } return absl::make_unique(value); } @@ -142,20 +130,15 @@ class ErrorParser : public ServiceConfigParser::Parser { absl::string_view name() const override { return name_; } - std::unique_ptr ParsePerMethodParams( - const ChannelArgs& /*arg*/, const Json& /*json*/, - grpc_error_handle* error) override { - GPR_DEBUG_ASSERT(error != nullptr); - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(MethodError()); - return nullptr; + absl::StatusOr> + ParsePerMethodParams(const ChannelArgs& /*arg*/, + const Json& /*json*/) override { + return absl::InvalidArgumentError(MethodError()); } - std::unique_ptr ParseGlobalParams( - const ChannelArgs& /*arg*/, const Json& /*json*/, - grpc_error_handle* error) override { - GPR_DEBUG_ASSERT(error != nullptr); - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(GlobalError()); - return nullptr; + absl::StatusOr> + ParseGlobalParams(const ChannelArgs& /*arg*/, const Json& /*json*/) override { + return absl::InvalidArgumentError(GlobalError()); } static const char* MethodError() { return "ErrorParser : methodError"; } @@ -187,19 +170,15 @@ class ServiceConfigTest : public ::testing::Test { }; TEST_F(ServiceConfigTest, ErrorCheck1) { - const char* test_json = ""; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), ""); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex("JSON parse error")); - GRPC_ERROR_UNREF(error); } TEST_F(ServiceConfigTest, BasicTest1) { - const char* test_json = "{}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), "{}"); + ASSERT_TRUE(service_config.ok()) << service_config.status(); } TEST_F(ServiceConfigTest, SkipMethodConfigWithNoNameOrEmptyName) { @@ -209,12 +188,13 @@ TEST_F(ServiceConfigTest, SkipMethodConfigWithNoNameOrEmptyName) { " {\"name\":[], \"method_param\":1}," " {\"name\":[{\"service\":\"TestServ\"}], \"method_param\":2}" "]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); - ASSERT_NE(vector_ptr, nullptr); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + auto vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); + ASSERT_EQ(vector_ptr->size(), 2UL); auto parsed_config = ((*vector_ptr)[1]).get(); EXPECT_EQ(static_cast(parsed_config)->value(), 2); } @@ -225,14 +205,13 @@ TEST_F(ServiceConfigTest, ErrorDuplicateMethodConfigNames) { " {\"name\":[{\"service\":\"TestServ\"}]}," " {\"name\":[{\"service\":\"TestServ\"}]}" "]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG - "multiple method configs with same name")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + "Service config parsing errors: [" + "errors parsing methodConfig: [" + "index 1: [" + "field:name error:multiple method configs with same name]]]"); } TEST_F(ServiceConfigTest, ErrorDuplicateMethodConfigNamesWithNullMethod) { @@ -241,14 +220,13 @@ TEST_F(ServiceConfigTest, ErrorDuplicateMethodConfigNamesWithNullMethod) { " {\"name\":[{\"service\":\"TestServ\",\"method\":null}]}," " {\"name\":[{\"service\":\"TestServ\"}]}" "]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG - "multiple method configs with same name")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + "Service config parsing errors: [" + "errors parsing methodConfig: [" + "index 1: [" + "field:name error:multiple method configs with same name]]]"); } TEST_F(ServiceConfigTest, ErrorDuplicateMethodConfigNamesWithEmptyMethod) { @@ -257,14 +235,13 @@ TEST_F(ServiceConfigTest, ErrorDuplicateMethodConfigNamesWithEmptyMethod) { " {\"name\":[{\"service\":\"TestServ\",\"method\":\"\"}]}," " {\"name\":[{\"service\":\"TestServ\"}]}" "]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG - "multiple method configs with same name")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + "Service config parsing errors: [" + "errors parsing methodConfig: [" + "index 1: [" + "field:name error:multiple method configs with same name]]]"); } TEST_F(ServiceConfigTest, ErrorDuplicateDefaultMethodConfigs) { @@ -273,14 +250,13 @@ TEST_F(ServiceConfigTest, ErrorDuplicateDefaultMethodConfigs) { " {\"name\":[{}]}," " {\"name\":[{}]}" "]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG - "multiple default method configs")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + "Service config parsing errors: [" + "errors parsing methodConfig: [" + "index 1: [" + "field:name error:multiple default method configs]]]"); } TEST_F(ServiceConfigTest, ErrorDuplicateDefaultMethodConfigsWithNullService) { @@ -289,14 +265,13 @@ TEST_F(ServiceConfigTest, ErrorDuplicateDefaultMethodConfigsWithNullService) { " {\"name\":[{\"service\":null}]}," " {\"name\":[{}]}" "]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG - "multiple default method configs")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + "Service config parsing errors: [" + "errors parsing methodConfig: [" + "index 1: [" + "field:name error:multiple default method configs]]]"); } TEST_F(ServiceConfigTest, ErrorDuplicateDefaultMethodConfigsWithEmptyService) { @@ -305,110 +280,98 @@ TEST_F(ServiceConfigTest, ErrorDuplicateDefaultMethodConfigsWithEmptyService) { " {\"name\":[{\"service\":\"\"}]}," " {\"name\":[{}]}" "]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG - "multiple default method configs")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + "Service config parsing errors: [" + "errors parsing methodConfig: [" + "index 1: [" + "field:name error:multiple default method configs]]]"); } TEST_F(ServiceConfigTest, ValidMethodConfig) { const char* test_json = "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}]}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); } TEST_F(ServiceConfigTest, Parser1BasicTest1) { const char* test_json = "{\"global_param\":5}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - EXPECT_EQ((static_cast(svc_cfg->GetGlobalParsedConfig(0))) + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + EXPECT_EQ((static_cast( + (*service_config)->GetGlobalParsedConfig(0))) ->value(), 5); - EXPECT_EQ(svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")), + EXPECT_EQ((*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")), nullptr); } TEST_F(ServiceConfigTest, Parser1BasicTest2) { const char* test_json = "{\"global_param\":1000}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - EXPECT_EQ((static_cast(svc_cfg->GetGlobalParsedConfig(0))) + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + EXPECT_EQ((static_cast( + (*service_config)->GetGlobalParsedConfig(0))) ->value(), 1000); } TEST_F(ServiceConfigTest, Parser1DisabledViaChannelArg) { - grpc_arg arg = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_DISABLE_PARSING), 1); - grpc_channel_args args = {1, &arg}; + const ChannelArgs args = ChannelArgs().Set(GRPC_ARG_DISABLE_PARSING, 1); const char* test_json = "{\"global_param\":5}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = - ServiceConfigImpl::Create(ChannelArgs::FromC(&args), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - EXPECT_EQ(svc_cfg->GetGlobalParsedConfig(0), nullptr); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + EXPECT_EQ((*service_config)->GetGlobalParsedConfig(0), nullptr); } TEST_F(ServiceConfigTest, Parser1ErrorInvalidType) { const char* test_json = "{\"global_param\":\"5\"}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - absl::StrCat("Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG, - TestParser1::InvalidTypeErrorMessage()))); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + absl::StrCat("Service config parsing errors: [", + TestParser1::InvalidTypeErrorMessage(), "]")); } TEST_F(ServiceConfigTest, Parser1ErrorInvalidValue) { const char* test_json = "{\"global_param\":-5}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - absl::StrCat("Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG, - TestParser1::InvalidValueErrorMessage()))); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + absl::StrCat("Service config parsing errors: [", + TestParser1::InvalidValueErrorMessage(), "]")); } TEST_F(ServiceConfigTest, Parser2BasicTest) { const char* test_json = "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}], " "\"method_param\":5}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + const auto* vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); ASSERT_NE(vector_ptr, nullptr); auto parsed_config = ((*vector_ptr)[1]).get(); EXPECT_EQ(static_cast(parsed_config)->value(), 5); } TEST_F(ServiceConfigTest, Parser2DisabledViaChannelArg) { - grpc_arg arg = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_DISABLE_PARSING), 1); - grpc_channel_args args = {1, &arg}; + const ChannelArgs args = ChannelArgs().Set(GRPC_ARG_DISABLE_PARSING, 1); const char* test_json = "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}], " "\"method_param\":5}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = - ServiceConfigImpl::Create(ChannelArgs::FromC(&args), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + const auto* vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); ASSERT_NE(vector_ptr, nullptr); auto parsed_config = ((*vector_ptr)[1]).get(); EXPECT_EQ(parsed_config, nullptr); @@ -418,30 +381,26 @@ TEST_F(ServiceConfigTest, Parser2ErrorInvalidType) { const char* test_json = "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}], " "\"method_param\":\"5\"}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT( - grpc_error_std_string(error), - ::testing::ContainsRegex(absl::StrCat( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG, - TestParser2::InvalidTypeErrorMessage()))); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + absl::StrCat("Service config parsing errors: [" + "errors parsing methodConfig: [" + "index 0: [", + TestParser2::InvalidTypeErrorMessage(), "]]]")); } TEST_F(ServiceConfigTest, Parser2ErrorInvalidValue) { const char* test_json = "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}], " "\"method_param\":-5}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT( - grpc_error_std_string(error), - ::testing::ContainsRegex(absl::StrCat( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG, - TestParser2::InvalidValueErrorMessage()))); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + absl::StrCat("Service config parsing errors: [" + "errors parsing methodConfig: [" + "index 0: [", + TestParser2::InvalidValueErrorMessage(), "]]]")); } TEST(ServiceConfigParserTest, DoubleRegistration) { @@ -480,30 +439,27 @@ class ErroredParsersScopingTest : public ::testing::Test { TEST_F(ErroredParsersScopingTest, GlobalParams) { const char* test_json = "{}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT( - grpc_error_std_string(error), - ::testing::ContainsRegex(absl::StrCat( - "Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG, - ErrorParser::GlobalError(), ".*", ErrorParser::GlobalError()))); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(service_config.status().message(), + absl::StrCat("Service config parsing errors: [", + ErrorParser::GlobalError(), "; ", + ErrorParser::GlobalError(), "]")); } TEST_F(ErroredParsersScopingTest, MethodParams) { const char* test_json = "{\"methodConfig\": [{}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT( - grpc_error_std_string(error), - ::testing::ContainsRegex(absl::StrCat( - "Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG, - ErrorParser::GlobalError(), ".*", ErrorParser::GlobalError(), - ".*Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG, - ErrorParser::MethodError(), ".*", ErrorParser::MethodError()))); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ( + service_config.status().message(), + absl::StrCat("Service config parsing errors: [", + ErrorParser::GlobalError(), "; ", ErrorParser::GlobalError(), + "; " + "errors parsing methodConfig: [" + "index 0: [", + ErrorParser::MethodError(), "; ", ErrorParser::MethodError(), + "]]]")); } // @@ -527,12 +483,11 @@ class ClientChannelParserTest : public ::testing::Test { TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigPickFirst) { const char* test_json = "{\"loadBalancingConfig\": [{\"pick_first\":{}}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* parsed_config = static_cast( - svc_cfg->GetGlobalParsedConfig(0)); + (*service_config)->GetGlobalParsedConfig(0)); auto lb_config = parsed_config->parsed_lb_config(); EXPECT_EQ(lb_config->name(), "pick_first"); } @@ -540,11 +495,10 @@ TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigPickFirst) { TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigRoundRobin) { const char* test_json = "{\"loadBalancingConfig\": [{\"round_robin\":{}}, {}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); auto parsed_config = static_cast( - svc_cfg->GetGlobalParsedConfig(0)); + (*service_config)->GetGlobalParsedConfig(0)); auto lb_config = parsed_config->parsed_lb_config(); EXPECT_EQ(lb_config->name(), "round_robin"); } @@ -553,12 +507,11 @@ TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigGrpclb) { const char* test_json = "{\"loadBalancingConfig\": " "[{\"grpclb\":{\"childPolicy\":[{\"pick_first\":{}}]}}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* parsed_config = static_cast( - svc_cfg->GetGlobalParsedConfig(0)); + (*service_config)->GetGlobalParsedConfig(0)); auto lb_config = parsed_config->parsed_lb_config(); EXPECT_EQ(lb_config->name(), "grpclb"); } @@ -576,28 +529,26 @@ TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigXds) { " } }\n" " ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* parsed_config = static_cast( - svc_cfg->GetGlobalParsedConfig(0)); + (*service_config)->GetGlobalParsedConfig(0)); auto lb_config = parsed_config->parsed_lb_config(); EXPECT_EQ(lb_config->name(), "xds_cluster_resolver_experimental"); } TEST_F(ClientChannelParserTest, UnknownLoadBalancingConfig) { const char* test_json = "{\"loadBalancingConfig\": [{\"unknown\":{}}]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), - ::testing::ContainsRegex("Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG - "Client channel global parser" CHILD_ERROR_TAG - "field:loadBalancingConfig " - "error:No known policies in list: unknown")); - GRPC_ERROR_UNREF(error); + std::string(service_config.status().message()), + ::testing::MatchesRegex( + "Service config parsing errors: \\[" + "error parsing client channel global parameters:" CHILD_ERROR_TAG + "field:loadBalancingConfig " + "error:No known policies in list: unknown.*")); } TEST_F(ClientChannelParserTest, InvalidGrpclbLoadBalancingConfig) { @@ -606,68 +557,63 @@ TEST_F(ClientChannelParserTest, InvalidGrpclbLoadBalancingConfig) { " {\"grpclb\":{\"childPolicy\":1}}," " {\"round_robin\":{}}" "]}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG - "Client channel global parser" CHILD_ERROR_TAG - "field:loadBalancingConfig error:" - "errors parsing grpclb LB policy config: \\[" - "error parsing childPolicy field: type should be array\\]")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + std::string(service_config.status().message()), + ::testing::MatchesRegex( + "Service config parsing errors: \\[" + "error parsing client channel global parameters:" CHILD_ERROR_TAG + "field:loadBalancingConfig error:" + "errors parsing grpclb LB policy config: \\[" + "error parsing childPolicy field: type should be array\\].*")); } TEST_F(ClientChannelParserTest, ValidLoadBalancingPolicy) { const char* test_json = "{\"loadBalancingPolicy\":\"pick_first\"}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* parsed_config = static_cast( - svc_cfg->GetGlobalParsedConfig(0)); + (*service_config)->GetGlobalParsedConfig(0)); EXPECT_EQ(parsed_config->parsed_deprecated_lb_policy(), "pick_first"); } TEST_F(ClientChannelParserTest, ValidLoadBalancingPolicyAllCaps) { const char* test_json = "{\"loadBalancingPolicy\":\"PICK_FIRST\"}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* parsed_config = static_cast( - svc_cfg->GetGlobalParsedConfig(0)); + (*service_config)->GetGlobalParsedConfig(0)); EXPECT_EQ(parsed_config->parsed_deprecated_lb_policy(), "pick_first"); } TEST_F(ClientChannelParserTest, UnknownLoadBalancingPolicy) { const char* test_json = "{\"loadBalancingPolicy\":\"unknown\"}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG - "Client channel global parser" CHILD_ERROR_TAG - "field:loadBalancingPolicy error:Unknown lb policy")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + std::string(service_config.status().message()), + ::testing::MatchesRegex( + "Service config parsing errors: \\[" + "error parsing client channel global parameters:" CHILD_ERROR_TAG + "field:loadBalancingPolicy error:Unknown lb policy.*")); } TEST_F(ClientChannelParserTest, LoadBalancingPolicyXdsNotAllowed) { const char* test_json = "{\"loadBalancingPolicy\":\"xds_cluster_resolver_experimental\"}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG - "Client channel global parser" CHILD_ERROR_TAG - "field:loadBalancingPolicy " - "error:xds_cluster_resolver_experimental requires " - "a config. Please use loadBalancingConfig instead.")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + std::string(service_config.status().message()), + ::testing::MatchesRegex( + "Service config parsing errors: \\[" + "error parsing client channel global parameters:" CHILD_ERROR_TAG + "field:loadBalancingPolicy " + "error:xds_cluster_resolver_experimental requires " + "a config. Please use loadBalancingConfig instead.*")); } TEST_F(ClientChannelParserTest, ValidTimeout) { @@ -680,11 +626,12 @@ TEST_F(ClientChannelParserTest, ValidTimeout) { " \"timeout\": \"5s\"\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + const auto* vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); ASSERT_NE(vector_ptr, nullptr); auto parsed_config = ((*vector_ptr)[0]).get(); EXPECT_EQ( @@ -703,16 +650,17 @@ TEST_F(ClientChannelParserTest, InvalidTimeout) { " \"timeout\": \"5sec\"\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG - "Client channel parser" CHILD_ERROR_TAG - "field:timeout error:type should be STRING of the form given " - "by google.proto.Duration")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + std::string(service_config.status().message()), + ::testing::MatchesRegex( + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing client channel method parameters: " CHILD_ERROR_TAG + "field:timeout error:type should be STRING of the form given " + "by google.proto.Duration.*")); } TEST_F(ClientChannelParserTest, ValidWaitForReady) { @@ -725,11 +673,12 @@ TEST_F(ClientChannelParserTest, ValidWaitForReady) { " \"waitForReady\": true\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + const auto* vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); ASSERT_NE(vector_ptr, nullptr); auto parsed_config = ((*vector_ptr)[0]).get(); ASSERT_TRUE( @@ -752,15 +701,16 @@ TEST_F(ClientChannelParserTest, InvalidWaitForReady) { " \"waitForReady\": \"true\"\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG - "Client channel parser" CHILD_ERROR_TAG - "field:waitForReady error:Type should be true/false")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + std::string(service_config.status().message()), + ::testing::MatchesRegex( + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing client channel method parameters: " CHILD_ERROR_TAG + "field:waitForReady error:Type should be true/false.*")); } TEST_F(ClientChannelParserTest, ValidHealthCheck) { @@ -770,12 +720,11 @@ TEST_F(ClientChannelParserTest, ValidHealthCheck) { " \"serviceName\": \"health_check_service_name\"\n" " }\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* parsed_config = static_cast( - svc_cfg->GetGlobalParsedConfig(0)); + (*service_config)->GetGlobalParsedConfig(0)); ASSERT_NE(parsed_config, nullptr); EXPECT_EQ(parsed_config->health_check_service_name(), "health_check_service_name"); @@ -791,13 +740,15 @@ TEST_F(ClientChannelParserTest, InvalidHealthCheckMultipleEntries) { " \"serviceName\": \"health_check_service_name1\"\n" " }\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + // TODO(roth): When we convert the JSON API to return absl::Status + // instead of grpc_error, change this expectation to be a fixed string + // equality match. + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( "JSON parsing failed" CHILD_ERROR_TAG "duplicate key \"healthCheckConfig\" at index 104")); - GRPC_ERROR_UNREF(error); } // @@ -827,11 +778,10 @@ TEST_F(RetryParserTest, ValidRetryThrottling) { " \"tokenRatio\": 1.0\n" " }\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* parsed_config = static_cast( - svc_cfg->GetGlobalParsedConfig(0)); + (*service_config)->GetGlobalParsedConfig(0)); ASSERT_NE(parsed_config, nullptr); EXPECT_EQ(parsed_config->max_milli_tokens(), 2000); EXPECT_EQ(parsed_config->milli_token_ratio(), 1000); @@ -843,16 +793,15 @@ TEST_F(RetryParserTest, RetryThrottlingMissingFields) { " \"retryThrottling\": {\n" " }\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT( - grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG "retryThrottling" CHILD_ERROR_TAG - "field:retryThrottling field:maxTokens error:Not found" - ".*field:retryThrottling field:tokenRatio error:Not found")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), + ::testing::ContainsRegex( + "Service config parsing errors: \\[" + "error parsing retry global parameters:" + ".*retryThrottling" CHILD_ERROR_TAG + "field:retryThrottling field:maxTokens error:Not found" + ".*field:retryThrottling field:tokenRatio error:Not found")); } TEST_F(RetryParserTest, InvalidRetryThrottlingNegativeMaxTokens) { @@ -863,16 +812,15 @@ TEST_F(RetryParserTest, InvalidRetryThrottlingNegativeMaxTokens) { " \"tokenRatio\": 1.0\n" " }\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT( - grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG "retryThrottling" CHILD_ERROR_TAG - "field:retryThrottling field:maxTokens error:should " - "be greater than zero")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), + ::testing::ContainsRegex( + "Service config parsing errors: \\[" + "error parsing retry global parameters:" + ".*retryThrottling" CHILD_ERROR_TAG + "field:retryThrottling field:maxTokens error:should " + "be greater than zero")); } TEST_F(RetryParserTest, InvalidRetryThrottlingInvalidTokenRatio) { @@ -883,16 +831,14 @@ TEST_F(RetryParserTest, InvalidRetryThrottlingInvalidTokenRatio) { " \"tokenRatio\": -1\n" " }\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT( - grpc_error_std_string(error), - ::testing::ContainsRegex("Service config parsing error" CHILD_ERROR_TAG - "Global Params" CHILD_ERROR_TAG - "retryThrottling" CHILD_ERROR_TAG - "field:retryThrottling field:tokenRatio " - "error:Failed parsing")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), + ::testing::ContainsRegex("Service config parsing errors: \\[" + "error parsing retry global parameters:" + ".*retryThrottling" CHILD_ERROR_TAG + "field:retryThrottling field:tokenRatio " + "error:Failed parsing")); } TEST_F(RetryParserTest, ValidRetryPolicy) { @@ -911,11 +857,12 @@ TEST_F(RetryParserTest, ValidRetryPolicy) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + const auto* vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); ASSERT_NE(vector_ptr, nullptr); const auto* parsed_config = static_cast(((*vector_ptr)[0]).get()); @@ -939,14 +886,15 @@ TEST_F(RetryParserTest, InvalidRetryPolicyWrongType) { " \"retryPolicy\": 5\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "field:retryPolicy error:should be of type object")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyRequiredFieldsMissing) { @@ -961,18 +909,19 @@ TEST_F(RetryParserTest, InvalidRetryPolicyRequiredFieldsMissing) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG ".*field:maxAttempts error:required field missing" ".*field:initialBackoff error:does not exist" ".*field:maxBackoff error:does not exist" ".*field:backoffMultiplier error:required field missing")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyMaxAttemptsWrongType) { @@ -991,15 +940,16 @@ TEST_F(RetryParserTest, InvalidRetryPolicyMaxAttemptsWrongType) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:maxAttempts error:should be of type number")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyMaxAttemptsBadValue) { @@ -1018,15 +968,16 @@ TEST_F(RetryParserTest, InvalidRetryPolicyMaxAttemptsBadValue) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), - ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG - "retryPolicy" CHILD_ERROR_TAG - "field:maxAttempts error:should be at least 2")); - GRPC_ERROR_UNREF(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT( + std::string(service_config.status().message()), + ::testing::ContainsRegex("Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" + "retryPolicy" CHILD_ERROR_TAG + "field:maxAttempts error:should be at least 2")); } TEST_F(RetryParserTest, InvalidRetryPolicyInitialBackoffWrongType) { @@ -1045,16 +996,17 @@ TEST_F(RetryParserTest, InvalidRetryPolicyInitialBackoffWrongType) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:initialBackoff error:type should be STRING of the " "form given by google.proto.Duration")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyInitialBackoffBadValue) { @@ -1073,15 +1025,16 @@ TEST_F(RetryParserTest, InvalidRetryPolicyInitialBackoffBadValue) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:initialBackoff error:must be greater than 0")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyMaxBackoffWrongType) { @@ -1100,16 +1053,17 @@ TEST_F(RetryParserTest, InvalidRetryPolicyMaxBackoffWrongType) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:maxBackoff error:type should be STRING of the form " "given by google.proto.Duration")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyMaxBackoffBadValue) { @@ -1128,15 +1082,16 @@ TEST_F(RetryParserTest, InvalidRetryPolicyMaxBackoffBadValue) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:maxBackoff error:must be greater than 0")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyBackoffMultiplierWrongType) { @@ -1155,15 +1110,16 @@ TEST_F(RetryParserTest, InvalidRetryPolicyBackoffMultiplierWrongType) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:backoffMultiplier error:should be of type number")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyBackoffMultiplierBadValue) { @@ -1182,15 +1138,16 @@ TEST_F(RetryParserTest, InvalidRetryPolicyBackoffMultiplierBadValue) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:backoffMultiplier error:must be greater than 0")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyEmptyRetryableStatusCodes) { @@ -1209,15 +1166,16 @@ TEST_F(RetryParserTest, InvalidRetryPolicyEmptyRetryableStatusCodes) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:retryableStatusCodes error:must be non-empty")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyRetryableStatusCodesWrongType) { @@ -1236,15 +1194,16 @@ TEST_F(RetryParserTest, InvalidRetryPolicyRetryableStatusCodesWrongType) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:retryableStatusCodes error:must be of type array")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyUnparseableRetryableStatusCodes) { @@ -1263,17 +1222,18 @@ TEST_F(RetryParserTest, InvalidRetryPolicyUnparseableRetryableStatusCodes) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:retryableStatusCodes " "error:failed to parse status code" ".*field:retryableStatusCodes " "error:status codes should be of type string")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, ValidRetryPolicyWithPerAttemptRecvTimeout) { @@ -1293,15 +1253,14 @@ TEST_F(RetryParserTest, ValidRetryPolicyWithPerAttemptRecvTimeout) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - grpc_arg arg = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); - grpc_channel_args args = {1, &arg}; - auto svc_cfg = - ServiceConfigImpl::Create(ChannelArgs::FromC(&args), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); + const ChannelArgs args = + ChannelArgs().Set(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING, 1); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + const auto* vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); ASSERT_NE(vector_ptr, nullptr); const auto* parsed_config = static_cast(((*vector_ptr)[0]).get()); @@ -1333,11 +1292,12 @@ TEST_F(RetryParserTest, " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + const auto* vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); ASSERT_NE(vector_ptr, nullptr); const auto* parsed_config = static_cast(((*vector_ptr)[0]).get()); @@ -1368,15 +1328,14 @@ TEST_F(RetryParserTest, " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - grpc_arg arg = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); - grpc_channel_args args = {1, &arg}; - auto svc_cfg = - ServiceConfigImpl::Create(ChannelArgs::FromC(&args), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); + const ChannelArgs args = + ChannelArgs().Set(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING, 1); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + const auto* vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); ASSERT_NE(vector_ptr, nullptr); const auto* parsed_config = static_cast(((*vector_ptr)[0]).get()); @@ -1406,20 +1365,19 @@ TEST_F(RetryParserTest, InvalidRetryPolicyPerAttemptRecvTimeoutUnparseable) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - grpc_arg arg = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); - grpc_channel_args args = {1, &arg}; - auto svc_cfg = - ServiceConfigImpl::Create(ChannelArgs::FromC(&args), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + const ChannelArgs args = + ChannelArgs().Set(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING, 1); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:perAttemptRecvTimeout error:type must be STRING " "of the form given by google.proto.Duration.")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyPerAttemptRecvTimeoutWrongType) { @@ -1439,20 +1397,19 @@ TEST_F(RetryParserTest, InvalidRetryPolicyPerAttemptRecvTimeoutWrongType) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - grpc_arg arg = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); - grpc_channel_args args = {1, &arg}; - auto svc_cfg = - ServiceConfigImpl::Create(ChannelArgs::FromC(&args), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + const ChannelArgs args = + ChannelArgs().Set(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING, 1); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:perAttemptRecvTimeout error:type must be STRING " "of the form given by google.proto.Duration.")); - GRPC_ERROR_UNREF(error); } TEST_F(RetryParserTest, InvalidRetryPolicyPerAttemptRecvTimeoutBadValue) { @@ -1472,19 +1429,18 @@ TEST_F(RetryParserTest, InvalidRetryPolicyPerAttemptRecvTimeoutBadValue) { " }\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - grpc_arg arg = grpc_channel_arg_integer_create( - const_cast(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); - grpc_channel_args args = {1, &arg}; - auto svc_cfg = - ServiceConfigImpl::Create(ChannelArgs::FromC(&args), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + const ChannelArgs args = + ChannelArgs().Set(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING, 1); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing retry method parameters:.*" "retryPolicy" CHILD_ERROR_TAG "field:perAttemptRecvTimeout error:must be greater than 0")); - GRPC_ERROR_UNREF(error); } // @@ -1517,11 +1473,12 @@ TEST_F(MessageSizeParserTest, Valid) { " \"maxResponseMessageBytes\": 1024\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); - const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( - grpc_slice_from_static_string("/TestServ/TestMethod")); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); + const auto* vector_ptr = + (*service_config) + ->GetMethodParsedConfigVector( + grpc_slice_from_static_string("/TestServ/TestMethod")); ASSERT_NE(vector_ptr, nullptr); auto parsed_config = static_cast(((*vector_ptr)[0]).get()); @@ -1540,15 +1497,16 @@ TEST_F(MessageSizeParserTest, InvalidMaxRequestMessageBytes) { " \"maxRequestMessageBytes\": -1024\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing message size method parameters:.*" "Message size parser" CHILD_ERROR_TAG "field:maxRequestMessageBytes error:should be non-negative")); - GRPC_ERROR_UNREF(error); } TEST_F(MessageSizeParserTest, InvalidMaxResponseMessageBytes) { @@ -1561,16 +1519,17 @@ TEST_F(MessageSizeParserTest, InvalidMaxResponseMessageBytes) { " \"maxResponseMessageBytes\": {}\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - EXPECT_THAT(grpc_error_std_string(error), + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); + EXPECT_THAT(std::string(service_config.status().message()), ::testing::ContainsRegex( - "Service config parsing error" CHILD_ERROR_TAG - "Method Params" CHILD_ERROR_TAG "methodConfig" CHILD_ERROR_TAG + "Service config parsing errors: \\[" + "errors parsing methodConfig: \\[" + "index 0: \\[" + "error parsing message size method parameters:.*" "Message size parser" CHILD_ERROR_TAG "field:maxResponseMessageBytes error:should be of type " "number")); - GRPC_ERROR_UNREF(error); } } // namespace testing diff --git a/test/core/ext/filters/rbac/rbac_service_config_parser_test.cc b/test/core/ext/filters/rbac/rbac_service_config_parser_test.cc index 210dec74ea8..f4c94c2da63 100644 --- a/test/core/ext/filters/rbac/rbac_service_config_parser_test.cc +++ b/test/core/ext/filters/rbac/rbac_service_config_parser_test.cc @@ -40,12 +40,11 @@ TEST(RbacServiceConfigParsingTest, EmptyRbacPolicy) { " } ]" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* vector_ptr = - svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); + (*service_config)->GetMethodParsedConfigVector(grpc_empty_slice()); ASSERT_NE(vector_ptr, nullptr); auto* parsed_rbac_config = static_cast( ((*vector_ptr)[RbacServiceConfigParser::ParserIndex()]).get()); @@ -69,11 +68,10 @@ TEST(RbacServiceConfigParsingTest, MissingChannelArg) { " } ]" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfigImpl::Create(ChannelArgs(), test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(ChannelArgs(), test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* vector_ptr = - svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); + (*service_config)->GetMethodParsedConfigVector(grpc_empty_slice()); ASSERT_NE(vector_ptr, nullptr); auto* parsed_rbac_config = static_cast( ((*vector_ptr)[RbacServiceConfigParser::ParserIndex()]).get()); @@ -91,12 +89,11 @@ TEST(RbacServiceConfigParsingTest, EmptyRbacPolicyArray) { " \"rbacPolicy\": []" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* vector_ptr = - svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); + (*service_config)->GetMethodParsedConfigVector(grpc_empty_slice()); ASSERT_NE(vector_ptr, nullptr); auto* parsed_rbac_config = static_cast( ((*vector_ptr)[RbacServiceConfigParser::ParserIndex()]).get()); @@ -114,12 +111,11 @@ TEST(RbacServiceConfigParsingTest, MultipleRbacPolicies) { " \"rbacPolicy\": [ {}, {}, {} ]" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* vector_ptr = - svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); + (*service_config)->GetMethodParsedConfigVector(grpc_empty_slice()); ASSERT_NE(vector_ptr, nullptr); auto* parsed_rbac_config = static_cast( ((*vector_ptr)[RbacServiceConfigParser::ParserIndex()]).get()); @@ -142,14 +138,14 @@ TEST(RbacServiceConfigParsingTest, BadRbacPolicyType) { " \"rbacPolicy\": 1234" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG - "field:rbacPolicy error:type should be ARRAY")); - GRPC_ERROR_UNREF(error); + "field:rbacPolicy error:type should be ARRAY")) + << service_config.status(); } TEST(RbacServiceConfigParsingTest, BadRulesType) { @@ -162,15 +158,15 @@ TEST(RbacServiceConfigParsingTest, BadRulesType) { " \"rbacPolicy\": [{\"rules\":1}]" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG "rbacPolicy\\[0\\]" CHILD_ERROR_TAG - "field:rules error:type should be OBJECT")); - GRPC_ERROR_UNREF(error); + "field:rules error:type should be OBJECT")) + << service_config.status(); } TEST(RbacServiceConfigParsingTest, BadActionAndPolicyType) { @@ -188,16 +184,16 @@ TEST(RbacServiceConfigParsingTest, BadActionAndPolicyType) { " } ]\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG "rbacPolicy\\[0\\]" CHILD_ERROR_TAG "field:action error:type should be NUMBER.*" - "field:policies error:type should be OBJECT")); - GRPC_ERROR_UNREF(error); + "field:policies error:type should be OBJECT")) + << service_config.status(); } TEST(RbacServiceConfigParsingTest, MissingPermissionAndPrincipals) { @@ -218,17 +214,17 @@ TEST(RbacServiceConfigParsingTest, MissingPermissionAndPrincipals) { " } ]\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG "rbacPolicy\\[0\\]" CHILD_ERROR_TAG "policies key:'policy'" CHILD_ERROR_TAG "field:permissions error:does not exist.*" - "field:principals error:does not exist")); - GRPC_ERROR_UNREF(error); + "field:principals error:does not exist")) + << service_config.status(); } TEST(RbacServiceConfigParsingTest, EmptyPrincipalAndPermission) { @@ -251,17 +247,17 @@ TEST(RbacServiceConfigParsingTest, EmptyPrincipalAndPermission) { " } ]\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "Rbac parser" CHILD_ERROR_TAG "rbacPolicy\\[0\\]" CHILD_ERROR_TAG "policies key:'policy'" CHILD_ERROR_TAG "permissions\\[0\\]" CHILD_ERROR_TAG "No valid rule found.*" - "principals\\[0\\]" CHILD_ERROR_TAG "No valid id found")); - GRPC_ERROR_UNREF(error); + "principals\\[0\\]" CHILD_ERROR_TAG "No valid id found")) + << service_config.status(); } TEST(RbacServiceConfigParsingTest, VariousPermissionsAndPrincipalsTypes) { @@ -308,12 +304,11 @@ TEST(RbacServiceConfigParsingTest, VariousPermissionsAndPrincipalsTypes) { " } ]\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* vector_ptr = - svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); + (*service_config)->GetMethodParsedConfigVector(grpc_empty_slice()); ASSERT_NE(vector_ptr, nullptr); auto* parsed_rbac_config = static_cast( ((*vector_ptr)[RbacServiceConfigParser::ParserIndex()]).get()); @@ -365,11 +360,11 @@ TEST(RbacServiceConfigParsingTest, VariousPermissionsAndPrincipalsBadTypes) { " } ]\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "Rbac parser" CHILD_ERROR_TAG "rbacPolicy\\[0\\]" CHILD_ERROR_TAG "policies key:'policy'" CHILD_ERROR_TAG @@ -414,8 +409,8 @@ TEST(RbacServiceConfigParsingTest, VariousPermissionsAndPrincipalsBadTypes) { "principals\\[9\\]" CHILD_ERROR_TAG "field:metadata error:type should be OBJECT.*" "principals\\[10\\]" CHILD_ERROR_TAG - "field:notId error:type should be OBJECT.*")); - GRPC_ERROR_UNREF(error); + "field:notId error:type should be OBJECT.*")) + << service_config.status(); } TEST(RbacServiceConfigParsingTest, HeaderMatcherVariousTypes) { @@ -449,12 +444,11 @@ TEST(RbacServiceConfigParsingTest, HeaderMatcherVariousTypes) { " } ]\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* vector_ptr = - svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); + (*service_config)->GetMethodParsedConfigVector(grpc_empty_slice()); ASSERT_NE(vector_ptr, nullptr); auto* parsed_rbac_config = static_cast( ((*vector_ptr)[RbacServiceConfigParser::ParserIndex()]).get()); @@ -492,11 +486,11 @@ TEST(RbacServiceConfigParsingTest, HeaderMatcherBadTypes) { " } ]\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex( "Rbac parser" CHILD_ERROR_TAG "rbacPolicy\\[0\\]" CHILD_ERROR_TAG "policies key:'policy'" CHILD_ERROR_TAG @@ -514,8 +508,8 @@ TEST(RbacServiceConfigParsingTest, HeaderMatcherBadTypes) { "permissions\\[5\\]" CHILD_ERROR_TAG "header" CHILD_ERROR_TAG "field:suffixMatch error:type should be STRING.*" "permissions\\[6\\]" CHILD_ERROR_TAG "header" CHILD_ERROR_TAG - "field:containsMatch error:type should be STRING.*")); - GRPC_ERROR_UNREF(error); + "field:containsMatch error:type should be STRING.*")) + << service_config.status(); } TEST(RbacServiceConfigParsingTest, StringMatcherVariousTypes) { @@ -546,12 +540,11 @@ TEST(RbacServiceConfigParsingTest, StringMatcherVariousTypes) { " } ]\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + ASSERT_TRUE(service_config.ok()) << service_config.status(); const auto* vector_ptr = - svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); + (*service_config)->GetMethodParsedConfigVector(grpc_empty_slice()); ASSERT_NE(vector_ptr, nullptr); auto* parsed_rbac_config = static_cast( ((*vector_ptr)[RbacServiceConfigParser::ParserIndex()]).get()); @@ -587,11 +580,11 @@ TEST(RbacServiceConfigParsingTest, StringMatcherBadTypes) { " } ]\n" " } ]\n" "}"; - grpc_error_handle error = GRPC_ERROR_NONE; ChannelArgs args = ChannelArgs().Set(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG, 1); - auto svc_cfg = ServiceConfigImpl::Create(args, test_json, &error); + auto service_config = ServiceConfigImpl::Create(args, test_json); + EXPECT_EQ(service_config.status().code(), absl::StatusCode::kInvalidArgument); EXPECT_THAT( - grpc_error_std_string(error), + std::string(service_config.status().message()), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG "rbacPolicy\\[0\\]" CHILD_ERROR_TAG "policies key:'policy'" CHILD_ERROR_TAG @@ -610,8 +603,8 @@ TEST(RbacServiceConfigParsingTest, StringMatcherBadTypes) { "field:safeRegex error:type should be OBJECT.*" "permissions\\[4\\]" CHILD_ERROR_TAG "requestedServerName" CHILD_ERROR_TAG - "field:contains error:type should be STRING.*")); - GRPC_ERROR_UNREF(error); + "field:contains error:type should be STRING.*")) + << service_config.status(); } } // namespace diff --git a/test/cpp/client/client_channel_stress_test.cc b/test/cpp/client/client_channel_stress_test.cc index 999c9a9fe64..397c35c817e 100644 --- a/test/cpp/client/client_channel_stress_test.cc +++ b/test/cpp/client/client_channel_stress_test.cc @@ -240,11 +240,10 @@ class ClientChannelStressTest { static grpc_core::Resolver::Result MakeResolverResult( const std::vector& balancer_address_data) { grpc_core::Resolver::Result result; - grpc_error_handle error = GRPC_ERROR_NONE; result.service_config = grpc_core::ServiceConfigImpl::Create( - grpc_core::ChannelArgs(), "{\"loadBalancingConfig\":[{\"grpclb\":{}}]}", - &error); - GPR_ASSERT(GRPC_ERROR_IS_NONE(error)); + grpc_core::ChannelArgs(), + "{\"loadBalancingConfig\":[{\"grpclb\":{}}]}"); + GPR_ASSERT(result.service_config.ok()); grpc_core::ServerAddressList balancer_addresses = CreateAddressListFromAddressDataList(balancer_address_data); result.args = grpc_core::SetGrpcLbBalancerAddresses( diff --git a/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc b/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc index cadb06175ad..f6aeeaab5fe 100644 --- a/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc +++ b/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc @@ -69,11 +69,10 @@ void TryConnectAndDestroy() { grpc_core::ServerAddressList addresses; addresses.emplace_back(address.addr, address.len, grpc_core::ChannelArgs()); grpc_core::Resolver::Result lb_address_result; - grpc_error_handle error = GRPC_ERROR_NONE; lb_address_result.service_config = grpc_core::ServiceConfigImpl::Create( - grpc_core::ChannelArgs(), "{\"loadBalancingConfig\":[{\"grpclb\":{}}]}", - &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + grpc_core::ChannelArgs(), "{\"loadBalancingConfig\":[{\"grpclb\":{}}]}"); + ASSERT_TRUE(lb_address_result.service_config.ok()) + << lb_address_result.service_config.status(); lb_address_result.args = grpc_core::SetGrpcLbBalancerAddresses( grpc_core::ChannelArgs(), addresses); response_generator->SetResponse(lb_address_result); diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index 6ea1027672f..960ca34b069 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -210,10 +210,9 @@ class FakeResolverResponseGeneratorWrapper { result.resolution_note = "fake resolver empty address list"; } if (service_config_json != nullptr) { - grpc_error_handle error = GRPC_ERROR_NONE; result.service_config = grpc_core::ServiceConfigImpl::Create( - grpc_core::ChannelArgs(), service_config_json, &error); - GPR_ASSERT(*result.service_config != nullptr); + grpc_core::ChannelArgs(), service_config_json); + GPR_ASSERT(result.service_config.ok()); } return result; } diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index bfdd2119e4d..2ec14ff4b7c 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -528,10 +528,9 @@ class GrpclbEnd2endTest : public ::testing::Test { grpc_core::Resolver::Result result; result.addresses = CreateLbAddressesFromAddressDataList(backend_address_data); - grpc_error_handle error = GRPC_ERROR_NONE; result.service_config = grpc_core::ServiceConfigImpl::Create( - grpc_core::ChannelArgs(), service_config_json, &error); - GPR_ASSERT(GRPC_ERROR_IS_NONE(error)); + grpc_core::ChannelArgs(), service_config_json); + GPR_ASSERT(result.service_config.ok()); grpc_core::ServerAddressList balancer_addresses = CreateLbAddressesFromAddressDataList(balancer_address_data); result.args = grpc_core::SetGrpcLbBalancerAddresses( diff --git a/test/cpp/end2end/rls_end2end_test.cc b/test/cpp/end2end/rls_end2end_test.cc index 471a99dce09..7b0b9118604 100644 --- a/test/cpp/end2end/rls_end2end_test.cc +++ b/test/cpp/end2end/rls_end2end_test.cc @@ -145,13 +145,9 @@ class FakeResolverResponseGeneratorWrapper { static grpc_core::Resolver::Result BuildFakeResults( absl::string_view service_config_json) { grpc_core::Resolver::Result result; - grpc_error_handle error = GRPC_ERROR_NONE; - result.service_config = grpc_core::ServiceConfigImpl::Create( - result.args, service_config_json, &error); - EXPECT_EQ(error, GRPC_ERROR_NONE) - << "JSON: " << service_config_json - << "Error: " << grpc_error_std_string(error); - EXPECT_NE(*result.service_config, nullptr); + result.service_config = + grpc_core::ServiceConfigImpl::Create(result.args, service_config_json); + EXPECT_TRUE(result.service_config.ok()) << result.service_config.status(); return result; } diff --git a/test/cpp/end2end/service_config_end2end_test.cc b/test/cpp/end2end/service_config_end2end_test.cc index 90252eaa53f..183f77530a1 100644 --- a/test/cpp/end2end/service_config_end2end_test.cc +++ b/test/cpp/end2end/service_config_end2end_test.cc @@ -198,10 +198,9 @@ class ServiceConfigEnd2endTest : public ::testing::Test { void SetNextResolutionValidServiceConfig(const std::vector& ports) { grpc_core::ExecCtx exec_ctx; grpc_core::Resolver::Result result = BuildFakeResults(ports); - grpc_error_handle error = GRPC_ERROR_NONE; - result.service_config = grpc_core::ServiceConfigImpl::Create( - grpc_core::ChannelArgs(), "{}", &error); - ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); + result.service_config = + grpc_core::ServiceConfigImpl::Create(grpc_core::ChannelArgs(), "{}"); + ASSERT_TRUE(result.service_config.ok()) << result.service_config.status(); response_generator_->SetResponse(result); } @@ -217,13 +216,8 @@ class ServiceConfigEnd2endTest : public ::testing::Test { const char* svc_cfg) { grpc_core::ExecCtx exec_ctx; grpc_core::Resolver::Result result = BuildFakeResults(ports); - grpc_error_handle error = GRPC_ERROR_NONE; - result.service_config = grpc_core::ServiceConfigImpl::Create( - grpc_core::ChannelArgs(), svc_cfg, &error); - if (!GRPC_ERROR_IS_NONE(error)) { - result.service_config = grpc_error_to_absl_status(error); - GRPC_ERROR_UNREF(error); - } + result.service_config = + grpc_core::ServiceConfigImpl::Create(grpc_core::ChannelArgs(), svc_cfg); response_generator_->SetResponse(result); }