[arrow] branch master updated: ARROW-5095: [Flight][C++] Expose server error message in DoGet

[we...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 827cd43  ARROW-5095: [Flight][C++] Expose server error message in DoGet
827cd43 is described below

commit 827cd439d870316f853785c3bad49bd90b1be8a4
Author: David Li <Da...@twosigma.com>
AuthorDate: Thu Apr 4 17:18:10 2019 -0500

    ARROW-5095: [Flight][C++] Expose server error message in DoGet
    
    An easy way to reproduce this is a broken Python server that raises an exception.
    
    Author: David Li <Da...@twosigma.com>
    
    Closes #4097 from lihalite/arrow-5095 and squashes the following commits:
    
    0377f47d6 <David Li> Expose server error message in DoGet
---
 cpp/src/arrow/flight/client.cc      |  3 +++
 cpp/src/arrow/flight/flight-test.cc | 16 ++++++++++++++++
 cpp/src/arrow/flight/test-server.cc |  8 ++++++++
 3 files changed, 27 insertions(+)

diff --git a/cpp/src/arrow/flight/client.cc b/cpp/src/arrow/flight/client.cc
index 30e9b61..154d34f 100644
--- a/cpp/src/arrow/flight/client.cc
+++ b/cpp/src/arrow/flight/client.cc
@@ -296,6 +296,9 @@ class FlightClient::FlightClientImpl {
     std::shared_ptr<Schema> schema;
     internal::FlightData data;
     if (!stream->Read(reinterpret_cast<pb::FlightData*>(&data))) {
+      // Get the gRPC status if not OK, to get any server error
+      // messages
+      RETURN_NOT_OK(internal::FromGrpcStatus(stream->Finish()));
       return Status(StatusCode::Invalid, "No data in Flight stream");
     }
     std::unique_ptr<ipc::Message> message;
diff --git a/cpp/src/arrow/flight/flight-test.cc b/cpp/src/arrow/flight/flight-test.cc
index 8a1adb2..b099146 100644
--- a/cpp/src/arrow/flight/flight-test.cc
+++ b/cpp/src/arrow/flight/flight-test.cc
@@ -25,6 +25,7 @@
 #include <thread>
 #include <vector>
 
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
 #include "arrow/ipc/test-common.h"
@@ -258,5 +259,20 @@ TEST_F(TestFlightClient, DoAction) {
   ASSERT_EQ(nullptr, result);
 }
 
+TEST_F(TestFlightClient, Issue5095) {
+  // Make sure the server-side error message is reflected to the
+  // client
+  Ticket ticket1{"ARROW-5095-fail"};
+  std::unique_ptr<RecordBatchReader> stream;
+  Status status = client_->DoGet(ticket1, &stream);
+  ASSERT_RAISES(IOError, status);
+  ASSERT_THAT(status.message(), ::testing::HasSubstr("Server-side error"));
+
+  Ticket ticket2{"ARROW-5095-success"};
+  status = client_->DoGet(ticket2, &stream);
+  ASSERT_RAISES(IOError, status);
+  ASSERT_THAT(status.message(), ::testing::HasSubstr("No data"));
+}
+
 }  // namespace flight
 }  // namespace arrow
diff --git a/cpp/src/arrow/flight/test-server.cc b/cpp/src/arrow/flight/test-server.cc
index a2f6158..316d89f 100644
--- a/cpp/src/arrow/flight/test-server.cc
+++ b/cpp/src/arrow/flight/test-server.cc
@@ -75,6 +75,14 @@ class FlightTestServer : public FlightServerBase {
 
   Status DoGet(const Ticket& request,
                std::unique_ptr<FlightDataStream>* data_stream) override {
+    // Test for ARROW-5095
+    if (request.ticket == "ARROW-5095-fail") {
+      return Status::UnknownError("Server-side error");
+    }
+    if (request.ticket == "ARROW-5095-success") {
+      return Status::OK();
+    }
+
     std::shared_ptr<RecordBatchReader> batch_reader;
     RETURN_NOT_OK(GetBatchForFlight(request, &batch_reader));