Cool yeah, I had noticed that and it is definitely something that should be fixed.
I think that is only one part of it though, right? We'd still need to toss the keys in the metadata for it to be picked up on the C++ side. I'm not sure if that set of keys is documented or even appropriate to rely on cross-language though. On Wed, Aug 19, 2020 at 2:09 PM David Li <li.david...@gmail.com> wrote: > Ah - I see. I filed https://issues.apache.org/jira/browse/ARROW-9802. > > At a quick glance, I think what's wrong is StatusUtils.toGrpcStatus > doesn't copy the metadata from the CallStatus into the gRPC status it > constructs. This should be an easy enough fix if you'd like to look at > it. (Adding an integration test to go with it would be ideal.) > > Constructing a gRPC status directly is also fine. In the future we may > explore non-gRPC transports, so it may not be as portable, but that's > probably a ways off. > > Thanks, > David > > On 8/19/20, Patrick Woody <patrick.woo...@gmail.com> wrote: > > Hey David, > > > > Appreciate the quick response! > > > > I had been doing it that way, but I found that If I was sending something > > along the lines of: > > > > ErrorFlightMetadata errorFlightMetadata = new ErrorFlightMetadata(); > > errorFlightMetadata.insert("my_custom_key", "hello"); > > listener.error(CallStatus.INTERNAL > > .withDescription("Testing description") > > .withCause(new RuntimeException("My cause")) > > .withMetadata(errorFlightMetadata) > > .toRuntimeException()); > > > > > > I would only receive the description back by the time it was throwing a > > Python error: > > > > FlightInternalError: gRPC returned internal error, with message: > > Testing description. Client context: IOError: Server never sent a data > > message. Detail: Internal. gRPC client debug context: > > {"created":"@1597858462.086707300","description":"Error received from > > peer > > > ipv6:[::1]:12233","file":"src/core/lib/surface/call.cc","file_line":1056,"grpc_message":"Testing > > description","grpc_status":13} > > > > (catching the exception and checking FlightInternalError.extra_info > returns > > nothing as well) > > > > However if I manually create a grpc status exception like so: > > > > private static Metadata.Key<byte[]> arrowStatusDetail = > > Metadata.Key.of("x-arrow-status-detail-bin", > > Metadata.BINARY_BYTE_MARSHALLER); > > private static Metadata.Key<byte[]> grpcStatusDetail = > > Metadata.Key.of("grpc-status-details-bin", > > Metadata.BINARY_BYTE_MARSHALLER); > > private static Metadata.Key<String> statusCode = > > Metadata.Key.of("x-arrow-status", Metadata.ASCII_STRING_MARSHALLER); > > private static Metadata.Key<byte[]> message = > > Metadata.Key.of("x-arrow-status-message-bin", > > Metadata.BINARY_BYTE_MARSHALLER); > > > > Metadata metadata = new Metadata(); > > metadata.put(arrowStatusDetail, "my_internal_details".getBytes()); > > metadata.put(grpcStatusDetail, "this_is_in_extra_info".getBytes()); > > metadata.put(statusCode, "1"); > > metadata.put(message, "my_internal_message".getBytes()); > > return new StatusRuntimeException(Status.INTERNAL, metadata); > > > > Then I receive this back on the Python side: > > > > FlightInternalError: my_internal_message. Detail: my_internal_details. > > Client context: IOError: Server never sent a data message. Detail: > > Internal. gRPC client debug context: > > {"created":"@1597859023.244971700","description":"Error received from > > peer > > > ipv6:[::1]:12233","file":"src/core/lib/surface/call.cc","file_line":1056,"grpc_message":"","grpc_status":13} > > > > and FlightInternalError.extra_info contains the bytes for > > "this_is_in_extra_info" - I can pretty much put whatever I need in there > > to add richer metadata and utilize it on the client. > > > > It feels a bit awkward / maybe incorrect to dive into the C++ code to > > hijack those metadata keys just to transport extra metadata from Java -> > > C++. If the above approach with CallStatus is incorrect for bringing > extra > > data to the client then let me know. > > > > Best, > > Pat > > > > > > > > On Wed, Aug 19, 2020 at 12:59 PM David Li <li.david...@gmail.com> wrote: > > > >> Hey Patrick, > >> > >> How are you sending the error to the client from the server side? You > >> should be calling `listener.error` or `listener.onError` with a > >> `FlightRuntimeException` (which you can get via > >> CallStatus.<STATUS_NAME>.withDescription(...).toRuntimeException); > >> this will get the status code and message to the client. > >> > >> The error metadata is if you need to send additional structured data > >> as part of the error. > >> > >> Best, > >> David > >> > >> On 8/19/20, Patrick Woody <patrick.woo...@gmail.com> wrote: > >> > Hey all, > >> > > >> > I'm working on a project that has a Java arrow flight server and has a > >> > pyarrow client, both version 1.0.0. I've been trying to get richer > >> > errors > >> > back to the python client instead of the generic "grpc error ...". > >> > > >> > I see that the spec explicitly doesn't try to define metadata: > >> > https://arrow.apache.org/docs/format/Flight.html#error-handling, but > I > >> > think it would certainly be useful to have them work together. It > >> > appears > >> > that the Java implementation doesn't seem to send the > >> > ErrorFlightMetadata > >> > and the C++ code has its own way of best effort extracting of > >> > additional > >> > metadata from the response > >> > > >> > https://github.com/apache/arrow/blob/master/cpp/src/arrow/flight/internal.cc#L86 > >> > > >> > As of now, I'm just manually making the grpc metadata/exception so > that > >> > pyarrow understands it but I'm curious if this is a problem others > have > >> run > >> > into / if I'm doing something silly. Similarly, I am wondering if this > >> > is > >> > something that requires cross-language definition - I'm a little > >> > unclear > >> on > >> > the divide between what is considered a part of the Flight spec v.s. > >> > implementation specific. Happy to help if there is some follow up work > >> > here. > >> > > >> > Thanks! > >> > Pat > >> > > >> > > >
