From 33fa301c5bec4de09520b4ea59308d83429db0a1 Mon Sep 17 00:00:00 2001 From: Yijie Ma Date: Mon, 5 Aug 2024 13:28:16 -0700 Subject: [PATCH] [c-ares] Return `UNAVAILABLE` status for `ARES_ECONNREFUSED` (#37371) This is so that the client can retry in this scenario. Fix #37306 Closes #37371 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/37371 from yijiem:better-ares-error-code 6f367d105197c243fdf95b280c25e4b0fe6396a9 PiperOrigin-RevId: 659662762 --- src/core/lib/event_engine/ares_resolver.cc | 2 ++ .../resolver/dns/c_ares/grpc_ares_wrapper.cc | 29 ++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/core/lib/event_engine/ares_resolver.cc b/src/core/lib/event_engine/ares_resolver.cc index 160933b99ee..00f50c27157 100644 --- a/src/core/lib/event_engine/ares_resolver.cc +++ b/src/core/lib/event_engine/ares_resolver.cc @@ -97,6 +97,8 @@ absl::Status AresStatusToAbslStatus(int status, absl::string_view error_msg) { return absl::UnimplementedError(error_msg); case ARES_ENOTFOUND: return absl::NotFoundError(error_msg); + case ARES_ECONNREFUSED: + return absl::UnavailableError(error_msg); default: return absl::UnknownError(error_msg); } diff --git a/src/core/resolver/dns/c_ares/grpc_ares_wrapper.cc b/src/core/resolver/dns/c_ares/grpc_ares_wrapper.cc index 373727a9902..0dcd1667d1e 100644 --- a/src/core/resolver/dns/c_ares/grpc_ares_wrapper.cc +++ b/src/core/resolver/dns/c_ares/grpc_ares_wrapper.cc @@ -181,6 +181,22 @@ class GrpcAresQuery final { const std::string name_; }; +static absl::Status AresStatusToAbslStatus(int status, + absl::string_view error_msg) { + switch (status) { + case ARES_ECANCELLED: + return absl::CancelledError(error_msg); + case ARES_ENOTIMP: + return absl::UnimplementedError(error_msg); + case ARES_ENOTFOUND: + return absl::NotFoundError(error_msg); + case ARES_ECONNREFUSED: + return absl::UnavailableError(error_msg); + default: + return absl::UnknownError(error_msg); + } +} + static grpc_ares_ev_driver* grpc_ares_ev_driver_ref( grpc_ares_ev_driver* ev_driver) ABSL_EXCLUSIVE_LOCKS_REQUIRED(&grpc_ares_request::mu) { @@ -715,8 +731,8 @@ static void on_hostbyname_done_locked(void* arg, int status, int /*timeouts*/, hr->qtype, hr->host, hr->is_balancer, ares_strerror(status)); GRPC_CARES_TRACE_LOG("request:%p on_hostbyname_done_locked: %s", r, error_msg.c_str()); - grpc_error_handle error = GRPC_ERROR_CREATE(error_msg); - r->error = grpc_error_add_child(error, r->error); + r->error = grpc_error_add_child(AresStatusToAbslStatus(status, error_msg), + r->error); } destroy_hostbyname_request_locked(hr); } @@ -761,8 +777,8 @@ static void on_srv_query_done_locked(void* arg, int status, int /*timeouts*/, ares_strerror(status)); GRPC_CARES_TRACE_LOG("request:%p on_srv_query_done_locked: %s", r, error_msg.c_str()); - grpc_error_handle error = GRPC_ERROR_CREATE(error_msg); - r->error = grpc_error_add_child(error, r->error); + r->error = grpc_error_add_child(AresStatusToAbslStatus(status, error_msg), + r->error); } delete q; } @@ -780,7 +796,6 @@ static void on_txt_done_locked(void* arg, int status, int /*timeouts*/, const size_t prefix_len = sizeof(g_service_config_attribute_prefix) - 1; struct ares_txt_ext* result = nullptr; struct ares_txt_ext* reply = nullptr; - grpc_error_handle error; if (status != ARES_SUCCESS) goto fail; GRPC_CARES_TRACE_LOG("request:%p on_txt_done_locked name=%s ARES_SUCCESS", r, q->name().c_str()); @@ -824,8 +839,8 @@ fail: q->name(), ares_strerror(status)); GRPC_CARES_TRACE_LOG("request:%p on_txt_done_locked %s", r, error_msg.c_str()); - error = GRPC_ERROR_CREATE(error_msg); - r->error = grpc_error_add_child(error, r->error); + r->error = + grpc_error_add_child(AresStatusToAbslStatus(status, error_msg), r->error); } grpc_error_handle set_request_dns_server(grpc_ares_request* r,