Skip to content

Commit

Permalink
Add conversion method for absl::Status to grpc_error* (grpc#25896)
Browse files Browse the repository at this point in the history
  • Loading branch information
drfloob authored Apr 8, 2021
1 parent ca769f6 commit a63f188
Show file tree
Hide file tree
Showing 9 changed files with 198 additions and 2 deletions.
36 changes: 36 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,7 @@ if(gRPC_BUILD_TESTS)
add_dependencies(buildtests_cxx duplicate_header_bad_client_test)
add_dependencies(buildtests_cxx end2end_test)
add_dependencies(buildtests_cxx error_details_test)
add_dependencies(buildtests_cxx error_utils_test)
add_dependencies(buildtests_cxx evaluate_args_test)
add_dependencies(buildtests_cxx eventmanager_libuv_test)
if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX)
Expand Down Expand Up @@ -10242,6 +10243,41 @@ target_link_libraries(error_details_test
)


endif()
if(gRPC_BUILD_TESTS)

add_executable(error_utils_test
test/core/transport/error_utils_test.cc
third_party/googletest/googletest/src/gtest-all.cc
third_party/googletest/googlemock/src/gmock-all.cc
)

target_include_directories(error_utils_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_utils_test
${_gRPC_PROTOBUF_LIBRARIES}
${_gRPC_ALLTARGETS_LIBRARIES}
grpc_test_util
)


endif()
if(gRPC_BUILD_TESTS)

Expand Down
9 changes: 9 additions & 0 deletions build_autogenerated.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5042,6 +5042,15 @@ targets:
deps:
- grpc++_error_details
- grpc_test_util
- name: error_utils_test
gtest: true
build: test
language: c++
headers: []
src:
- test/core/transport/error_utils_test.cc
deps:
- grpc_test_util
- name: evaluate_args_test
gtest: true
build: test
Expand Down
4 changes: 4 additions & 0 deletions src/core/lib/iomgr/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ grpc_error* grpc_error_create(const char* file, int line,
#define GRPC_ERROR_CREATE_FROM_COPIED_STRING(desc) \
grpc_error_create(__FILE__, __LINE__, grpc_slice_from_copied_string(desc), \
NULL, 0)
#define GRPC_ERROR_CREATE_FROM_STRING_VIEW(desc) \
grpc_error_create( \
__FILE__, __LINE__, \
grpc_slice_from_copied_buffer((desc).data(), (desc).size()), NULL, 0)

// Create an error that references some other errors. This function adds a
// reference to each error in errs - it does not consume an existing reference
Expand Down
10 changes: 10 additions & 0 deletions src/core/lib/transport/error_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,16 @@ absl::Status grpc_error_to_absl_status(grpc_error* error) {
GRPC_SLICE_LENGTH(message)));
}

grpc_error* absl_status_to_grpc_error(absl::Status status) {
// Special error checks
if (status.ok()) {
return GRPC_ERROR_NONE;
}
return grpc_error_set_int(
GRPC_ERROR_CREATE_FROM_STRING_VIEW(status.message()),
GRPC_ERROR_INT_GRPC_STATUS, static_cast<grpc_status_code>(status.code()));
}

bool grpc_error_has_clear_grpc_status(grpc_error* error) {
intptr_t unused;
if (grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &unused)) {
Expand Down
6 changes: 6 additions & 0 deletions src/core/lib/transport/error_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ void grpc_error_get_status(grpc_error* error, grpc_millis deadline,
/// Does NOT consume a ref to grpc_error.
absl::Status grpc_error_to_absl_status(grpc_error* error);

/// Utility function to convert an absl::Status \a status to grpc_error. Note
/// that this method does not return "special case" errors such as
/// GRPC_ERROR_CANCELLED, with the exception of GRPC_ERROR_NONE returned for
/// \a absl::OkStatus().
grpc_error* absl_status_to_grpc_error(absl::Status status);

/// A utility function to check whether there is a clear status code that
/// doesn't need to be guessed in \a error. This means that \a error or some
/// child has GRPC_ERROR_INT_GRPC_STATUS set, or that it is GRPC_ERROR_NONE or
Expand Down
14 changes: 14 additions & 0 deletions test/core/transport/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,20 @@ grpc_cc_test(
],
)

grpc_cc_test(
name = "error_utils_test",
srcs = ["error_utils_test.cc"],
external_deps = [
"gtest",
],
language = "C++",
deps = [
"//:gpr",
"//:grpc",
"//test/core/util:grpc_test_util",
],
)

grpc_cc_test(
name = "metadata_test",
srcs = ["metadata_test.cc"],
Expand Down
95 changes: 95 additions & 0 deletions test/core/transport/error_utils_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
//
// Copyright 2021 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

#include "src/core/lib/transport/error_utils.h"

#include <gtest/gtest.h>
#include "absl/status/status.h"

#include "src/core/lib/slice/slice_internal.h"
#include "test/core/util/test_config.h"

namespace {

// ---- Ok Status ----
TEST(ErrorUtilsTest, AbslOkToGrpcError) {
grpc_error* error = absl_status_to_grpc_error(absl::OkStatus());
ASSERT_EQ(GRPC_ERROR_NONE, error);
GRPC_ERROR_UNREF(error);
}

TEST(ErrorUtilsTest, GrpcSpecialErrorNoneToAbslStatus) {
absl::Status status = grpc_error_to_absl_status(GRPC_ERROR_NONE);
ASSERT_TRUE(status.ok());
ASSERT_EQ(status.message(), "");
}

// ---- Asymmetry of conversions of "Special" errors ----
TEST(ErrorUtilsTest, AbslStatusToGrpcErrorDoesNotReturnSpecialVariables) {
grpc_error* error =
absl_status_to_grpc_error(absl::CancelledError("Cancelled"));
ASSERT_NE(error, GRPC_ERROR_CANCELLED);
GRPC_ERROR_UNREF(error);
}

TEST(ErrorUtilsTest, GrpcSpecialErrorCancelledToAbslStatus) {
absl::Status status = grpc_error_to_absl_status(GRPC_ERROR_CANCELLED);
ASSERT_TRUE(absl::IsCancelled(status));
ASSERT_EQ(status.message(), "Cancelled");
}

TEST(ErrorUtilsTest, GrpcSpecialErrorOOMToAbslStatus) {
absl::Status status = grpc_error_to_absl_status(GRPC_ERROR_OOM);
ASSERT_TRUE(absl::IsResourceExhausted(status));
ASSERT_EQ(status.message(), "Out of memory");
}

// ---- Ordinary statuses ----
TEST(ErrorUtilsTest, AbslUnavailableToGrpcError) {
grpc_error* error =
absl_status_to_grpc_error(absl::UnavailableError("Making tea"));
// Status code checks
intptr_t code;
ASSERT_TRUE(grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &code));
ASSERT_EQ(static_cast<grpc_status_code>(code), GRPC_STATUS_UNAVAILABLE);
// Status message checks
grpc_slice 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);
GRPC_ERROR_UNREF(error);
}

TEST(ErrorUtilsTest, GrpcErrorUnavailableToAbslStatus) {
grpc_error* error = grpc_error_set_int(
GRPC_ERROR_CREATE_FROM_STATIC_STRING(
"weighted_target: all children report state TRANSIENT_FAILURE"),
GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
absl::Status status = grpc_error_to_absl_status(error);
ASSERT_TRUE(absl::IsUnavailable(status));
ASSERT_EQ(status.message(),
"weighted_target: all children report state TRANSIENT_FAILURE");
GRPC_ERROR_UNREF(error);
}

} // namespace

int main(int argc, char** argv) {
testing::InitGoogleTest(&argc, argv);
grpc::testing::TestEnvironment env(argc, argv);
return RUN_ALL_TESTS();
};
2 changes: 0 additions & 2 deletions test/core/uri/uri_parser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
*
*/

// TODO(hork): rewrite with googletest

#include "src/core/lib/uri/uri_parser.h"

#include "absl/strings/str_join.h"
Expand Down
24 changes: 24 additions & 0 deletions tools/run_tests/generated/tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -4367,6 +4367,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_utils_test",
"platforms": [
"linux",
"mac",
"posix",
"windows"
],
"uses_polling": true
},
{
"args": [],
"benchmark": false,
Expand Down

0 comments on commit a63f188

Please sign in to comment.