Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Revert "Added absl::Status support to error_utils (#27358)" (#27418)

Browse Source 
This reverts commit 9b3f75d322.
pull/27429/head
Craig Tiller 3 years ago committed by GitHub
parent 97631cf34c
commit 933676c56c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 34 additions and 85 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 42
      src/core/lib/iomgr/error.cc
  2. 2
      src/core/lib/iomgr/error.h
  3. 27
      src/core/lib/transport/error_utils.cc
  4. 2
      src/ruby/spec/client_server_spec.rb
  5. 10
      test/core/iomgr/error_test.cc
  6. 6
      test/core/transport/error_utils_test.cc
  7. 2
      test/cpp/end2end/end2end_test.cc
  8. 28
      test/cpp/microbenchmarks/bm_error.cc

42
src/core/lib/iomgr/error.cc
Unescape View File

@ -87,9 +87,9 @@ absl::Status grpc_wsa_error(const grpc_core::DebugLocation& location, int err,
char* utf8_message = gpr_format_message(err); char* utf8_message = gpr_format_message(err);
absl::Status s = absl::Status s =
StatusCreate(absl::StatusCode::kUnknown, "WSA Error", location, {}); StatusCreate(absl::StatusCode::kUnknown, "WSA Error", location, {});
StatusSetInt(&s, grpc_core::StatusIntProperty::kWsaError, err); StatusSetInt(&s, grpc_core::StatusIntProperty::WSA_ERROR, err);
StatusSetStr(&s, grpc_core::StatusStrProperty::kOsError, utf8_message); StatusSetStr(&s, grpc_core::StatusStrProperty::OS_ERROR, utf8_message);
StatusSetStr(&s, grpc_core::StatusStrProperty::kSyscall, call_name); StatusSetStr(&s, grpc_core::StatusStrProperty::SYSCALL, call_name);
} }
#endif #endif
@ -108,22 +108,6 @@ bool grpc_error_get_int(grpc_error_handle error, grpc_error_ints which,
*p = *value; *p = *value;
return true; return true;
} else { } else {
// TODO(veblush): Remove this once absl::Status migration is done
if (which == GRPC_ERROR_INT_GRPC_STATUS) {
switch (error.code()) {
case absl::StatusCode::kOk:
*p = GRPC_STATUS_OK;
return true;
case absl::StatusCode::kResourceExhausted:
*p = GRPC_STATUS_RESOURCE_EXHAUSTED;
return true;
case absl::StatusCode::kCancelled:
*p = GRPC_STATUS_CANCELLED;
return true;
default:
break;
}
}
return false; return false;
} }
} }
@ -146,12 +130,6 @@ bool grpc_error_get_str(grpc_error_handle error, grpc_error_strs which,
*s = grpc_slice_from_copied_buffer(value->c_str(), value->size()); *s = grpc_slice_from_copied_buffer(value->c_str(), value->size());
return true; return true;
} else { } else {
// TODO(veblush): Remove this once absl::Status migration is done
if (which == GRPC_ERROR_STR_DESCRIPTION && !error.message().empty()) {
*s = grpc_slice_from_copied_buffer(error.message().data(),
error.message().size());
return true;
}
return false; return false;
} }
} }
@ -577,11 +555,11 @@ struct special_error_status_map {
const special_error_status_map error_status_map[] = { const special_error_status_map error_status_map[] = {
{GRPC_STATUS_OK, "", 0}, // GRPC_ERROR_NONE {GRPC_STATUS_OK, "", 0}, // GRPC_ERROR_NONE
{GRPC_STATUS_INVALID_ARGUMENT, "", 0}, // GRPC_ERROR_RESERVED_1 {GRPC_STATUS_INVALID_ARGUMENT, "", 0}, // GRPC_ERROR_RESERVED_1
{GRPC_STATUS_RESOURCE_EXHAUSTED, "RESOURCE_EXHAUSTED", {GRPC_STATUS_RESOURCE_EXHAUSTED, "Out of memory",
strlen("RESOURCE_EXHAUSTED")}, // GRPC_ERROR_OOM strlen("Out of memory")}, // GRPC_ERROR_OOM
{GRPC_STATUS_INVALID_ARGUMENT, "", 0}, // GRPC_ERROR_RESERVED_2 {GRPC_STATUS_INVALID_ARGUMENT, "", 0}, // GRPC_ERROR_RESERVED_2
{GRPC_STATUS_CANCELLED, "CANCELLED", {GRPC_STATUS_CANCELLED, "Cancelled",
strlen("CANCELLED")}, // GRPC_ERROR_CANCELLED strlen("Cancelled")}, // GRPC_ERROR_CANCELLED
}; };
bool grpc_error_get_int(grpc_error_handle err, grpc_error_ints which, bool grpc_error_get_int(grpc_error_handle err, grpc_error_ints which,
@ -653,9 +631,9 @@ grpc_error_handle grpc_error_add_child(grpc_error_handle src,
} }
} }
static const char* no_error_string = "\"OK\""; static const char* no_error_string = "\"No Error\"";
static const char* oom_error_string = "\"RESOURCE_EXHAUSTED\""; static const char* oom_error_string = "\"Out of memory\"";
static const char* cancelled_error_string = "\"CANCELLED\""; static const char* cancelled_error_string = "\"Cancelled\"";
struct kv_pair { struct kv_pair {
char* key; char* key;

2
src/core/lib/iomgr/error.h
Unescape View File

@ -157,7 +157,7 @@ void grpc_enable_error_creation();
#ifdef GRPC_ERROR_IS_ABSEIL_STATUS #ifdef GRPC_ERROR_IS_ABSEIL_STATUS
#define GRPC_ERROR_NONE absl::OkStatus() #define GRPC_ERROR_NONE absl::OkStatus()
#define GRPC_ERROR_OOM absl::Status(absl::ResourceExhaustedError("")) #define GRPC_ERROR_OOM absl::Status(absl::ResourceExhaustedError)
#define GRPC_ERROR_CANCELLED absl::CancelledError() #define GRPC_ERROR_CANCELLED absl::CancelledError()
#define GRPC_ERROR_REF(err) (err) #define GRPC_ERROR_REF(err) (err)

27
src/core/lib/transport/error_utils.cc
Unescape View File

@ -22,7 +22,6 @@
#include <grpc/support/string_util.h> #include <grpc/support/string_util.h>
#include "src/core/lib/gprpp/status_helper.h"
#include "src/core/lib/iomgr/error_internal.h" #include "src/core/lib/iomgr/error_internal.h"
#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_internal.h"
#include "src/core/lib/transport/status_conversion.h" #include "src/core/lib/transport/status_conversion.h"
@ -34,14 +33,7 @@ static grpc_error_handle recursively_find_error_with_field(
if (grpc_error_get_int(error, which, &unused)) { if (grpc_error_get_int(error, which, &unused)) {
return error; return error;
} }
#ifdef GRPC_ERROR_IS_ABSEIL_STATUS if (grpc_error_is_special(error)) return nullptr;
std::vector<absl::Status> children = grpc_core::StatusGetChildren(error);
for (auto child : children) {
grpc_error_handle result = recursively_find_error_with_field(child, which);
if (result != GRPC_ERROR_NONE) return result;
}
#else
if (grpc_error_is_special(error)) return GRPC_ERROR_NONE;
// Otherwise, search through its children. // Otherwise, search through its children.
uint8_t slot = error->first_err; uint8_t slot = error->first_err;
while (slot != UINT8_MAX) { while (slot != UINT8_MAX) {
@ -52,8 +44,7 @@ static grpc_error_handle recursively_find_error_with_field(
if (result) return result; if (result) return result;
slot = lerr->next; slot = lerr->next;
} }
#endif return nullptr;
return GRPC_ERROR_NONE;
} }
void grpc_error_get_status(grpc_error_handle error, grpc_millis deadline, void grpc_error_get_status(grpc_error_handle error, grpc_millis deadline,
@ -128,12 +119,7 @@ void grpc_error_get_status(grpc_error_handle error, grpc_millis deadline,
if (slice != nullptr) { if (slice != nullptr) {
if (!grpc_error_get_str(found_error, GRPC_ERROR_STR_GRPC_MESSAGE, slice)) { if (!grpc_error_get_str(found_error, GRPC_ERROR_STR_GRPC_MESSAGE, slice)) {
if (!grpc_error_get_str(found_error, GRPC_ERROR_STR_DESCRIPTION, slice)) { if (!grpc_error_get_str(found_error, GRPC_ERROR_STR_DESCRIPTION, slice)) {
#ifdef GRPC_ERROR_IS_ABSEIL_STATUS
*slice =
grpc_slice_from_copied_string(grpc_error_std_string(error).c_str());
#else
*slice = grpc_slice_from_static_string("unknown error"); *slice = grpc_slice_from_static_string("unknown error");
#endif
} }
} }
} }
@ -167,14 +153,6 @@ bool grpc_error_has_clear_grpc_status(grpc_error_handle error) {
if (grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &unused)) { if (grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &unused)) {
return true; return true;
} }
#ifdef GRPC_ERROR_IS_ABSEIL_STATUS
std::vector<absl::Status> children = grpc_core::StatusGetChildren(error);
for (auto child : children) {
if (grpc_error_has_clear_grpc_status(child)) {
return true;
}
}
#else
uint8_t slot = error->first_err; uint8_t slot = error->first_err;
while (slot != UINT8_MAX) { while (slot != UINT8_MAX) {
grpc_linked_error* lerr = grpc_linked_error* lerr =
@ -184,6 +162,5 @@ bool grpc_error_has_clear_grpc_status(grpc_error_handle error) {
} }
slot = lerr->next; slot = lerr->next;
} }
#endif
return false; return false;
} }

2
src/ruby/spec/client_server_spec.rb
Unescape View File

@ -311,7 +311,7 @@ shared_examples 'basic GRPC message delivery is OK' do
it 'clients can cancel a call on the server' do it 'clients can cancel a call on the server' do
expected_code = StatusCodes::CANCELLED expected_code = StatusCodes::CANCELLED
expected_details = 'CANCELLED' expected_details = 'Cancelled'
cancel_proc = proc { |call| call.cancel } cancel_proc = proc { |call| call.cancel }
client_cancel_test(cancel_proc, expected_code, expected_details) client_cancel_test(cancel_proc, expected_code, expected_details)
end end

10
test/core/iomgr/error_test.cc
Unescape View File

@ -28,7 +28,7 @@
static void test_set_get_int() { static void test_set_get_int() {
grpc_error_handle error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Test"); grpc_error_handle error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Test");
GPR_ASSERT(error != GRPC_ERROR_NONE); GPR_ASSERT(error);
intptr_t i = 0; intptr_t i = 0;
GPR_ASSERT(grpc_error_get_int(error, GRPC_ERROR_INT_FILE_LINE, &i)); GPR_ASSERT(grpc_error_get_int(error, GRPC_ERROR_INT_FILE_LINE, &i));
GPR_ASSERT(i); // line set will never be 0 GPR_ASSERT(i); // line set will never be 0
@ -110,7 +110,7 @@ static void test_create_referencing() {
GRPC_ERROR_STR_GRPC_MESSAGE, grpc_slice_from_static_string("message")); GRPC_ERROR_STR_GRPC_MESSAGE, grpc_slice_from_static_string("message"));
grpc_error_handle parent = grpc_error_handle parent =
GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING("Parent", &child, 1); GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING("Parent", &child, 1);
GPR_ASSERT(parent != GRPC_ERROR_NONE); GPR_ASSERT(parent);
GRPC_ERROR_UNREF(child); GRPC_ERROR_UNREF(child);
GRPC_ERROR_UNREF(parent); GRPC_ERROR_UNREF(parent);
@ -130,7 +130,7 @@ static void test_create_referencing_many() {
grpc_error_handle parent = grpc_error_handle parent =
GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING("Parent", children, 3); GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING("Parent", children, 3);
GPR_ASSERT(parent != GRPC_ERROR_NONE); GPR_ASSERT(parent);
for (size_t i = 0; i < 3; ++i) { for (size_t i = 0; i < 3; ++i) {
GRPC_ERROR_UNREF(children[i]); GRPC_ERROR_UNREF(children[i]);
@ -188,8 +188,6 @@ static void test_os_error() {
} }
static void test_overflow() { static void test_overflow() {
// absl::Status doesn't have a limit so there is no overflow
#ifndef GRPC_ERROR_IS_ABSEIL_STATUS
grpc_error_handle error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Overflow"); grpc_error_handle error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Overflow");
for (size_t i = 0; i < 150; ++i) { for (size_t i = 0; i < 150; ++i) {
@ -213,7 +211,7 @@ static void test_overflow() {
GPR_ASSERT(i == 10); GPR_ASSERT(i == 10);
GRPC_ERROR_UNREF(error); GRPC_ERROR_UNREF(error);
#endif ;
} }
int main(int argc, char** argv) { int main(int argc, char** argv) {

6
test/core/transport/error_utils_test.cc
Unescape View File

@ -41,7 +41,7 @@ TEST(ErrorUtilsTest, GrpcSpecialErrorNoneToAbslStatus) {
// ---- Asymmetry of conversions of "Special" errors ---- // ---- Asymmetry of conversions of "Special" errors ----
TEST(ErrorUtilsTest, AbslStatusToGrpcErrorDoesNotReturnSpecialVariables) { TEST(ErrorUtilsTest, AbslStatusToGrpcErrorDoesNotReturnSpecialVariables) {
grpc_error_handle error = grpc_error_handle error =
absl_status_to_grpc_error(absl::CancelledError("CANCELLED")); absl_status_to_grpc_error(absl::CancelledError("Cancelled"));
ASSERT_NE(error, GRPC_ERROR_CANCELLED); ASSERT_NE(error, GRPC_ERROR_CANCELLED);
GRPC_ERROR_UNREF(error); GRPC_ERROR_UNREF(error);
} }
@ -49,13 +49,13 @@ TEST(ErrorUtilsTest, AbslStatusToGrpcErrorDoesNotReturnSpecialVariables) {
TEST(ErrorUtilsTest, GrpcSpecialErrorCancelledToAbslStatus) { TEST(ErrorUtilsTest, GrpcSpecialErrorCancelledToAbslStatus) {
absl::Status status = grpc_error_to_absl_status(GRPC_ERROR_CANCELLED); absl::Status status = grpc_error_to_absl_status(GRPC_ERROR_CANCELLED);
ASSERT_TRUE(absl::IsCancelled(status)); ASSERT_TRUE(absl::IsCancelled(status));
ASSERT_EQ(status.message(), "CANCELLED"); ASSERT_EQ(status.message(), "Cancelled");
} }
TEST(ErrorUtilsTest, GrpcSpecialErrorOOMToAbslStatus) { TEST(ErrorUtilsTest, GrpcSpecialErrorOOMToAbslStatus) {
absl::Status status = grpc_error_to_absl_status(GRPC_ERROR_OOM); absl::Status status = grpc_error_to_absl_status(GRPC_ERROR_OOM);
ASSERT_TRUE(absl::IsResourceExhausted(status)); ASSERT_TRUE(absl::IsResourceExhausted(status));
ASSERT_EQ(status.message(), "RESOURCE_EXHAUSTED"); ASSERT_EQ(status.message(), "Out of memory");
} }
// ---- Ordinary statuses ---- // ---- Ordinary statuses ----

2
test/cpp/end2end/end2end_test.cc
Unescape View File

@ -1630,7 +1630,7 @@ TEST_P(ProxyEnd2endTest, ClientCancelsRpc) {
Status s = stub_->Echo(&context, request, &response); Status s = stub_->Echo(&context, request, &response);
cancel_thread.join(); cancel_thread.join();
EXPECT_EQ(StatusCode::CANCELLED, s.error_code()); EXPECT_EQ(StatusCode::CANCELLED, s.error_code());
EXPECT_EQ(s.error_message(), "CANCELLED"); EXPECT_EQ(s.error_message(), "Cancelled");
} }
// Server cancels rpc after 1ms // Server cancels rpc after 1ms

28
test/cpp/microbenchmarks/bm_error.cc
Unescape View File

@ -28,15 +28,11 @@
#include "test/cpp/microbenchmarks/helpers.h" #include "test/cpp/microbenchmarks/helpers.h"
#include "test/cpp/util/test_config.h" #include "test/cpp/util/test_config.h"
class ErrorHandleHolder { class ErrorDeleter {
public: public:
explicit ErrorHandleHolder(grpc_error_handle error) : error_(error) {} void operator()(grpc_error_handle error) { GRPC_ERROR_UNREF(error); }
~ErrorHandleHolder() { GRPC_ERROR_UNREF(error_); }
const grpc_error_handle& get() const { return error_; }
private:
grpc_error_handle error_;
}; };
typedef std::unique_ptr<grpc_error, ErrorDeleter> ErrorPtr;
static void BM_ErrorCreateFromStatic(benchmark::State& state) { static void BM_ErrorCreateFromStatic(benchmark::State& state) {
TrackCounters track_counters; TrackCounters track_counters;
@ -136,7 +132,7 @@ BENCHMARK(BM_ErrorGetIntFromNoError);
static void BM_ErrorGetMissingInt(benchmark::State& state) { static void BM_ErrorGetMissingInt(benchmark::State& state) {
TrackCounters track_counters; TrackCounters track_counters;
ErrorHandleHolder error(grpc_error_set_int( ErrorPtr error(grpc_error_set_int(
GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_INDEX, 1)); GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_INDEX, 1));
for (auto _ : state) { for (auto _ : state) {
intptr_t value; intptr_t value;
@ -148,7 +144,7 @@ BENCHMARK(BM_ErrorGetMissingInt);
static void BM_ErrorGetPresentInt(benchmark::State& state) { static void BM_ErrorGetPresentInt(benchmark::State& state) {
TrackCounters track_counters; TrackCounters track_counters;
ErrorHandleHolder error(grpc_error_set_int( ErrorPtr error(grpc_error_set_int(
GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_OFFSET, 1)); GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_OFFSET, 1));
for (auto _ : state) { for (auto _ : state) {
intptr_t value; intptr_t value;
@ -184,7 +180,7 @@ class SimpleError {
private: private:
const grpc_millis deadline_ = GRPC_MILLIS_INF_FUTURE; const grpc_millis deadline_ = GRPC_MILLIS_INF_FUTURE;
ErrorHandleHolder error_{GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error")}; ErrorPtr error_{GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error")};
}; };
class ErrorWithGrpcStatus { class ErrorWithGrpcStatus {
@ -194,7 +190,7 @@ class ErrorWithGrpcStatus {
private: private:
const grpc_millis deadline_ = GRPC_MILLIS_INF_FUTURE; const grpc_millis deadline_ = GRPC_MILLIS_INF_FUTURE;
ErrorHandleHolder error_{grpc_error_set_int( ErrorPtr error_{grpc_error_set_int(
GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_GRPC_STATUS, GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_GRPC_STATUS,
GRPC_STATUS_UNIMPLEMENTED)}; GRPC_STATUS_UNIMPLEMENTED)};
}; };
@ -206,7 +202,7 @@ class ErrorWithHttpError {
private: private:
const grpc_millis deadline_ = GRPC_MILLIS_INF_FUTURE; const grpc_millis deadline_ = GRPC_MILLIS_INF_FUTURE;
ErrorHandleHolder error_{grpc_error_set_int( ErrorPtr error_{grpc_error_set_int(
GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_HTTP2_ERROR, GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_HTTP2_ERROR,
GRPC_HTTP2_COMPRESSION_ERROR)}; GRPC_HTTP2_COMPRESSION_ERROR)};
}; };
@ -218,11 +214,11 @@ class ErrorWithNestedGrpcStatus {
private: private:
const grpc_millis deadline_ = GRPC_MILLIS_INF_FUTURE; const grpc_millis deadline_ = GRPC_MILLIS_INF_FUTURE;
ErrorHandleHolder nested_error_{grpc_error_set_int( ErrorPtr nested_error_{grpc_error_set_int(
GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_GRPC_STATUS, GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_GRPC_STATUS,
GRPC_STATUS_UNIMPLEMENTED)}; GRPC_STATUS_UNIMPLEMENTED)};
grpc_error_handle nested_errors_[1] = {nested_error_.get()}; grpc_error_handle nested_errors_[1] = {nested_error_.get()};
ErrorHandleHolder error_{GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( ErrorPtr error_{GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING(
"Error", nested_errors_, 1)}; "Error", nested_errors_, 1)};
}; };
@ -231,7 +227,7 @@ static void BM_ErrorStringOnNewError(benchmark::State& state) {
TrackCounters track_counters; TrackCounters track_counters;
for (auto _ : state) { for (auto _ : state) {
Fixture fixture; Fixture fixture;
grpc_error_std_string(fixture.error()); grpc_error_string(fixture.error());
} }
track_counters.Finish(state); track_counters.Finish(state);
} }
@ -241,7 +237,7 @@ static void BM_ErrorStringRepeatedly(benchmark::State& state) {
TrackCounters track_counters; TrackCounters track_counters;
Fixture fixture; Fixture fixture;
for (auto _ : state) { for (auto _ : state) {
grpc_error_std_string(fixture.error()); grpc_error_string(fixture.error());
} }
track_counters.Finish(state); track_counters.Finish(state);
} }

Write Preview
Loading…
Cancel
Save