ARROW-16069: [C++][FlightRPC] Refactor out gRPC error code handling

cpp/src/arrow/flight/flight_internals_test.cc

@@ -482,5 +482,61 @@ TEST_F(TestCookieParsing, CookieCache) {
   AddCookieVerifyCache({"id0=0;", "id1=1;", "id2=2"}, "id0=0; id1=1; id2=2");
 }
 
+// ----------------------------------------------------------------------
+// Transport abstraction tests
+
+TEST(TransportErrorHandling, ReconstructStatus) {
+  Status current = Status::Invalid("Base error message");
+  // Invalid code
+  EXPECT_RAISES_WITH_MESSAGE_THAT(
+      Invalid,
+      ::testing::HasSubstr(". Also, server sent unknown or invalid Arrow status code -1"),
+      internal::ReconstructStatus("-1", current, util::nullopt, util::nullopt,
+                                  util::nullopt, /*detail=*/nullptr));
+  EXPECT_RAISES_WITH_MESSAGE_THAT(
+      Invalid,
+      ::testing::HasSubstr(
+          ". Also, server sent unknown or invalid Arrow status code foobar"),
+      internal::ReconstructStatus("foobar", current, util::nullopt, util::nullopt,
+                                  util::nullopt, /*detail=*/nullptr));
+
+  // Override code
+  EXPECT_RAISES_WITH_MESSAGE_THAT(
+      AlreadyExists, ::testing::HasSubstr("Base error message"),
+      internal::ReconstructStatus(
+          std::to_string(static_cast<int>(StatusCode::AlreadyExists)), current,
+          util::nullopt, util::nullopt, util::nullopt, /*detail=*/nullptr));
+
+  // Override message
+  EXPECT_RAISES_WITH_MESSAGE_THAT(
+      AlreadyExists, ::testing::HasSubstr("Custom error message"),
+      internal::ReconstructStatus(
+          std::to_string(static_cast<int>(StatusCode::AlreadyExists)), current,
+          "Custom error message", util::nullopt, util::nullopt, /*detail=*/nullptr));
+
+  // With detail
+  EXPECT_RAISES_WITH_MESSAGE_THAT(
+      AlreadyExists,
+      ::testing::AllOf(::testing::HasSubstr("Custom error message"),
+                       ::testing::HasSubstr(". Detail: Detail message")),
+      internal::ReconstructStatus(
+          std::to_string(static_cast<int>(StatusCode::AlreadyExists)), current,
+          "Custom error message", "Detail message", util::nullopt, /*detail=*/nullptr));
+
+  // With detail and bin
+  auto reconstructed = internal::ReconstructStatus(
+      std::to_string(static_cast<int>(StatusCode::AlreadyExists)), current,
+      "Custom error message", "Detail message", "Binary error details",
+      /*detail=*/nullptr);
+  EXPECT_RAISES_WITH_MESSAGE_THAT(
+      AlreadyExists,
+      ::testing::AllOf(::testing::HasSubstr("Custom error message"),
+                       ::testing::HasSubstr(". Detail: Detail message")),
+      reconstructed);
+  auto detail = FlightStatusDetail::UnwrapStatus(reconstructed);
+  ASSERT_NE(detail, nullptr);
+  ASSERT_EQ(detail->extra_info(), "Binary error details");
+}
+
 }  // namespace flight
 }  // namespace arrow

cpp/src/arrow/flight/flight_test.cc

@@ -109,6 +109,12 @@ class GrpcCudaDataTest : public CudaDataTest {
 };
 ARROW_FLIGHT_TEST_CUDA_DATA(GrpcCudaDataTest);
 
+class GrpcErrorHandlingTest : public ErrorHandlingTest {
+ protected:
+  std::string transport() const override { return "grpc"; }
+};
+ARROW_FLIGHT_TEST_ERROR_HANDLING(GrpcErrorHandlingTest);
+
 //------------------------------------------------------------
 // Ad-hoc gRPC-specific tests
 

cpp/src/arrow/flight/test_definitions.cc

@@ -1363,5 +1363,128 @@ void CudaDataTest::TestDoExchange() {
 
 #endif
 
+//------------------------------------------------------------
+// Test error handling
+
+namespace {
+constexpr std::initializer_list<StatusCode> kStatusCodes = {
+    StatusCode::OutOfMemory,
+    StatusCode::KeyError,
+    StatusCode::TypeError,
+    StatusCode::Invalid,
+    StatusCode::IOError,
+    StatusCode::CapacityError,
+    StatusCode::IndexError,
+    StatusCode::Cancelled,
+    StatusCode::UnknownError,
+    StatusCode::NotImplemented,
+    StatusCode::SerializationError,
+    StatusCode::RError,
+    StatusCode::CodeGenError,
+    StatusCode::ExpressionValidationError,
+    StatusCode::ExecutionError,
+    StatusCode::AlreadyExists,
+};
+
+constexpr std::initializer_list<FlightStatusCode> kFlightStatusCodes = {
+    FlightStatusCode::Internal,     FlightStatusCode::TimedOut,
+    FlightStatusCode::Cancelled,    FlightStatusCode::Unauthenticated,
+    FlightStatusCode::Unauthorized, FlightStatusCode::Unavailable,
+    FlightStatusCode::Failed,
+};
+arrow::Result<StatusCode> TryConvertStatusCode(int raw_code) {
+  for (const auto status_code : kStatusCodes) {
+    if (raw_code == static_cast<int>(status_code)) {
+      return status_code;
+    }
+  }
+  return Status::Invalid(raw_code);
+}
+arrow::Result<FlightStatusCode> TryConvertFlightStatusCode(int raw_code) {
+  for (const auto status_code : kFlightStatusCodes) {
+    if (raw_code == static_cast<int>(status_code)) {
+      return status_code;
+    }
+  }
+  return Status::Invalid(raw_code);
+}
+
+class TestStatusDetail : public StatusDetail {
+ public:
+  const char* type_id() const override { return "test-status-detail"; }
+  std::string ToString() const override { return "Custom status detail"; }
+};
+class ErrorHandlingTestServer : public FlightServerBase {
+ public:
+  Status GetFlightInfo(const ServerCallContext& context, const FlightDescriptor& request,
+                       std::unique_ptr<FlightInfo>* info) override {
+    if (request.path.size() >= 2) {
+      const int raw_code = std::atoi(request.path[0].c_str());
+      ARROW_ASSIGN_OR_RAISE(StatusCode code, TryConvertStatusCode(raw_code));
+
+      if (request.path.size() == 2) {
+        return Status(code, request.path[1]);
+      } else if (request.path.size() == 3) {
+        return Status(code, request.path[1], std::make_shared<TestStatusDetail>());
+      } else {
+        const int raw_code = std::atoi(request.path[2].c_str());
+        ARROW_ASSIGN_OR_RAISE(FlightStatusCode flight_code,
+                              TryConvertFlightStatusCode(raw_code));
+        return Status(code, request.path[1],
+                      std::make_shared<FlightStatusDetail>(flight_code, request.path[3]));
+      }
+    }
+    return Status::NotImplemented("NYI");
+  }
+};
+}  // namespace
+
+void ErrorHandlingTest::SetUp() {
+  ASSERT_OK_AND_ASSIGN(auto location, Location::ForScheme(transport(), "127.0.0.1", 0));
+  ASSERT_OK(MakeServer<ErrorHandlingTestServer>(
+      location, &server_, &client_,
+      [](FlightServerOptions* options) { return Status::OK(); },
+      [](FlightClientOptions* options) { return Status::OK(); }));
+}
+void ErrorHandlingTest::TearDown() {
+  ASSERT_OK(client_->Close());
+  ASSERT_OK(server_->Shutdown());
+}
+
+void ErrorHandlingTest::TestGetFlightInfo() {
+  std::unique_ptr<FlightInfo> info;
+  for (const auto code : kStatusCodes) {
+    ARROW_SCOPED_TRACE("C++ status code: ", static_cast<int>(code));
+    auto descr = FlightDescriptor::Path(
+        {std::to_string(static_cast<int>(code)), "Expected message"});
+    auto status = client_->GetFlightInfo(descr).status();
+    EXPECT_EQ(status.code(), code);
+    EXPECT_THAT(status.message(), ::testing::HasSubstr("Expected message"));
+
+    // Custom status detail
+    descr = FlightDescriptor::Path(
+        {std::to_string(static_cast<int>(code)), "Expected message", ""});
+    status = client_->GetFlightInfo(descr).status();
+    EXPECT_EQ(status.code(), code);
+    EXPECT_THAT(status.message(), ::testing::HasSubstr("Expected message"));
+    EXPECT_THAT(status.message(), ::testing::HasSubstr("Detail: Custom status detail"));
+
+    // Flight status detail
+    for (const auto flight_code : kFlightStatusCodes) {
+      ARROW_SCOPED_TRACE("Flight status code: ", static_cast<int>(flight_code));
+      descr = FlightDescriptor::Path(
+          {std::to_string(static_cast<int>(code)), "Expected message",
+           std::to_string(static_cast<int>(flight_code)), "Expected detail message"});
+      status = client_->GetFlightInfo(descr).status();
+      // Don't check status code, since Flight code may override it
+      EXPECT_THAT(status.message(), ::testing::HasSubstr("Expected message"));
+      auto detail = FlightStatusDetail::UnwrapStatus(status);
+      ASSERT_NE(detail, nullptr);
+      EXPECT_EQ(detail->code(), flight_code);
+      EXPECT_THAT(detail->extra_info(), ::testing::HasSubstr("Expected detail message"));
+    }
+  }
+}
+
 }  // namespace flight
 }  // namespace arrow

cpp/src/arrow/flight/test_definitions.h

@@ -255,5 +255,24 @@ class ARROW_FLIGHT_EXPORT CudaDataTest : public FlightTest {
   TEST_F(FIXTURE, TestDoPut) { TestDoPut(); }                                \
   TEST_F(FIXTURE, TestDoExchange) { TestDoExchange(); }
 
+/// \brief Tests of error handling.
+class ARROW_FLIGHT_EXPORT ErrorHandlingTest : public FlightTest {
+ public:
+  void SetUp() override;
+  void TearDown() override;
+
+  // Test methods
+  void TestGetFlightInfo();
+
+ private:
+  std::unique_ptr<FlightClient> client_;
+  std::unique_ptr<FlightServerBase> server_;
+};
+
+#define ARROW_FLIGHT_TEST_ERROR_HANDLING(FIXTURE)                                 \
+  static_assert(std::is_base_of<ErrorHandlingTest, FIXTURE>::value,               \
+                ARROW_STRINGIFY(FIXTURE) " must inherit from ErrorHandlingTest"); \
+  TEST_F(FIXTURE, TestGetFlightInfo) { TestGetFlightInfo(); }
+
 }  // namespace flight
 }  // namespace arrow