From 1c6634ac44f6c1fb2b550fe2ef27e3f85373a482 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Fri, 1 Oct 2021 18:58:47 -0700 Subject: [PATCH] Changed grpc_error_get|set_str to use std string instead of slice (#27466) * Changed grpc_error_get|set_str to use std string * Fix init order in tests with gtest * Undo gtest-tify credentials_test --- BUILD | 1 + CMakeLists.txt | 66 ++++--- build_autogenerated.yaml | 23 +-- .../filters/client_channel/client_channel.cc | 5 +- .../resolver/dns/c_ares/grpc_ares_wrapper.cc | 6 +- .../filters/http/client/http_client_filter.cc | 5 +- .../filters/http/server/http_server_filter.cc | 38 ++-- .../chttp2/transport/chttp2_transport.cc | 46 ++--- .../transport/chttp2/transport/frame_data.cc | 8 +- .../chttp2/transport/frame_goaway.cc | 3 +- .../chttp2/transport/frame_rst_stream.cc | 3 +- .../ext/transport/chttp2/transport/internal.h | 2 +- src/core/lib/http/httpcli.cc | 3 +- src/core/lib/iomgr/endpoint_cfstream.cc | 3 +- src/core/lib/iomgr/error.cc | 38 ++-- src/core/lib/iomgr/error.h | 7 +- src/core/lib/iomgr/load_file.cc | 4 +- src/core/lib/iomgr/resolve_address_posix.cc | 12 +- .../lib/iomgr/socket_utils_common_posix.cc | 3 +- src/core/lib/iomgr/tcp_client_posix.cc | 25 +-- src/core/lib/iomgr/tcp_client_windows.cc | 3 +- src/core/lib/iomgr/tcp_posix.cc | 3 +- src/core/lib/iomgr/tcp_server_windows.cc | 9 +- .../google_default_credentials.cc | 2 +- src/core/lib/security/transport/tsi_error.cc | 8 +- src/core/lib/surface/call.cc | 23 ++- src/core/lib/surface/lame_client.cc | 3 +- src/core/lib/surface/validate_metadata.cc | 7 +- src/core/lib/transport/error_utils.cc | 32 ++-- src/core/lib/transport/error_utils.h | 2 +- src/core/lib/transport/metadata_batch.cc | 4 +- .../core/bad_client/tests/bad_streaming_id.cc | 2 +- test/core/bad_client/tests/out_of_bounds.cc | 2 +- test/core/bad_client/tests/unknown_frame.cc | 2 +- test/core/iomgr/BUILD | 3 + test/core/iomgr/error_test.cc | 170 ++++++++---------- test/core/iomgr/work_serializer_test.cc | 2 +- test/core/security/aws_request_signer_test.cc | 15 +- test/core/security/credentials_test.cc | 25 ++- .../grpc_tls_certificate_distributor_test.cc | 9 +- .../grpc_tls_certificate_provider_test.cc | 9 +- test/core/transport/error_utils_test.cc | 6 +- test/cpp/microbenchmarks/bm_error.cc | 11 +- tools/run_tests/generated/tests.json | 48 ++--- 44 files changed, 322 insertions(+), 379 deletions(-) diff --git a/BUILD b/BUILD index 8b0d85dbf32..1290b38f1ea 100644 --- a/BUILD +++ b/BUILD @@ -1400,6 +1400,7 @@ grpc_cc_library( "grpc_codegen", "grpc_trace", "slice", + "slice_refcount", "useful", ], ) diff --git a/CMakeLists.txt b/CMakeLists.txt index 38f0363914c..d6e9cf70d38 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -596,7 +596,6 @@ if(gRPC_BUILD_TESTS) endif() add_dependencies(buildtests_c endpoint_pair_test) add_dependencies(buildtests_c env_test) - add_dependencies(buildtests_c error_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_c ev_epollex_linux_test) endif() @@ -861,6 +860,7 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx end2end_test) add_dependencies(buildtests_cxx endpoint_config_test) add_dependencies(buildtests_cxx error_details_test) + add_dependencies(buildtests_cxx error_test) add_dependencies(buildtests_cxx error_utils_test) add_dependencies(buildtests_cxx evaluate_args_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) @@ -5270,34 +5270,6 @@ target_link_libraries(env_test ) -endif() -if(gRPC_BUILD_TESTS) - -add_executable(error_test - test/core/iomgr/endpoint_tests.cc - test/core/iomgr/error_test.cc -) - -target_include_directories(error_test - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} - ${CMAKE_CURRENT_SOURCE_DIR}/include - ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} - ${_gRPC_RE2_INCLUDE_DIR} - ${_gRPC_SSL_INCLUDE_DIR} - ${_gRPC_UPB_GENERATED_DIR} - ${_gRPC_UPB_GRPC_GENERATED_DIR} - ${_gRPC_UPB_INCLUDE_DIR} - ${_gRPC_XXHASH_INCLUDE_DIR} - ${_gRPC_ZLIB_INCLUDE_DIR} -) - -target_link_libraries(error_test - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util -) - - endif() if(gRPC_BUILD_TESTS) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) @@ -11050,6 +11022,42 @@ target_link_libraries(error_details_test ) +endif() +if(gRPC_BUILD_TESTS) + +add_executable(error_test + test/core/iomgr/endpoint_tests.cc + test/core/iomgr/error_test.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + +target_include_directories(error_test + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/include + ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + ${_gRPC_RE2_INCLUDE_DIR} + ${_gRPC_SSL_INCLUDE_DIR} + ${_gRPC_UPB_GENERATED_DIR} + ${_gRPC_UPB_GRPC_GENERATED_DIR} + ${_gRPC_UPB_INCLUDE_DIR} + ${_gRPC_XXHASH_INCLUDE_DIR} + ${_gRPC_ZLIB_INCLUDE_DIR} + third_party/googletest/googletest/include + third_party/googletest/googletest + third_party/googletest/googlemock/include + third_party/googletest/googlemock + ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(error_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc_test_util +) + + endif() if(gRPC_BUILD_TESTS) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index e39714a47d7..fc562d201ce 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -3332,17 +3332,6 @@ targets: deps: - grpc_test_util uses_polling: false -- name: error_test - build: test - language: c - headers: - - test/core/iomgr/endpoint_tests.h - src: - - test/core/iomgr/endpoint_tests.cc - - test/core/iomgr/error_test.cc - deps: - - grpc_test_util - uses_polling: false - name: ev_epollex_linux_test build: test language: c @@ -5782,6 +5771,18 @@ targets: deps: - grpc++_error_details - grpc_test_util +- name: error_test + gtest: true + build: test + language: c++ + headers: + - test/core/iomgr/endpoint_tests.h + src: + - test/core/iomgr/endpoint_tests.cc + - test/core/iomgr/error_test.cc + deps: + - grpc_test_util + uses_polling: false - name: error_utils_test gtest: true build: test diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 4ffd76a5d00..ef90db17b6a 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -2904,11 +2904,10 @@ void ClientChannel::LoadBalancedCall::RecvTrailingMetadataReady( if (error != GRPC_ERROR_NONE) { // Get status from error. grpc_status_code code; - grpc_slice message = grpc_empty_slice(); + std::string message; grpc_error_get_status(error, self->deadline_, &code, &message, /*http_error=*/nullptr, /*error_string=*/nullptr); - status = absl::Status(static_cast(code), - StringViewFromSlice(message)); + status = absl::Status(static_cast(code), message); } else { // Get status from headers. const auto& fields = self->recv_trailing_metadata_->legacy_index()->named; diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc index 045937e59c5..39330c8977f 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc @@ -841,13 +841,13 @@ void grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( if (host.empty()) { error = grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("unparseable host:port"), - GRPC_ERROR_STR_TARGET_ADDRESS, grpc_slice_from_copied_string(name)); + GRPC_ERROR_STR_TARGET_ADDRESS, name); goto error_cleanup; } else if (port.empty()) { if (default_port == nullptr) { error = grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("no port in name"), - GRPC_ERROR_STR_TARGET_ADDRESS, grpc_slice_from_copied_string(name)); + GRPC_ERROR_STR_TARGET_ADDRESS, name); goto error_cleanup; } port = default_port; @@ -879,7 +879,7 @@ void grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( } else { error = grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("cannot parse authority"), - GRPC_ERROR_STR_TARGET_ADDRESS, grpc_slice_from_copied_string(name)); + GRPC_ERROR_STR_TARGET_ADDRESS, name); goto error_cleanup; } int status = diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index 29c41843d1f..29ea067cc3c 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -131,11 +131,10 @@ static grpc_error_handle client_filter_incoming_metadata( grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Received http2 :status header with non-200 OK status"), - GRPC_ERROR_STR_VALUE, grpc_slice_from_copied_string(val)), + GRPC_ERROR_STR_VALUE, val), GRPC_ERROR_INT_GRPC_STATUS, grpc_http2_status_to_grpc_status(atoi(val))), - GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_cpp_string(std::move(msg))); + GRPC_ERROR_STR_GRPC_MESSAGE, msg); gpr_free(val); return e; } diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index c5fb286cd20..f70d0fc4671 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -187,11 +187,10 @@ static grpc_error_handle hs_filter_incoming_metadata(grpc_call_element* elem, } b->Remove(GRPC_BATCH_METHOD); } else { - hs_add_error( - error_name, &error, - grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Missing header"), - GRPC_ERROR_STR_KEY, grpc_slice_from_static_string(":method"))); + hs_add_error(error_name, &error, + grpc_error_set_str( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("Missing header"), + GRPC_ERROR_STR_KEY, ":method")); } if (b->legacy_index()->named.te != nullptr) { @@ -207,7 +206,7 @@ static grpc_error_handle hs_filter_incoming_metadata(grpc_call_element* elem, hs_add_error(error_name, &error, grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("Missing header"), - GRPC_ERROR_STR_KEY, grpc_slice_from_static_string("te"))); + GRPC_ERROR_STR_KEY, "te")); } if (b->legacy_index()->named.scheme != nullptr) { @@ -224,11 +223,10 @@ static grpc_error_handle hs_filter_incoming_metadata(grpc_call_element* elem, } b->Remove(GRPC_BATCH_SCHEME); } else { - hs_add_error( - error_name, &error, - grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Missing header"), - GRPC_ERROR_STR_KEY, grpc_slice_from_static_string(":scheme"))); + hs_add_error(error_name, &error, + grpc_error_set_str( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("Missing header"), + GRPC_ERROR_STR_KEY, ":scheme")); } if (b->legacy_index()->named.content_type != nullptr) { @@ -265,11 +263,10 @@ static grpc_error_handle hs_filter_incoming_metadata(grpc_call_element* elem, } if (b->legacy_index()->named.path == nullptr) { - hs_add_error( - error_name, &error, - grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Missing header"), - GRPC_ERROR_STR_KEY, grpc_slice_from_static_string(":path"))); + hs_add_error(error_name, &error, + grpc_error_set_str( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("Missing header"), + GRPC_ERROR_STR_KEY, ":path")); } else if (*calld->recv_initial_metadata_flags & GRPC_INITIAL_METADATA_CACHEABLE_REQUEST) { /* We have a cacheable request made with GET verb. The path contains the @@ -327,11 +324,10 @@ static grpc_error_handle hs_filter_incoming_metadata(grpc_call_element* elem, } if (b->legacy_index()->named.authority == nullptr) { - hs_add_error( - error_name, &error, - grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Missing header"), - GRPC_ERROR_STR_KEY, grpc_slice_from_static_string(":authority"))); + hs_add_error(error_name, &error, + grpc_error_set_str( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("Missing header"), + GRPC_ERROR_STR_KEY, ":authority")); } channel_data* chand = static_cast(elem->channel_data); diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 0a26c7d6d4b..be0faff3afa 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -1078,7 +1078,7 @@ static void queue_setting_update(grpc_chttp2_transport* t, void grpc_chttp2_add_incoming_goaway(grpc_chttp2_transport* t, uint32_t goaway_error, uint32_t last_stream_id, - const grpc_slice& goaway_text) { + absl::string_view goaway_text) { // Discard the error from a previous goaway frame (if any) if (t->goaway_error != GRPC_ERROR_NONE) { GRPC_ERROR_UNREF(t->goaway_error); @@ -1107,7 +1107,7 @@ void grpc_chttp2_add_incoming_goaway(grpc_chttp2_transport* t, // for new connections on that channel. if (GPR_UNLIKELY(t->is_client && goaway_error == GRPC_HTTP2_ENHANCE_YOUR_CALM && - grpc_slice_str_cmp(goaway_text, "too_many_pings") == 0)) { + goaway_text == "too_many_pings")) { gpr_log(GPR_ERROR, "Received a GOAWAY with error code ENHANCE_YOUR_CALM and debug " "data equal to \"too_many_pings\""); @@ -1230,9 +1230,9 @@ void grpc_chttp2_complete_closure_step(grpc_chttp2_transport* t, if (closure->error_data.error == GRPC_ERROR_NONE) { closure->error_data.error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Error in HTTP transport completing operation"); - closure->error_data.error = grpc_error_set_str( - closure->error_data.error, GRPC_ERROR_STR_TARGET_ADDRESS, - grpc_slice_from_copied_string(t->peer_string.c_str())); + closure->error_data.error = + grpc_error_set_str(closure->error_data.error, + GRPC_ERROR_STR_TARGET_ADDRESS, t->peer_string); } closure->error_data.error = grpc_error_add_child(closure->error_data.error, error); @@ -1752,12 +1752,12 @@ static void send_goaway(grpc_chttp2_transport* t, grpc_error_handle error) { grpc_error_std_string(error).c_str()); t->sent_goaway_state = GRPC_CHTTP2_GOAWAY_SEND_SCHEDULED; grpc_http2_error_code http_error; - grpc_slice slice; - grpc_error_get_status(error, GRPC_MILLIS_INF_FUTURE, nullptr, &slice, + std::string message; + grpc_error_get_status(error, GRPC_MILLIS_INF_FUTURE, nullptr, &message, &http_error, nullptr); - grpc_chttp2_goaway_append(t->last_new_stream_id, - static_cast(http_error), - grpc_slice_ref_internal(slice), &t->qbuf); + grpc_chttp2_goaway_append( + t->last_new_stream_id, static_cast(http_error), + grpc_slice_from_cpp_string(std::move(message)), &t->qbuf); grpc_chttp2_initiate_write(t, GRPC_CHTTP2_INITIATE_WRITE_GOAWAY_SENT); GRPC_ERROR_UNREF(error); } @@ -2091,8 +2091,9 @@ void grpc_chttp2_cancel_stream(grpc_chttp2_transport* t, grpc_chttp2_stream* s, void grpc_chttp2_fake_status(grpc_chttp2_transport* t, grpc_chttp2_stream* s, grpc_error_handle error) { grpc_status_code status; - grpc_slice slice; - grpc_error_get_status(error, s->deadline, &status, &slice, nullptr, nullptr); + std::string message; + grpc_error_get_status(error, s->deadline, &status, &message, nullptr, + nullptr); if (status != GRPC_STATUS_OK) { s->seen_error = true; } @@ -2110,12 +2111,12 @@ void grpc_chttp2_fake_status(grpc_chttp2_transport* t, grpc_chttp2_stream* s, grpc_chttp2_incoming_metadata_buffer_replace_or_add( &s->trailing_metadata_buffer, GRPC_MDSTR_GRPC_STATUS, grpc_core::UnmanagedMemorySlice(status_string))); - if (!GRPC_SLICE_IS_EMPTY(slice)) { - GRPC_LOG_IF_ERROR( - "add_status_message", - grpc_chttp2_incoming_metadata_buffer_replace_or_add( - &s->trailing_metadata_buffer, GRPC_MDSTR_GRPC_MESSAGE, - grpc_slice_ref_internal(slice))); + if (!message.empty()) { + grpc_slice message_slice = grpc_slice_from_cpp_string(std::move(message)); + GRPC_LOG_IF_ERROR("add_status_message", + grpc_chttp2_incoming_metadata_buffer_replace_or_add( + &s->trailing_metadata_buffer, + GRPC_MDSTR_GRPC_MESSAGE, message_slice)); } s->published_metadata[1] = GRPC_METADATA_SYNTHESIZED_FROM_FAKE; grpc_chttp2_maybe_complete_recv_trailing_metadata(t, s); @@ -2255,8 +2256,8 @@ static void close_from_api(grpc_chttp2_transport* t, grpc_chttp2_stream* s, uint8_t* p; uint32_t len = 0; grpc_status_code grpc_status; - grpc_slice slice; - grpc_error_get_status(error, s->deadline, &grpc_status, &slice, nullptr, + std::string message; + grpc_error_get_status(error, s->deadline, &grpc_status, &message, nullptr, nullptr); GPR_ASSERT(grpc_status >= 0 && (int)grpc_status < 100); @@ -2349,7 +2350,7 @@ static void close_from_api(grpc_chttp2_transport* t, grpc_chttp2_stream* s, GPR_ASSERT(p == GRPC_SLICE_END_PTR(status_hdr)); len += static_cast GRPC_SLICE_LENGTH(status_hdr); - size_t msg_len = GRPC_SLICE_LENGTH(slice); + size_t msg_len = message.length(); GPR_ASSERT(msg_len <= UINT32_MAX); grpc_core::VarintWriter<1> msg_len_writer(msg_len); message_pfx = GRPC_SLICE_MALLOC(14 + msg_len_writer.length()); @@ -2394,7 +2395,8 @@ static void close_from_api(grpc_chttp2_transport* t, grpc_chttp2_stream* s, } grpc_slice_buffer_add(&t->qbuf, status_hdr); grpc_slice_buffer_add(&t->qbuf, message_pfx); - grpc_slice_buffer_add(&t->qbuf, grpc_slice_ref_internal(slice)); + grpc_slice_buffer_add(&t->qbuf, + grpc_slice_from_cpp_string(std::move(message))); grpc_chttp2_reset_ping_clock(t); grpc_chttp2_add_rst_stream_to_next_write(t, s->id, GRPC_HTTP2_NO_ERROR, &s->stats.outgoing); diff --git a/src/core/ext/transport/chttp2/transport/frame_data.cc b/src/core/ext/transport/chttp2/transport/frame_data.cc index 88de4670a74..91a3f2d766d 100644 --- a/src/core/ext/transport/chttp2/transport/frame_data.cc +++ b/src/core/ext/transport/chttp2/transport/frame_data.cc @@ -133,10 +133,10 @@ grpc_error_handle grpc_deframe_unprocessed_incoming_frames( absl::StrFormat("Bad GRPC frame type 0x%02x", p->frame_type)); p->error = grpc_error_set_int(p->error, GRPC_ERROR_INT_STREAM_ID, static_cast(s->id)); - p->error = grpc_error_set_str( - p->error, GRPC_ERROR_STR_RAW_BYTES, - grpc_slice_from_moved_string(grpc_core::UniquePtr( - grpc_dump_slice(*slice, GPR_DUMP_HEX | GPR_DUMP_ASCII)))); + grpc_core::UniquePtr dmp( + grpc_dump_slice(*slice, GPR_DUMP_HEX | GPR_DUMP_ASCII)); + p->error = grpc_error_set_str(p->error, GRPC_ERROR_STR_RAW_BYTES, + dmp.get()); p->error = grpc_error_set_int(p->error, GRPC_ERROR_INT_OFFSET, cur - beg); p->state = GRPC_CHTTP2_DATA_ERROR; diff --git a/src/core/ext/transport/chttp2/transport/frame_goaway.cc b/src/core/ext/transport/chttp2/transport/frame_goaway.cc index 45f250fb145..eb90e2e980b 100644 --- a/src/core/ext/transport/chttp2/transport/frame_goaway.cc +++ b/src/core/ext/transport/chttp2/transport/frame_goaway.cc @@ -139,7 +139,8 @@ grpc_error_handle grpc_chttp2_goaway_parser_parse(void* parser, if (is_last) { grpc_chttp2_add_incoming_goaway( t, p->error_code, p->last_stream_id, - grpc_slice_new(p->debug_data, p->debug_length, gpr_free)); + absl::string_view(p->debug_data, p->debug_length)); + gpr_free(p->debug_data); p->debug_data = nullptr; } return GRPC_ERROR_NONE; diff --git a/src/core/ext/transport/chttp2/transport/frame_rst_stream.cc b/src/core/ext/transport/chttp2/transport/frame_rst_stream.cc index fcdb544bb65..8288c6cfb94 100644 --- a/src/core/ext/transport/chttp2/transport/frame_rst_stream.cc +++ b/src/core/ext/transport/chttp2/transport/frame_rst_stream.cc @@ -109,8 +109,7 @@ grpc_error_handle grpc_chttp2_rst_stream_parser_parse(void* parser, grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("RST_STREAM"), GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_cpp_string(absl::StrCat( - "Received RST_STREAM with error code ", reason))), + absl::StrCat("Received RST_STREAM with error code ", reason)), GRPC_ERROR_INT_HTTP2_ERROR, static_cast(reason)); } grpc_chttp2_mark_stream_closed(t, s, true, true, error); diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index c416c3c7b22..1eb8069b51c 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -767,7 +767,7 @@ grpc_chttp2_stream* grpc_chttp2_parsing_accept_stream(grpc_chttp2_transport* t, void grpc_chttp2_add_incoming_goaway(grpc_chttp2_transport* t, uint32_t goaway_error, uint32_t last_stream_id, - const grpc_slice& goaway_text); + absl::string_view goaway_text); void grpc_chttp2_parsing_become_skip_parser(grpc_chttp2_transport* t); diff --git a/src/core/lib/http/httpcli.cc b/src/core/lib/http/httpcli.cc index 722cd572f83..9a98d69144a 100644 --- a/src/core/lib/http/httpcli.cc +++ b/src/core/lib/http/httpcli.cc @@ -118,8 +118,7 @@ static void append_error(internal_request* req, grpc_error_handle error) { std::string addr_text = grpc_sockaddr_to_uri(addr); req->overall_error = grpc_error_add_child( req->overall_error, - grpc_error_set_str(error, GRPC_ERROR_STR_TARGET_ADDRESS, - grpc_slice_from_cpp_string(std::move(addr_text)))); + grpc_error_set_str(error, GRPC_ERROR_STR_TARGET_ADDRESS, addr_text)); } static void do_read(internal_request* req) { diff --git a/src/core/lib/iomgr/endpoint_cfstream.cc b/src/core/lib/iomgr/endpoint_cfstream.cc index 0cc341bcd64..2f389dfed0f 100644 --- a/src/core/lib/iomgr/endpoint_cfstream.cc +++ b/src/core/lib/iomgr/endpoint_cfstream.cc @@ -110,8 +110,7 @@ static grpc_error_handle CFStreamAnnotateError(grpc_error_handle src_error, return grpc_error_set_str( grpc_error_set_int(src_error, GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE), - GRPC_ERROR_STR_TARGET_ADDRESS, - grpc_slice_from_copied_string(ep->peer_string.c_str())); + GRPC_ERROR_STR_TARGET_ADDRESS, ep->peer_string); } static void CallReadCb(CFStreamEndpoint* ep, grpc_error_handle error) { diff --git a/src/core/lib/iomgr/error.cc b/src/core/lib/iomgr/error.cc index 538d1363dd8..ba72d150e65 100644 --- a/src/core/lib/iomgr/error.cc +++ b/src/core/lib/iomgr/error.cc @@ -35,6 +35,7 @@ #include "src/core/lib/gpr/useful.h" #include "src/core/lib/iomgr/error_internal.h" #include "src/core/lib/slice/slice_internal.h" +#include "src/core/lib/slice/slice_utils.h" grpc_core::DebugOnlyTraceFlag grpc_trace_error_refcount(false, "error_refcount"); @@ -129,26 +130,23 @@ bool grpc_error_get_int(grpc_error_handle error, grpc_error_ints which, grpc_error_handle grpc_error_set_str(grpc_error_handle src, grpc_error_strs which, - const grpc_slice& str) { + absl::string_view str) { grpc_core::StatusSetStr( - &src, static_cast(which), - std::string(reinterpret_cast(GRPC_SLICE_START_PTR(str)), - GRPC_SLICE_LENGTH(str))); + &src, static_cast(which), str); return src; } bool grpc_error_get_str(grpc_error_handle error, grpc_error_strs which, - grpc_slice* s) { + std::string* s) { absl::optional value = grpc_core::StatusGetStr( error, static_cast(which)); if (value.has_value()) { - *s = grpc_slice_from_copied_buffer(value->c_str(), value->size()); + *s = std::move(*value); return true; } 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()); + *s = std::move(*value); return true; } return false; @@ -597,27 +595,26 @@ bool grpc_error_get_int(grpc_error_handle err, grpc_error_ints which, grpc_error_handle grpc_error_set_str(grpc_error_handle src, grpc_error_strs which, - const grpc_slice& str) { + absl::string_view str) { grpc_error_handle new_err = copy_error_and_unref(src); - internal_set_str(&new_err, which, str); + internal_set_str(&new_err, which, + grpc_slice_from_copied_buffer(str.data(), str.length())); return new_err; } bool grpc_error_get_str(grpc_error_handle err, grpc_error_strs which, - grpc_slice* str) { + std::string* s) { if (grpc_error_is_special(err)) { if (which != GRPC_ERROR_STR_GRPC_MESSAGE) return false; const special_error_status_map& msg = error_status_map[reinterpret_cast(err)]; - str->refcount = &grpc_core::kNoopRefcount; - str->data.refcounted.bytes = - reinterpret_cast(const_cast(msg.msg)); - str->data.refcounted.length = msg.len; + *s = std::string(msg.msg, msg.len); return true; } uint8_t slot = err->strs[which]; if (slot != UINT8_MAX) { - *str = *reinterpret_cast(err->arena + slot); + grpc_slice* slice = reinterpret_cast(err->arena + slot); + *s = std::string(grpc_core::StringViewFromSlice(*slice)); return true; } else { return false; @@ -903,9 +900,8 @@ grpc_error_handle grpc_os_error(const char* file, int line, int err, grpc_slice_from_static_string(strerror(err)), nullptr, 0), GRPC_ERROR_INT_ERRNO, err), - GRPC_ERROR_STR_OS_ERROR, - grpc_slice_from_static_string(strerror(err))), - GRPC_ERROR_STR_SYSCALL, grpc_slice_from_copied_string(call_name)); + GRPC_ERROR_STR_OS_ERROR, strerror(err)), + GRPC_ERROR_STR_SYSCALL, call_name); } #ifdef GPR_WINDOWS @@ -919,8 +915,8 @@ grpc_error_handle grpc_wsa_error(const char* file, int line, int err, grpc_slice_from_static_string("OS Error"), NULL, 0), GRPC_ERROR_INT_WSA_ERROR, err), - GRPC_ERROR_STR_OS_ERROR, grpc_slice_from_copied_string(utf8_message)), - GRPC_ERROR_STR_SYSCALL, grpc_slice_from_static_string(call_name)); + GRPC_ERROR_STR_OS_ERROR, utf8_message), + GRPC_ERROR_STR_SYSCALL, call_name); gpr_free(utf8_message); return error; } diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index 2a1d50a923d..e776853396d 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -364,15 +364,12 @@ grpc_error_handle grpc_error_set_int(grpc_error_handle src, /// intptr_t for `p`, even if the value of `p` is not used. bool grpc_error_get_int(grpc_error_handle error, grpc_error_ints which, intptr_t* p); -/// This call takes ownership of the slice; the error is responsible for -/// eventually unref-ing it. grpc_error_handle grpc_error_set_str( grpc_error_handle src, grpc_error_strs which, - const grpc_slice& str) GRPC_MUST_USE_RESULT; + absl::string_view str) GRPC_MUST_USE_RESULT; /// Returns false if the specified string is not set. -/// Caller does NOT own the slice. bool grpc_error_get_str(grpc_error_handle error, grpc_error_strs which, - grpc_slice* s); + std::string* str); /// Add a child error: an error that is believed to have contributed to this /// error occurring. Allows root causing high level errors from lower level diff --git a/src/core/lib/iomgr/load_file.cc b/src/core/lib/iomgr/load_file.cc index a1878229e82..9068670118d 100644 --- a/src/core/lib/iomgr/load_file.cc +++ b/src/core/lib/iomgr/load_file.cc @@ -71,8 +71,8 @@ end: grpc_error_set_str(GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( "Failed to load file", &error, 1), GRPC_ERROR_STR_FILENAME, - grpc_slice_from_copied_string( - filename)); // TODO(ncteisen), always static? + + filename); GRPC_ERROR_UNREF(error); error = error_out; } diff --git a/src/core/lib/iomgr/resolve_address_posix.cc b/src/core/lib/iomgr/resolve_address_posix.cc index 32816fec0af..1427cbda965 100644 --- a/src/core/lib/iomgr/resolve_address_posix.cc +++ b/src/core/lib/iomgr/resolve_address_posix.cc @@ -57,7 +57,7 @@ static grpc_error_handle posix_blocking_resolve_address( if (host.empty()) { err = grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("unparseable host:port"), - GRPC_ERROR_STR_TARGET_ADDRESS, grpc_slice_from_copied_string(name)); + GRPC_ERROR_STR_TARGET_ADDRESS, name); goto done; } @@ -65,7 +65,7 @@ static grpc_error_handle posix_blocking_resolve_address( if (default_port == nullptr) { err = grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("no port in name"), - GRPC_ERROR_STR_TARGET_ADDRESS, grpc_slice_from_copied_string(name)); + GRPC_ERROR_STR_TARGET_ADDRESS, name); goto done; } port = default_port; @@ -101,11 +101,9 @@ static grpc_error_handle posix_blocking_resolve_address( grpc_error_set_int( GRPC_ERROR_CREATE_FROM_STATIC_STRING(gai_strerror(s)), GRPC_ERROR_INT_ERRNO, s), - GRPC_ERROR_STR_OS_ERROR, - grpc_slice_from_static_string(gai_strerror(s))), - GRPC_ERROR_STR_SYSCALL, - grpc_slice_from_static_string("getaddrinfo")), - GRPC_ERROR_STR_TARGET_ADDRESS, grpc_slice_from_copied_string(name)); + GRPC_ERROR_STR_OS_ERROR, gai_strerror(s)), + GRPC_ERROR_STR_SYSCALL, "getaddrinfo"), + GRPC_ERROR_STR_TARGET_ADDRESS, name); goto done; } diff --git a/src/core/lib/iomgr/socket_utils_common_posix.cc b/src/core/lib/iomgr/socket_utils_common_posix.cc index 53df6d6943d..7c6706beff6 100644 --- a/src/core/lib/iomgr/socket_utils_common_posix.cc +++ b/src/core/lib/iomgr/socket_utils_common_posix.cc @@ -443,8 +443,7 @@ static grpc_error_handle error_for_fd(int fd, if (fd >= 0) return GRPC_ERROR_NONE; std::string addr_str = grpc_sockaddr_to_string(addr, false); grpc_error_handle err = grpc_error_set_str( - GRPC_OS_ERROR(errno, "socket"), GRPC_ERROR_STR_TARGET_ADDRESS, - grpc_slice_from_copied_string(addr_str.c_str())); + GRPC_OS_ERROR(errno, "socket"), GRPC_ERROR_STR_TARGET_ADDRESS, addr_str); return err; } diff --git a/src/core/lib/iomgr/tcp_client_posix.cc b/src/core/lib/iomgr/tcp_client_posix.cc index 3e8d20a9d88..f3b5220ddae 100644 --- a/src/core/lib/iomgr/tcp_client_posix.cc +++ b/src/core/lib/iomgr/tcp_client_posix.cc @@ -160,8 +160,7 @@ static void on_writable(void* acp, grpc_error_handle error) { gpr_mu_lock(&ac->mu); if (error != GRPC_ERROR_NONE) { error = - grpc_error_set_str(error, GRPC_ERROR_STR_OS_ERROR, - grpc_slice_from_static_string("Timeout occurred")); + grpc_error_set_str(error, GRPC_ERROR_STR_OS_ERROR, "Timeout occurred"); goto finish; } @@ -220,23 +219,16 @@ finish: fd = nullptr; } done = (--ac->refs == 0); - // Create a copy of the data from "ac" to be accessed after the unlock, as - // "ac" and its contents may be deallocated by the time they are read. - const grpc_slice addr_str_slice = grpc_slice_from_cpp_string(ac->addr_str); gpr_mu_unlock(&ac->mu); if (error != GRPC_ERROR_NONE) { - grpc_slice str; + std::string str; bool ret = grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &str); GPR_ASSERT(ret); - std::string description = absl::StrCat("Failed to connect to remote host: ", - grpc_core::StringViewFromSlice(str)); + std::string description = + absl::StrCat("Failed to connect to remote host: ", str); + error = grpc_error_set_str(error, GRPC_ERROR_STR_DESCRIPTION, description); error = - grpc_error_set_str(error, GRPC_ERROR_STR_DESCRIPTION, - grpc_slice_from_cpp_string(std::move(description))); - error = grpc_error_set_str(error, GRPC_ERROR_STR_TARGET_ADDRESS, - addr_str_slice /* takes ownership */); - } else { - grpc_slice_unref_internal(addr_str_slice); + grpc_error_set_str(error, GRPC_ERROR_STR_TARGET_ADDRESS, ac->addr_str); } if (done) { // This is safe even outside the lock, because "done", the sentinel, is @@ -309,9 +301,8 @@ void grpc_tcp_client_create_from_prepared_fd( if (errno != EWOULDBLOCK && errno != EINPROGRESS) { grpc_slice_allocator_destroy(slice_allocator); grpc_error_handle error = GRPC_OS_ERROR(errno, "connect"); - error = grpc_error_set_str( - error, GRPC_ERROR_STR_TARGET_ADDRESS, - grpc_slice_from_cpp_string(grpc_sockaddr_to_uri(addr))); + error = grpc_error_set_str(error, GRPC_ERROR_STR_TARGET_ADDRESS, + grpc_sockaddr_to_uri(addr)); grpc_fd_orphan(fdobj, nullptr, nullptr, "tcp_client_connect_error"); grpc_core::ExecCtx::Run(DEBUG_LOCATION, closure, error); return; diff --git a/src/core/lib/iomgr/tcp_client_windows.cc b/src/core/lib/iomgr/tcp_client_windows.cc index 5138b889ac7..f4e86bbdca3 100644 --- a/src/core/lib/iomgr/tcp_client_windows.cc +++ b/src/core/lib/iomgr/tcp_client_windows.cc @@ -225,8 +225,7 @@ failure: grpc_error_handle final_error = grpc_error_set_str(GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( "Failed to connect", &error, 1), - GRPC_ERROR_STR_TARGET_ADDRESS, - grpc_slice_from_cpp_string(std::move(target_uri))); + GRPC_ERROR_STR_TARGET_ADDRESS, target_uri); GRPC_ERROR_UNREF(error); grpc_slice_allocator_destroy(slice_allocator); if (socket != NULL) { diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 2f75ffc066c..a3e312b4e62 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -592,8 +592,7 @@ static grpc_error_handle tcp_annotate_error(grpc_error_handle src_error, /* All tcp errors are marked with UNAVAILABLE so that application may * choose to retry. */ GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE), - GRPC_ERROR_STR_TARGET_ADDRESS, - grpc_slice_from_copied_string(tcp->peer_string.c_str())); + GRPC_ERROR_STR_TARGET_ADDRESS, tcp->peer_string); } static void tcp_handle_read(void* arg /* grpc_tcp */, grpc_error_handle error); diff --git a/src/core/lib/iomgr/tcp_server_windows.cc b/src/core/lib/iomgr/tcp_server_windows.cc index 80f4d35eaeb..e853c4f6acf 100644 --- a/src/core/lib/iomgr/tcp_server_windows.cc +++ b/src/core/lib/iomgr/tcp_server_windows.cc @@ -230,11 +230,10 @@ static grpc_error_handle prepare_socket(SOCKET sock, failure: GPR_ASSERT(error != GRPC_ERROR_NONE); grpc_error_set_int( - grpc_error_set_str( - GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - "Failed to prepare server socket", &error, 1), - GRPC_ERROR_STR_TARGET_ADDRESS, - grpc_slice_from_cpp_string(grpc_sockaddr_to_uri(addr))), + grpc_error_set_str(GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( + "Failed to prepare server socket", &error, 1), + GRPC_ERROR_STR_TARGET_ADDRESS, + grpc_sockaddr_to_uri(addr)), GRPC_ERROR_INT_FD, (intptr_t)sock); GRPC_ERROR_UNREF(error); if (sock != INVALID_SOCKET) closesocket(sock); diff --git a/src/core/lib/security/credentials/google_default/google_default_credentials.cc b/src/core/lib/security/credentials/google_default/google_default_credentials.cc index 1d95fe30b17..18710c5ecac 100644 --- a/src/core/lib/security/credentials/google_default/google_default_credentials.cc +++ b/src/core/lib/security/credentials/google_default/google_default_credentials.cc @@ -290,7 +290,7 @@ static grpc_error_handle create_default_creds_from_path( if (json.type() != Json::Type::OBJECT) { error = grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("Failed to parse JSON"), - GRPC_ERROR_STR_RAW_BYTES, grpc_slice_ref_internal(creds_data)); + GRPC_ERROR_STR_RAW_BYTES, grpc_core::StringViewFromSlice(creds_data)); goto end; } diff --git a/src/core/lib/security/transport/tsi_error.cc b/src/core/lib/security/transport/tsi_error.cc index 4d337b4622f..23b1081128c 100644 --- a/src/core/lib/security/transport/tsi_error.cc +++ b/src/core/lib/security/transport/tsi_error.cc @@ -22,9 +22,7 @@ grpc_error_handle grpc_set_tsi_error_result(grpc_error_handle error, tsi_result result) { - return grpc_error_set_int( - grpc_error_set_str( - error, GRPC_ERROR_STR_TSI_ERROR, - grpc_slice_from_static_string(tsi_result_to_string(result))), - GRPC_ERROR_INT_TSI_CODE, result); + return grpc_error_set_int(grpc_error_set_str(error, GRPC_ERROR_STR_TSI_ERROR, + tsi_result_to_string(result)), + GRPC_ERROR_INT_TSI_CODE, result); } diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index df09c1931a7..196035ffa64 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -725,8 +725,7 @@ static grpc_error_handle error_from_status(grpc_status_code status, // guarantee that can be short-lived. return grpc_error_set_int( grpc_error_set_str(GRPC_ERROR_CREATE_FROM_COPIED_STRING(description), - GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_copied_string(description)), + GRPC_ERROR_STR_GRPC_MESSAGE, description), GRPC_ERROR_INT_GRPC_STATUS, status); } @@ -741,12 +740,12 @@ static void set_final_status(grpc_call* call, grpc_error_handle error) { gpr_log(GPR_DEBUG, "%s", grpc_error_std_string(error).c_str()); } if (call->is_client) { + std::string status_details; grpc_error_get_status(error, call->send_deadline, - call->final_op.client.status, - call->final_op.client.status_details, nullptr, - call->final_op.client.error_string); - // explicitly take a ref - grpc_slice_ref_internal(*call->final_op.client.status_details); + call->final_op.client.status, &status_details, + nullptr, call->final_op.client.error_string); + *call->final_op.client.status_details = + grpc_slice_from_cpp_string(std::move(status_details)); call->status_error.set(error); GRPC_ERROR_UNREF(error); grpc_core::channelz::ChannelNode* channelz_channel = @@ -1068,12 +1067,11 @@ static void recv_trailing_filter(void* args, grpc_metadata_batch* b, if (b->legacy_index()->named.grpc_message != nullptr) { error = grpc_error_set_str( error, GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_ref_internal( + grpc_core::StringViewFromSlice( GRPC_MDVALUE(b->legacy_index()->named.grpc_message->md))); b->Remove(GRPC_BATCH_GRPC_MESSAGE); } else if (error != GRPC_ERROR_NONE) { - error = grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_empty_slice()); + error = grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, ""); } set_final_status(call, GRPC_ERROR_REF(error)); b->Remove(GRPC_BATCH_GRPC_STATUS); @@ -1763,9 +1761,8 @@ static grpc_call_error call_start_batch(grpc_call* call, const grpc_op* ops, if (status_error != GRPC_ERROR_NONE) { char* msg = grpc_slice_to_c_string( GRPC_MDVALUE(call->send_extra_metadata[1].md)); - status_error = - grpc_error_set_str(status_error, GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_copied_string(msg)); + status_error = grpc_error_set_str(status_error, + GRPC_ERROR_STR_GRPC_MESSAGE, msg); gpr_free(msg); } } diff --git a/src/core/lib/surface/lame_client.cc b/src/core/lib/surface/lame_client.cc index 742ad099f24..a1fbd069c95 100644 --- a/src/core/lib/surface/lame_client.cc +++ b/src/core/lib/surface/lame_client.cc @@ -182,8 +182,7 @@ grpc_channel* grpc_lame_client_channel_create(const char* target, grpc_error_set_int( GRPC_ERROR_CREATE_FROM_STATIC_STRING("lame client channel"), GRPC_ERROR_INT_GRPC_STATUS, error_code), - GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_static_string(error_message)); + GRPC_ERROR_STR_GRPC_MESSAGE, error_message); grpc_arg error_arg = grpc_core::MakeLameClientErrorArg(&error); grpc_channel_args args = {1, &error_arg}; grpc_channel* channel = grpc_channel_create( diff --git a/src/core/lib/surface/validate_metadata.cc b/src/core/lib/surface/validate_metadata.cc index 1f16ec25db1..077acb11c52 100644 --- a/src/core/lib/surface/validate_metadata.cc +++ b/src/core/lib/surface/validate_metadata.cc @@ -47,12 +47,15 @@ static grpc_error_handle conforms_to(const grpc_slice& slice, const uint8_t* e = GRPC_SLICE_END_PTR(slice); for (; p != e; p++) { if (!legal_bits.is_set(*p)) { + size_t len; + grpc_core::UniquePtr ptr(gpr_dump_return_len( + reinterpret_cast GRPC_SLICE_START_PTR(slice), + GRPC_SLICE_LENGTH(slice), GPR_DUMP_HEX | GPR_DUMP_ASCII, &len)); grpc_error_handle error = grpc_error_set_str( grpc_error_set_int(GRPC_ERROR_CREATE_FROM_COPIED_STRING(err_desc), GRPC_ERROR_INT_OFFSET, p - GRPC_SLICE_START_PTR(slice)), - GRPC_ERROR_STR_RAW_BYTES, - grpc_dump_slice_to_slice(slice, GPR_DUMP_HEX | GPR_DUMP_ASCII)); + GRPC_ERROR_STR_RAW_BYTES, absl::string_view(ptr.get(), len)); return error; } } diff --git a/src/core/lib/transport/error_utils.cc b/src/core/lib/transport/error_utils.cc index d46b63af57d..3fdfa92d217 100644 --- a/src/core/lib/transport/error_utils.cc +++ b/src/core/lib/transport/error_utils.cc @@ -57,22 +57,22 @@ static grpc_error_handle recursively_find_error_with_field( } void grpc_error_get_status(grpc_error_handle error, grpc_millis deadline, - grpc_status_code* code, grpc_slice* slice, + grpc_status_code* code, std::string* message, grpc_http2_error_code* http_error, const char** error_string) { // Fast path: We expect no error. if (GPR_LIKELY(error == GRPC_ERROR_NONE)) { if (code != nullptr) *code = GRPC_STATUS_OK; - if (slice != nullptr) { + if (message != nullptr) { // Normally, we call grpc_error_get_str( - // error, GRPC_ERROR_STR_GRPC_MESSAGE, slice). + // error, GRPC_ERROR_STR_GRPC_MESSAGE, message). // We can fastpath since we know that: // 1) Error is null // 2) which == GRPC_ERROR_STR_GRPC_MESSAGE - // 3) The resulting slice is statically known. - // 4) Said resulting slice is of length 0 (""). + // 3) The resulting message is statically known. + // 4) Said resulting message is "". // This means 3 movs, instead of 10s of instructions and a strlen. - *slice = grpc_core::ExternallyManagedSlice(""); + *message = ""; } if (http_error != nullptr) { *http_error = GRPC_HTTP2_NO_ERROR; @@ -125,14 +125,15 @@ void grpc_error_get_status(grpc_error_handle error, grpc_millis deadline, // If the error has a status message, use it. Otherwise, fall back to // the error description. - 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_DESCRIPTION, slice)) { + if (message != nullptr) { + if (!grpc_error_get_str(found_error, GRPC_ERROR_STR_GRPC_MESSAGE, + message)) { + if (!grpc_error_get_str(found_error, GRPC_ERROR_STR_DESCRIPTION, + message)) { #ifdef GRPC_ERROR_IS_ABSEIL_STATUS - *slice = - grpc_slice_from_copied_string(grpc_error_std_string(error).c_str()); + *message = grpc_error_std_string(error)); #else - *slice = grpc_slice_from_static_string("unknown error"); + *message = "unknown error"; #endif } } @@ -143,13 +144,10 @@ absl::Status grpc_error_to_absl_status(grpc_error_handle error) { grpc_status_code status; // TODO(yashykt): This should be updated once we decide on how to use the // absl::Status payload to capture all the contents of grpc_error. - grpc_slice message; + std::string message; grpc_error_get_status(error, GRPC_MILLIS_INF_FUTURE, &status, &message, nullptr /* http_error */, nullptr /* error_string */); - return absl::Status(static_cast(status), - absl::string_view(reinterpret_cast( - GRPC_SLICE_START_PTR(message)), - GRPC_SLICE_LENGTH(message))); + return absl::Status(static_cast(status), message); } grpc_error_handle absl_status_to_grpc_error(absl::Status status) { diff --git a/src/core/lib/transport/error_utils.h b/src/core/lib/transport/error_utils.h index 777e63230fa..4cc2db68f9c 100644 --- a/src/core/lib/transport/error_utils.h +++ b/src/core/lib/transport/error_utils.h @@ -35,7 +35,7 @@ /// msg, http_status, error_string) are unneeded, they can be passed as /// NULL. void grpc_error_get_status(grpc_error_handle error, grpc_millis deadline, - grpc_status_code* code, grpc_slice* slice, + grpc_status_code* code, std::string* message, grpc_http2_error_code* http_error, const char** error_string); diff --git a/src/core/lib/transport/metadata_batch.cc b/src/core/lib/transport/metadata_batch.cc index a0e00d2f7ac..00e6074addf 100644 --- a/src/core/lib/transport/metadata_batch.cc +++ b/src/core/lib/transport/metadata_batch.cc @@ -74,7 +74,7 @@ grpc_error_handle grpc_attach_md_to_error(grpc_error_handle src, grpc_mdelem md) { grpc_error_handle out = grpc_error_set_str( grpc_error_set_str(src, GRPC_ERROR_STR_KEY, - grpc_slice_ref_internal(GRPC_MDKEY(md))), - GRPC_ERROR_STR_VALUE, grpc_slice_ref_internal(GRPC_MDVALUE(md))); + grpc_core::StringViewFromSlice(GRPC_MDKEY(md))), + GRPC_ERROR_STR_VALUE, grpc_core::StringViewFromSlice(GRPC_MDVALUE(md))); return out; } diff --git a/test/core/bad_client/tests/bad_streaming_id.cc b/test/core/bad_client/tests/bad_streaming_id.cc index a76f55dd2fd..85b5c94fa15 100644 --- a/test/core/bad_client/tests/bad_streaming_id.cc +++ b/test/core/bad_client/tests/bad_streaming_id.cc @@ -124,9 +124,9 @@ TEST(BadStreamingId, ClosedStreamId) { } // namespace int main(int argc, char** argv) { - grpc_init(); grpc::testing::TestEnvironment env(argc, argv); ::testing::InitGoogleTest(&argc, argv); + grpc_init(); int retval = RUN_ALL_TESTS(); grpc_shutdown(); return retval; diff --git a/test/core/bad_client/tests/out_of_bounds.cc b/test/core/bad_client/tests/out_of_bounds.cc index ce3f1d8a0a7..7bf1a914931 100644 --- a/test/core/bad_client/tests/out_of_bounds.cc +++ b/test/core/bad_client/tests/out_of_bounds.cc @@ -104,9 +104,9 @@ TEST(OutOfBounds, WindowUpdate) { } // namespace int main(int argc, char** argv) { - grpc_init(); grpc::testing::TestEnvironment env(argc, argv); ::testing::InitGoogleTest(&argc, argv); + grpc_init(); int retval = RUN_ALL_TESTS(); grpc_shutdown(); return retval; diff --git a/test/core/bad_client/tests/unknown_frame.cc b/test/core/bad_client/tests/unknown_frame.cc index 04ce10e1e81..e11625bf3e7 100644 --- a/test/core/bad_client/tests/unknown_frame.cc +++ b/test/core/bad_client/tests/unknown_frame.cc @@ -57,9 +57,9 @@ TEST(UnknownFrameType, Test) { } // namespace int main(int argc, char** argv) { - grpc_init(); grpc::testing::TestEnvironment env(argc, argv); ::testing::InitGoogleTest(&argc, argv); + grpc_init(); int retval = RUN_ALL_TESTS(); grpc_shutdown(); return retval; diff --git a/test/core/iomgr/BUILD b/test/core/iomgr/BUILD index b06aedd3bb6..9fa7c57d371 100644 --- a/test/core/iomgr/BUILD +++ b/test/core/iomgr/BUILD @@ -68,6 +68,9 @@ grpc_cc_test( grpc_cc_test( name = "error_test", srcs = ["error_test.cc"], + external_deps = [ + "gtest", + ], language = "C++", uses_polling = False, deps = [ diff --git a/test/core/iomgr/error_test.cc b/test/core/iomgr/error_test.cc index dcc2aead6fc..d9430bf14bd 100644 --- a/test/core/iomgr/error_test.cc +++ b/test/core/iomgr/error_test.cc @@ -20,117 +20,114 @@ #include +#include + #include #include #include #include "test/core/util/test_config.h" -static void test_set_get_int() { +TEST(ErrorTest, SetGetInt) { grpc_error_handle error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Test"); - GPR_ASSERT(error != GRPC_ERROR_NONE); + EXPECT_NE(error, GRPC_ERROR_NONE); intptr_t i = 0; - GPR_ASSERT(grpc_error_get_int(error, GRPC_ERROR_INT_FILE_LINE, &i)); - GPR_ASSERT(i); // line set will never be 0 - GPR_ASSERT(!grpc_error_get_int(error, GRPC_ERROR_INT_ERRNO, &i)); - GPR_ASSERT(!grpc_error_get_int(error, GRPC_ERROR_INT_SIZE, &i)); + EXPECT_TRUE(grpc_error_get_int(error, GRPC_ERROR_INT_FILE_LINE, &i)); + EXPECT_TRUE(i); // line set will never be 0 + EXPECT_TRUE(!grpc_error_get_int(error, GRPC_ERROR_INT_ERRNO, &i)); + EXPECT_TRUE(!grpc_error_get_int(error, GRPC_ERROR_INT_SIZE, &i)); intptr_t errnumber = 314; error = grpc_error_set_int(error, GRPC_ERROR_INT_ERRNO, errnumber); - GPR_ASSERT(grpc_error_get_int(error, GRPC_ERROR_INT_ERRNO, &i)); - GPR_ASSERT(i == errnumber); + EXPECT_TRUE(grpc_error_get_int(error, GRPC_ERROR_INT_ERRNO, &i)); + EXPECT_EQ(i, errnumber); intptr_t http = 2; error = grpc_error_set_int(error, GRPC_ERROR_INT_HTTP2_ERROR, http); - GPR_ASSERT(grpc_error_get_int(error, GRPC_ERROR_INT_HTTP2_ERROR, &i)); - GPR_ASSERT(i == http); + EXPECT_TRUE(grpc_error_get_int(error, GRPC_ERROR_INT_HTTP2_ERROR, &i)); + EXPECT_EQ(i, http); GRPC_ERROR_UNREF(error); } -static void test_set_get_str() { +TEST(ErrorTest, SetGetStr) { grpc_error_handle error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Test"); - grpc_slice str; - GPR_ASSERT(!grpc_error_get_str(error, GRPC_ERROR_STR_SYSCALL, &str)); - GPR_ASSERT(!grpc_error_get_str(error, GRPC_ERROR_STR_TSI_ERROR, &str)); + std::string str; + EXPECT_TRUE(!grpc_error_get_str(error, GRPC_ERROR_STR_SYSCALL, &str)); + EXPECT_TRUE(!grpc_error_get_str(error, GRPC_ERROR_STR_TSI_ERROR, &str)); - GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_FILE, &str)); - GPR_ASSERT(strstr((char*)GRPC_SLICE_START_PTR(str), - "error_test.c")); // __FILE__ expands differently on - // Windows. All should at least - // contain error_test.c + EXPECT_TRUE(grpc_error_get_str(error, GRPC_ERROR_STR_FILE, &str)); + EXPECT_THAT(str, testing::HasSubstr("error_test.c")); + // __FILE__ expands differently on + // Windows. All should at least + // contain error_test.c - GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &str)); - GPR_ASSERT(!strncmp((char*)GRPC_SLICE_START_PTR(str), "Test", - GRPC_SLICE_LENGTH(str))); + EXPECT_TRUE(grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &str)); + EXPECT_EQ(str, "Test"); - error = grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_static_string("longer message")); - GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, &str)); - GPR_ASSERT(!strncmp((char*)GRPC_SLICE_START_PTR(str), "longer message", - GRPC_SLICE_LENGTH(str))); + error = + grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, "longer message"); + EXPECT_TRUE(grpc_error_get_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, &str)); + EXPECT_EQ(str, "longer message"); GRPC_ERROR_UNREF(error); } -static void test_copy_and_unref() { +TEST(ErrorTest, CopyAndUnRef) { // error1 has one ref - grpc_error_handle error1 = grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Test"), GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_static_string("message")); - grpc_slice str; - GPR_ASSERT(grpc_error_get_str(error1, GRPC_ERROR_STR_GRPC_MESSAGE, &str)); - GPR_ASSERT(!strncmp((char*)GRPC_SLICE_START_PTR(str), "message", - GRPC_SLICE_LENGTH(str))); + grpc_error_handle error1 = + grpc_error_set_str(GRPC_ERROR_CREATE_FROM_STATIC_STRING("Test"), + GRPC_ERROR_STR_GRPC_MESSAGE, "message"); + std::string str; + EXPECT_TRUE(grpc_error_get_str(error1, GRPC_ERROR_STR_GRPC_MESSAGE, &str)); + EXPECT_EQ(str, "message"); // error 1 has two refs GRPC_ERROR_REF(error1); // this gives error3 a ref to the new error, and decrements error1 to one ref - grpc_error_handle error3 = grpc_error_set_str( - error1, GRPC_ERROR_STR_SYSCALL, grpc_slice_from_static_string("syscall")); - GPR_ASSERT(error3 != error1); // should not be the same because of extra ref - GPR_ASSERT(grpc_error_get_str(error3, GRPC_ERROR_STR_GRPC_MESSAGE, &str)); - GPR_ASSERT(!strncmp((char*)GRPC_SLICE_START_PTR(str), "message", - GRPC_SLICE_LENGTH(str))); + grpc_error_handle error3 = + grpc_error_set_str(error1, GRPC_ERROR_STR_SYSCALL, "syscall"); + EXPECT_NE(error3, error1); // should not be the same because of extra ref + EXPECT_TRUE(grpc_error_get_str(error3, GRPC_ERROR_STR_GRPC_MESSAGE, &str)); + EXPECT_EQ(str, "message"); // error 1 should not have a syscall but 3 should - GPR_ASSERT(!grpc_error_get_str(error1, GRPC_ERROR_STR_SYSCALL, &str)); - GPR_ASSERT(grpc_error_get_str(error3, GRPC_ERROR_STR_SYSCALL, &str)); - GPR_ASSERT(!strncmp((char*)GRPC_SLICE_START_PTR(str), "syscall", - GRPC_SLICE_LENGTH(str))); + EXPECT_TRUE(!grpc_error_get_str(error1, GRPC_ERROR_STR_SYSCALL, &str)); + EXPECT_TRUE(grpc_error_get_str(error3, GRPC_ERROR_STR_SYSCALL, &str)); + EXPECT_EQ(str, "syscall"); GRPC_ERROR_UNREF(error1); GRPC_ERROR_UNREF(error3); } -static void test_create_referencing() { - grpc_error_handle child = grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Child"), - GRPC_ERROR_STR_GRPC_MESSAGE, grpc_slice_from_static_string("message")); +TEST(ErrorTest, CreateReferencing) { + grpc_error_handle child = + grpc_error_set_str(GRPC_ERROR_CREATE_FROM_STATIC_STRING("Child"), + GRPC_ERROR_STR_GRPC_MESSAGE, "message"); grpc_error_handle parent = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING("Parent", &child, 1); - GPR_ASSERT(parent != GRPC_ERROR_NONE); + EXPECT_NE(parent, GRPC_ERROR_NONE); GRPC_ERROR_UNREF(child); GRPC_ERROR_UNREF(parent); } -static void test_create_referencing_many() { +TEST(ErrorTest, CreateReferencingMany) { grpc_error_handle children[3]; - children[0] = grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Child1"), - GRPC_ERROR_STR_GRPC_MESSAGE, grpc_slice_from_static_string("message")); + children[0] = + grpc_error_set_str(GRPC_ERROR_CREATE_FROM_STATIC_STRING("Child1"), + GRPC_ERROR_STR_GRPC_MESSAGE, "message"); children[1] = grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("Child2"), GRPC_ERROR_INT_HTTP2_ERROR, 5); - children[2] = grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Child3"), - GRPC_ERROR_STR_GRPC_MESSAGE, grpc_slice_from_static_string("message 3")); + children[2] = + grpc_error_set_str(GRPC_ERROR_CREATE_FROM_STATIC_STRING("Child3"), + GRPC_ERROR_STR_GRPC_MESSAGE, "message 3"); grpc_error_handle parent = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING("Parent", children, 3); - GPR_ASSERT(parent != GRPC_ERROR_NONE); + EXPECT_NE(parent, GRPC_ERROR_NONE); for (size_t i = 0; i < 3; ++i) { GRPC_ERROR_UNREF(children[i]); @@ -138,29 +135,26 @@ static void test_create_referencing_many() { GRPC_ERROR_UNREF(parent); } -static void print_error_string() { +TEST(ErrorTest, PrintErrorString) { grpc_error_handle error = grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"), GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNIMPLEMENTED); error = grpc_error_set_int(error, GRPC_ERROR_INT_SIZE, 666); - error = grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_static_string("message")); + error = grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, "message"); // gpr_log(GPR_DEBUG, "%s", grpc_error_std_string(error).c_str()); GRPC_ERROR_UNREF(error); } -static void print_error_string_reference() { +TEST(ErrorTest, PrintErrorStringReference) { grpc_error_handle children[2]; children[0] = grpc_error_set_str( grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("1"), GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNIMPLEMENTED), - GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_static_string("message for child 1")); + GRPC_ERROR_STR_GRPC_MESSAGE, "message for child 1"); children[1] = grpc_error_set_str( grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("2sd"), GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_INTERNAL), - GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_static_string("message for child 2")); + GRPC_ERROR_STR_GRPC_MESSAGE, "message for child 2"); grpc_error_handle parent = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING("Parent", children, 2); @@ -171,23 +165,22 @@ static void print_error_string_reference() { GRPC_ERROR_UNREF(parent); } -static void test_os_error() { +TEST(ErrorTest, TestOsError) { int fake_errno = 5; const char* syscall = "syscall name"; grpc_error_handle error = GRPC_OS_ERROR(fake_errno, syscall); intptr_t i = 0; - GPR_ASSERT(grpc_error_get_int(error, GRPC_ERROR_INT_ERRNO, &i)); - GPR_ASSERT(i == fake_errno); + EXPECT_TRUE(grpc_error_get_int(error, GRPC_ERROR_INT_ERRNO, &i)); + EXPECT_EQ(i, fake_errno); - grpc_slice str; - GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_SYSCALL, &str)); - GPR_ASSERT(!strncmp((char*)GRPC_SLICE_START_PTR(str), syscall, - GRPC_SLICE_LENGTH(str))); + std::string str; + EXPECT_TRUE(grpc_error_get_str(error, GRPC_ERROR_STR_SYSCALL, &str)); + EXPECT_EQ(str, syscall); GRPC_ERROR_UNREF(error); } -static void test_overflow() { +TEST(ErrorTest, 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"); @@ -198,19 +191,18 @@ static void test_overflow() { } error = grpc_error_set_int(error, GRPC_ERROR_INT_HTTP2_ERROR, 5); - error = - grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_static_string("message for child 2")); + error = grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, + "message for child 2"); error = grpc_error_set_int(error, GRPC_ERROR_INT_GRPC_STATUS, 5); intptr_t i; - GPR_ASSERT(grpc_error_get_int(error, GRPC_ERROR_INT_HTTP2_ERROR, &i)); - GPR_ASSERT(i == 5); - GPR_ASSERT(!grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &i)); + EXPECT_TRUE(grpc_error_get_int(error, GRPC_ERROR_INT_HTTP2_ERROR, &i)); + EXPECT_EQ(i, 5); + EXPECT_TRUE(!grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &i)); error = grpc_error_set_int(error, GRPC_ERROR_INT_HTTP2_ERROR, 10); - GPR_ASSERT(grpc_error_get_int(error, GRPC_ERROR_INT_HTTP2_ERROR, &i)); - GPR_ASSERT(i == 10); + EXPECT_TRUE(grpc_error_get_int(error, GRPC_ERROR_INT_HTTP2_ERROR, &i)); + EXPECT_EQ(i, 10); GRPC_ERROR_UNREF(error); #endif @@ -218,17 +210,9 @@ static void test_overflow() { int main(int argc, char** argv) { grpc::testing::TestEnvironment env(argc, argv); + ::testing::InitGoogleTest(&argc, argv); grpc_init(); - test_set_get_int(); - test_set_get_str(); - test_copy_and_unref(); - print_error_string(); - print_error_string_reference(); - test_os_error(); - test_create_referencing(); - test_create_referencing_many(); - test_overflow(); + int retval = RUN_ALL_TESTS(); grpc_shutdown(); - - return 0; + return retval; } diff --git a/test/core/iomgr/work_serializer_test.cc b/test/core/iomgr/work_serializer_test.cc index 7a5ce5b4e5e..09258747800 100644 --- a/test/core/iomgr/work_serializer_test.cc +++ b/test/core/iomgr/work_serializer_test.cc @@ -107,8 +107,8 @@ TEST(WorkSerializerTest, ExecuteMany) { int main(int argc, char** argv) { grpc::testing::TestEnvironment env(argc, argv); - grpc_init(); ::testing::InitGoogleTest(&argc, argv); + grpc_init(); int retval = RUN_ALL_TESTS(); grpc_shutdown(); return retval; diff --git a/test/core/security/aws_request_signer_test.cc b/test/core/security/aws_request_signer_test.cc index cb97292d7a0..699f32f17b8 100644 --- a/test/core/security/aws_request_signer_test.cc +++ b/test/core/security/aws_request_signer_test.cc @@ -246,13 +246,10 @@ TEST(GrpcAwsRequestSignerTest, InvalidUrl) { grpc_core::AwsRequestSigner signer("access_key_id", "secret_access_key", "token", "POST", "invalid_url", "us-east-1", "", {}, &error); - grpc_slice expected_error_description = - grpc_slice_from_static_string("Invalid Aws request url."); - grpc_slice actual_error_description; + std::string actual_error_description; GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &actual_error_description)); - EXPECT_TRUE(grpc_slice_cmp(expected_error_description, - actual_error_description) == 0); + EXPECT_EQ(actual_error_description, "Invalid Aws request url."); GRPC_ERROR_UNREF(error); } @@ -262,13 +259,11 @@ TEST(GrpcAwsRequestSignerTest, DuplicateRequestDate) { "access_key_id", "secret_access_key", "token", "POST", "invalid_url", "us-east-1", "", {{"date", kBotoTestDate}, {"x-amz-date", kAmzTestDate}}, &error); - grpc_slice expected_error_description = grpc_slice_from_static_string( - "Only one of {date, x-amz-date} can be specified, not both."); - grpc_slice actual_error_description; + std::string actual_error_description; GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &actual_error_description)); - EXPECT_TRUE(grpc_slice_cmp(expected_error_description, - actual_error_description) == 0); + EXPECT_EQ(actual_error_description, + "Only one of {date, x-amz-date} can be specified, not both."); GRPC_ERROR_UNREF(error); } diff --git a/test/core/security/credentials_test.cc b/test/core/security/credentials_test.cc index cb5bc3a63ef..9e9abf8ec03 100644 --- a/test/core/security/credentials_test.cc +++ b/test/core/security/credentials_test.cc @@ -491,13 +491,13 @@ static void check_request_metadata(void* arg, grpc_error_handle error) { if (state->expected_error == GRPC_ERROR_NONE) { GPR_ASSERT(error == GRPC_ERROR_NONE); } else { - grpc_slice expected_error; + std::string expected_error; GPR_ASSERT(grpc_error_get_str(state->expected_error, GRPC_ERROR_STR_DESCRIPTION, &expected_error)); - grpc_slice actual_error; + std::string actual_error; GPR_ASSERT( grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &actual_error)); - GPR_ASSERT(grpc_slice_cmp(expected_error, actual_error) == 0); + GPR_ASSERT(expected_error == actual_error); GRPC_ERROR_UNREF(state->expected_error); } gpr_log(GPR_INFO, "expected_size=%" PRIdPTR " actual_size=%" PRIdPTR, @@ -2677,11 +2677,9 @@ test_url_external_account_creds_failure_invalid_credential_source_url(void) { auto creds = grpc_core::UrlExternalAccountCredentials::Create(options, {}, &error); GPR_ASSERT(creds == nullptr); - grpc_slice actual_error_slice; - GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, - &actual_error_slice)); - absl::string_view actual_error = - grpc_core::StringViewFromSlice(actual_error_slice); + std::string actual_error; + GPR_ASSERT( + grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &actual_error)); GPR_ASSERT(absl::StartsWith(actual_error, "Invalid credential source url.")); GRPC_ERROR_UNREF(error); } @@ -3219,12 +3217,11 @@ static void test_aws_external_account_creds_failure_unmatched_environment_id( auto creds = grpc_core::AwsExternalAccountCredentials::Create(options, {}, &error); GPR_ASSERT(creds == nullptr); - grpc_slice expected_error_slice = - grpc_slice_from_static_string("environment_id does not match."); - grpc_slice actual_error_slice; - GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, - &actual_error_slice)); - GPR_ASSERT(grpc_slice_cmp(expected_error_slice, actual_error_slice) == 0); + std::string expected_error = "environment_id does not match."; + std::string actual_error; + GPR_ASSERT( + grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &actual_error)); + GPR_ASSERT(expected_error == actual_error); GRPC_ERROR_UNREF(error); } diff --git a/test/core/security/grpc_tls_certificate_distributor_test.cc b/test/core/security/grpc_tls_certificate_distributor_test.cc index 17aac541276..24a25a1bf93 100644 --- a/test/core/security/grpc_tls_certificate_distributor_test.cc +++ b/test/core/security/grpc_tls_certificate_distributor_test.cc @@ -137,18 +137,13 @@ class GrpcTlsCertificateDistributorTest : public ::testing::Test { std::string root_error_str; std::string identity_error_str; if (root_cert_error != GRPC_ERROR_NONE) { - grpc_slice root_error_slice; GPR_ASSERT(grpc_error_get_str( - root_cert_error, GRPC_ERROR_STR_DESCRIPTION, &root_error_slice)); - root_error_str = std::string(StringViewFromSlice(root_error_slice)); + root_cert_error, GRPC_ERROR_STR_DESCRIPTION, &root_error_str)); } if (identity_cert_error != GRPC_ERROR_NONE) { - grpc_slice identity_error_slice; GPR_ASSERT(grpc_error_get_str(identity_cert_error, GRPC_ERROR_STR_DESCRIPTION, - &identity_error_slice)); - identity_error_str = - std::string(StringViewFromSlice(identity_error_slice)); + &identity_error_str)); } state_->error_queue.emplace_back(std::move(root_error_str), std::move(identity_error_str)); diff --git a/test/core/security/grpc_tls_certificate_provider_test.cc b/test/core/security/grpc_tls_certificate_provider_test.cc index 40e6f03ccd4..a13e8078172 100644 --- a/test/core/security/grpc_tls_certificate_provider_test.cc +++ b/test/core/security/grpc_tls_certificate_provider_test.cc @@ -139,18 +139,13 @@ class GrpcTlsCertificateProviderTest : public ::testing::Test { std::string root_error_str; std::string identity_error_str; if (root_cert_error != GRPC_ERROR_NONE) { - grpc_slice root_error_slice; GPR_ASSERT(grpc_error_get_str( - root_cert_error, GRPC_ERROR_STR_DESCRIPTION, &root_error_slice)); - root_error_str = std::string(StringViewFromSlice(root_error_slice)); + root_cert_error, GRPC_ERROR_STR_DESCRIPTION, &root_error_str)); } if (identity_cert_error != GRPC_ERROR_NONE) { - grpc_slice identity_error_slice; GPR_ASSERT(grpc_error_get_str(identity_cert_error, GRPC_ERROR_STR_DESCRIPTION, - &identity_error_slice)); - identity_error_str = - std::string(StringViewFromSlice(identity_error_slice)); + &identity_error_str)); } state_->error_queue.emplace_back(std::move(root_error_str), std::move(identity_error_str)); diff --git a/test/core/transport/error_utils_test.cc b/test/core/transport/error_utils_test.cc index 2c33e097642..ebbb9f963cd 100644 --- a/test/core/transport/error_utils_test.cc +++ b/test/core/transport/error_utils_test.cc @@ -67,11 +67,9 @@ TEST(ErrorUtilsTest, AbslUnavailableToGrpcError) { ASSERT_TRUE(grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &code)); ASSERT_EQ(static_cast(code), GRPC_STATUS_UNAVAILABLE); // Status message checks - grpc_slice message; + std::string message; ASSERT_TRUE(grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &message)); - absl::string_view str = grpc_core::StringViewFromSlice(message); - ASSERT_EQ(str, "Making tea"); - grpc_slice_unref(message); + ASSERT_EQ(message, "Making tea"); GRPC_ERROR_UNREF(error); } diff --git a/test/cpp/microbenchmarks/bm_error.cc b/test/cpp/microbenchmarks/bm_error.cc index 0492b1792e0..544d88794a9 100644 --- a/test/cpp/microbenchmarks/bm_error.cc +++ b/test/cpp/microbenchmarks/bm_error.cc @@ -74,7 +74,7 @@ static void BM_ErrorCreateAndSetIntAndStr(benchmark::State& state) { grpc_error_set_int( GRPC_ERROR_CREATE_FROM_STATIC_STRING("GOAWAY received"), GRPC_ERROR_INT_HTTP2_ERROR, (intptr_t)0), - GRPC_ERROR_STR_RAW_BYTES, grpc_slice_from_static_string("raw bytes"))); + GRPC_ERROR_STR_RAW_BYTES, "raw bytes")); } track_counters.Finish(state); } @@ -97,8 +97,7 @@ static void BM_ErrorCreateAndSetStrLoop(benchmark::State& state) { grpc_error_handle error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error"); const char* str = "hello"; for (auto _ : state) { - error = grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, - grpc_slice_from_static_string(str)); + error = grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, str); } GRPC_ERROR_UNREF(error); track_counters.Finish(state); @@ -253,9 +252,9 @@ static void BM_ErrorGetStatus(benchmark::State& state) { grpc_core::ExecCtx exec_ctx; for (auto _ : state) { grpc_status_code status; - grpc_slice slice; - grpc_error_get_status(fixture.error(), fixture.deadline(), &status, &slice, - nullptr, nullptr); + std::string message; + grpc_error_get_status(fixture.error(), fixture.deadline(), &status, + &message, nullptr, nullptr); } track_counters.Finish(state); diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index d50029cde79..dd7d6dac40c 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -1051,30 +1051,6 @@ ], "uses_polling": false }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c", - "name": "error_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": false - }, { "args": [], "benchmark": false, @@ -4677,6 +4653,30 @@ ], "uses_polling": true }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "error_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": false + }, { "args": [], "benchmark": false,