From 2d0d1775a9208fa8f137d2e3546a3f3701d895ea Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Wed, 15 Jun 2022 21:41:55 -0700 Subject: [PATCH] Revert "Enable GRPC_ERROR_IS_ABSEIL_STATUS (#29869)" (#30031) This reverts commit c6058b5e6b0f53d45794c01320b1acdaba232772. --- BUILD | 15 +++------------ bazel/grpc_build_system.bzl | 2 +- include/grpc/impl/codegen/port_platform.h | 7 +++---- .../ext/filters/client_channel/backup_poller.cc | 2 -- .../ext/filters/client_channel/dynamic_filters.cc | 2 -- src/core/ext/filters/deadline/deadline_filter.cc | 1 - .../message_compress/message_compress_filter.cc | 1 - .../message_compress/message_decompress_filter.cc | 1 - .../chttp2/transport/chttp2_transport.cc | 1 - .../ext/transport/chttp2/transport/flow_control.h | 2 -- .../ext/transport/chttp2/transport/frame_data.cc | 1 - .../transport/chttp2/transport/hpack_parser.cc | 1 - .../ext/transport/chttp2/transport/parsing.cc | 1 - .../cronet/transport/cronet_transport.cc | 1 - src/core/lib/surface/completion_queue.cc | 1 - src/core/lib/transport/byte_stream.cc | 2 -- src/core/lib/transport/error_utils.cc | 8 +------- src/cpp/client/secure_credentials.cc | 2 +- .../grpcio_tests/tests/unit/_rpc_part_1_test.py | 6 ++++-- src/ruby/lib/grpc/errors.rb | 2 +- test/cpp/server/server_request_call_test.cc | 10 +--------- .../linux/grpc_bazel_build_in_docker.sh | 2 +- 22 files changed, 16 insertions(+), 55 deletions(-) diff --git a/BUILD b/BUILD index 4d96c66378a..d0a6c546f2b 100644 --- a/BUILD +++ b/BUILD @@ -144,8 +144,8 @@ config_setting( ) config_setting( - name = "disable_use_abseil_status", - values = {"define": "use_abseil_status=false"}, + name = "use_abseil_status", + values = {"define": "use_abseil_status=true"}, ) python_config_settings() @@ -2716,7 +2716,6 @@ grpc_cc_library( "slice_buffer", "slice_refcount", "sockaddr_utils", - "status_helper", "table", "thread_quota", "time", @@ -3269,10 +3268,7 @@ grpc_cc_library( hdrs = [ "src/core/ext/filters/deadline/deadline_filter.h", ], - external_deps = [ - "absl/status", - "absl/types:optional", - ], + external_deps = ["absl/types:optional"], language = "c++", tags = ["grpc-autodeps"], deps = [ @@ -4724,7 +4720,6 @@ grpc_cc_library( ], external_deps = [ "absl/memory", - "absl/status", "absl/status:statusor", "absl/strings", ], @@ -6062,7 +6057,6 @@ grpc_cc_library( "resource_quota_trace", "slice", "slice_refcount", - "status_helper", "time", "uri_parser", "useful", @@ -6286,7 +6280,6 @@ grpc_cc_library( hdrs = GRPCXX_HDRS, external_deps = [ "absl/base:core_headers", - "absl/status", "absl/strings", "absl/synchronization", "absl/memory", @@ -6330,7 +6323,6 @@ grpc_cc_library( hdrs = GRPCXX_HDRS, external_deps = [ "absl/base:core_headers", - "absl/status", "absl/strings", "absl/synchronization", "absl/memory", @@ -6588,7 +6580,6 @@ grpc_cc_library( ], external_deps = [ "absl/base:core_headers", - "absl/status", "absl/time", "absl/types:optional", "upb_lib", diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index a51f1677c08..c09d2bd3921 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -178,7 +178,7 @@ def grpc_cc_library( "//conditions:default": [], }) + select({ - "//:disable_use_abseil_status": ["GRPC_ERROR_IS_NOT_ABSEIL_STATUS=1"], + "//:use_abseil_status": ["GRPC_ERROR_IS_ABSEIL_STATUS=1"], "//conditions:default": [], }), hdrs = hdrs + public_hdrs, diff --git a/include/grpc/impl/codegen/port_platform.h b/include/grpc/impl/codegen/port_platform.h index ae0e35cfb7f..a4bd31d1524 100644 --- a/include/grpc/impl/codegen/port_platform.h +++ b/include/grpc/impl/codegen/port_platform.h @@ -49,11 +49,10 @@ #endif // GPR_ABSEIL_SYNC /* - * Defines GRPC_ERROR_IS_NOT_ABSEIL_STATUS to not use absl::Status for - * grpc_error_handle. This is a temporary knob for migration process. + * Defines GRPC_ERROR_IS_ABSEIL_STATUS to use absl::Status for grpc_error_handle */ -#ifndef GRPC_ERROR_IS_NOT_ABSEIL_STATUS -#define GRPC_ERROR_IS_ABSEIL_STATUS 1 +#ifndef GRPC_ERROR_IS_ABSEIL_STATUS +// #define GRPC_ERROR_IS_ABSEIL_STATUS 1 #endif /* Get windows.h included everywhere (we need it) */ diff --git a/src/core/ext/filters/client_channel/backup_poller.cc b/src/core/ext/filters/client_channel/backup_poller.cc index 1ee9d136ab3..394d075079a 100644 --- a/src/core/ext/filters/client_channel/backup_poller.cc +++ b/src/core/ext/filters/client_channel/backup_poller.cc @@ -22,8 +22,6 @@ #include -#include "absl/status/status.h" - #include #include #include diff --git a/src/core/ext/filters/client_channel/dynamic_filters.cc b/src/core/ext/filters/client_channel/dynamic_filters.cc index be1d2589f5e..f312ec7c6da 100644 --- a/src/core/ext/filters/client_channel/dynamic_filters.cc +++ b/src/core/ext/filters/client_channel/dynamic_filters.cc @@ -24,8 +24,6 @@ #include #include -#include "absl/status/status.h" - #include #include diff --git a/src/core/ext/filters/deadline/deadline_filter.cc b/src/core/ext/filters/deadline/deadline_filter.cc index 2e34c08c53c..369cc7fd218 100644 --- a/src/core/ext/filters/deadline/deadline_filter.cc +++ b/src/core/ext/filters/deadline/deadline_filter.cc @@ -20,7 +20,6 @@ #include -#include "absl/status/status.h" #include "absl/types/optional.h" #include diff --git a/src/core/ext/filters/http/message_compress/message_compress_filter.cc b/src/core/ext/filters/http/message_compress/message_compress_filter.cc index e31f34ddb7c..aa3eb7cc313 100644 --- a/src/core/ext/filters/http/message_compress/message_compress_filter.cc +++ b/src/core/ext/filters/http/message_compress/message_compress_filter.cc @@ -28,7 +28,6 @@ #include #include "absl/meta/type_traits.h" -#include "absl/status/status.h" #include "absl/types/optional.h" #include diff --git a/src/core/ext/filters/http/message_compress/message_decompress_filter.cc b/src/core/ext/filters/http/message_compress/message_decompress_filter.cc index 277989004df..508d58f654d 100644 --- a/src/core/ext/filters/http/message_compress/message_decompress_filter.cc +++ b/src/core/ext/filters/http/message_compress/message_decompress_filter.cc @@ -27,7 +27,6 @@ #include #include -#include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/types/optional.h" diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index e17a0b36bff..b8ab5156bab 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -65,7 +65,6 @@ #include "src/core/lib/gprpp/manual_constructor.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted.h" -#include "src/core/lib/gprpp/status_helper.h" #include "src/core/lib/gprpp/time.h" #include "src/core/lib/http/parser.h" #include "src/core/lib/iomgr/combiner.h" diff --git a/src/core/ext/transport/chttp2/transport/flow_control.h b/src/core/ext/transport/chttp2/transport/flow_control.h index c50aa5f1e23..185f18b38ca 100644 --- a/src/core/ext/transport/chttp2/transport/flow_control.h +++ b/src/core/ext/transport/chttp2/transport/flow_control.h @@ -26,8 +26,6 @@ #include -#include "absl/status/status.h" - #include "src/core/ext/transport/chttp2/transport/http2_settings.h" #include "src/core/lib/debug/trace.h" #include "src/core/lib/gprpp/time.h" diff --git a/src/core/ext/transport/chttp2/transport/frame_data.cc b/src/core/ext/transport/chttp2/transport/frame_data.cc index 8cb4354f37f..b3eb18c1d4b 100644 --- a/src/core/ext/transport/chttp2/transport/frame_data.cc +++ b/src/core/ext/transport/chttp2/transport/frame_data.cc @@ -23,7 +23,6 @@ #include #include "absl/base/attributes.h" -#include "absl/status/status.h" #include "absl/strings/str_format.h" #include diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc index 5354338cba3..e73d3753ca6 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -31,7 +31,6 @@ #include #include "absl/base/attributes.h" -#include "absl/status/status.h" #include "absl/strings/match.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" diff --git a/src/core/ext/transport/chttp2/transport/parsing.cc b/src/core/ext/transport/chttp2/transport/parsing.cc index d91af3801be..21577204811 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.cc +++ b/src/core/ext/transport/chttp2/transport/parsing.cc @@ -24,7 +24,6 @@ #include #include "absl/base/attributes.h" -#include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index 2fe5fdf311e..45ddfcc8f02 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -29,7 +29,6 @@ #include #include -#include "absl/status/status.h" #include "absl/strings/match.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" diff --git a/src/core/lib/surface/completion_queue.cc b/src/core/lib/surface/completion_queue.cc index 2e9f7a5be05..c4255acaca0 100644 --- a/src/core/lib/surface/completion_queue.cc +++ b/src/core/lib/surface/completion_queue.cc @@ -29,7 +29,6 @@ #include #include -#include "absl/status/status.h" #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" diff --git a/src/core/lib/transport/byte_stream.cc b/src/core/lib/transport/byte_stream.cc index ac49481f4dc..3b909182f69 100644 --- a/src/core/lib/transport/byte_stream.cc +++ b/src/core/lib/transport/byte_stream.cc @@ -23,8 +23,6 @@ #include #include -#include "absl/status/status.h" - #include #include diff --git a/src/core/lib/transport/error_utils.cc b/src/core/lib/transport/error_utils.cc index 4ae57c03b8d..49a964ab165 100644 --- a/src/core/lib/transport/error_utils.cc +++ b/src/core/lib/transport/error_utils.cc @@ -22,18 +22,12 @@ #include -#include - #include "absl/strings/string_view.h" #include -#include "src/core/lib/gprpp/status_helper.h" -#include "src/core/lib/transport/status_conversion.h" - -#ifndef GRPC_ERROR_IS_ABSEIL_STATUS #include "src/core/lib/iomgr/error_internal.h" -#endif +#include "src/core/lib/transport/status_conversion.h" static grpc_error_handle recursively_find_error_with_field( grpc_error_handle error, grpc_error_ints which) { diff --git a/src/cpp/client/secure_credentials.cc b/src/cpp/client/secure_credentials.cc index 4db969b6b8f..012a8f7c62e 100644 --- a/src/cpp/client/secure_credentials.cc +++ b/src/cpp/client/secure_credentials.cc @@ -39,7 +39,7 @@ #include #include #include - +// TODO(yashykt): We shouldn't be including "src/core" headers. #include "src/core/lib/gpr/env.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/error.h" diff --git a/src/python/grpcio_tests/tests/unit/_rpc_part_1_test.py b/src/python/grpcio_tests/tests/unit/_rpc_part_1_test.py index c3354a621a1..1c0886cc069 100644 --- a/src/python/grpcio_tests/tests/unit/_rpc_part_1_test.py +++ b/src/python/grpcio_tests/tests/unit/_rpc_part_1_test.py @@ -114,8 +114,10 @@ class RPCPart1Test(BaseRPCTest, unittest.TestCase): # sanity checks on to make sure returned string contains default members # of the error debug_error_string = exception_context.exception.debug_error_string() - self.assertIn('grpc_status', debug_error_string) - self.assertIn('grpc_message', debug_error_string) + self.assertIn('created', debug_error_string) + self.assertIn('description', debug_error_string) + self.assertIn('file', debug_error_string) + self.assertIn('file_line', debug_error_string) def testFailedUnaryRequestFutureUnaryResponse(self): request = b'\x37\x17' diff --git a/src/ruby/lib/grpc/errors.rb b/src/ruby/lib/grpc/errors.rb index 1fceb6ba447..1ab0811d327 100644 --- a/src/ruby/lib/grpc/errors.rb +++ b/src/ruby/lib/grpc/errors.rb @@ -43,7 +43,7 @@ module GRPC debug_error_string = nil) exception_message = "#{code}:#{details}" if debug_error_string - exception_message += ". debug_error_string:{#{debug_error_string}}" + exception_message += ". debug_error_string:#{debug_error_string}" end super(exception_message) @code = code diff --git a/test/cpp/server/server_request_call_test.cc b/test/cpp/server/server_request_call_test.cc index b9875efd08c..467188b2e72 100644 --- a/test/cpp/server/server_request_call_test.cc +++ b/test/cpp/server/server_request_call_test.cc @@ -135,15 +135,7 @@ TEST(ServerRequestCallTest, ShortDeadlineDoesNotCauseOkayFalse) { ctx.set_deadline(std::chrono::system_clock::now() + std::chrono::milliseconds(1)); grpc::Status status = stub->Echo(&ctx, request, &response); - if (i == 0 && status.error_code() == grpc::OK) { - // This first call may take more time to trigger a deadline excceded - // because code needs various initiations so it can get OK instead of - // DEADLINE_EXCEEDED. Server-side delay isn't affected so a call - // would be complete before the client-side deadline timer is fired. - // To prevent this case, OK is also accepted for the first call only. - } else { - EXPECT_EQ(StatusCode::DEADLINE_EXCEEDED, status.error_code()); - } + EXPECT_EQ(StatusCode::DEADLINE_EXCEEDED, status.error_code()); gpr_log(GPR_INFO, "Success."); } gpr_log(GPR_INFO, "Done sending RPCs."); diff --git a/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh b/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh index 86664bab613..71b585a8fc1 100755 --- a/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh +++ b/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh @@ -35,7 +35,7 @@ python3 tools/run_tests/python_utils/bazel_report_helper.py --report_path bazel_ bazel_build_with_abseil_status/bazel_wrapper \ --bazelrc=tools/remote_build/include/test_locally_with_resultstore_results.bazelrc \ build \ - --define=use_strict_warning=true --define=use_abseil_status=false \ + --define=use_strict_warning=true --define=use_abseil_status=true \ -- \ //src/core/... \ //src/compiler/... \