The intent is that the application can define/use whatever keys it wants - though C++/Python don't expose that fully, which is another shortcoming that should be addressed. We should also perhaps consider standardizing/exposing a way to set that extra_info header in Python/C++ from Java and vice versa.
David On 8/19/20, Patrick Woody <patrick.woo...@gmail.com> wrote: > 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 >> >> > >> >> >> > >> >
