fix: call ResponseMetadataHanlder#onTrailers before calling onClose

[mutianf]

metadataHandler.onTrailers(..) should be called before onClose(..). With the current implementation, I don't think it's possible for ServerStreamingCallables or UnaryCallable#futureCalls() to get the trailers because the trailers are set after the call is closed?

The comment in the code and test case also only showed synchronized calls for UnaryCallables.

Use a server stream call as an example, the way I'm trying to access the trailer is:

class MyStreamingCallable<RequestT, ResponseT> extends ServerStreamingCallable  {
  @Override
  public void call(RequestT request, ResponseObserver<ResponseT> responseObserver, ApiCallContext context) {
      final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata();
      MyResponseObserver<ResponseT> innerObserver = new MyResponseObserver<>(responseObserver, responseMetadata);
      ApiCallContext withResponseMetadata = responseMetadata.addHandlers(context);
      innerCallable.call(request, innerObserver, withResponseMetadata);
  }

  // Response observer class
  private class MyResponseObserver<ResponseT> implements ResponseObserver<ResponseT> {
    @Override
    public void onComplete() {
      // With the current implementation, trailers will be null because we're still calling #onClose()
      Metadata trailers = responseMetadata.getTrailingMetadata();
      ...
      outerObserver.onComplete();
    }

    // onError will be similar to onComplete
  }
}

Unless GrpcResponseMetadata is not intended to be used this way?

[chanseokoh]

If you want to get the trailer on an event, I guess you can just extend GrpcResponseMetadata, which implements ResponseMetadataHandler.onTrailers()? Or, implement ResponseMetadataHandler.onTrailers() yourself?

But to be fair, I'm not actually familiar with this.

[mutianf]

If you want to get the trailer on an event, I guess you can just extend GrpcResponseMetadata, which implements ResponseMetadataHandler.onTrailers()? Or, implement ResponseMetadataHandler.onTrailers() yourself?

But to be fair, I'm not actually familiar with this.

Yes, it shouldn't be hard to get the trailers. I guess I'm proposing this change because I thought GrpcResponseMetadata gives client libraries a nice and easy way to access headers and trailers, so we won't need to implement our own versions of interceptors etc in the client libraries. But with the current implementation I'm not sure how I can get the trailers easily 😢 which defeats the point of providing a ResponeHandler class.

[vam-google]

LGTM