Allow return of HTTP response headers from method invocations

[philliphoff]

Could there also be a way of extracting HTTP headers sent by an application with the response? For example, if the response was an opaque set of bytes, the Content-Type response header might be needed in order to differentiate between multiple types of data. Something of this sort:

class HttpResponseExtension
{
   IDictionary<string, string> Headers { get; }
}

class MethodResponse<TResponse>
{
    TResponse Content { get; }

    HttpResponseExtension Metadata { get; }
}

class DaprClient
{
    abstract Task<MethodResponse<TResponse>> InvokeMethodWithResponseAsync<TRequest, TResponse>(string appId, string methodName, HTTPExtension requestMetadata, CancellationToken);
}

@amolenk suggested a variation on this theme:

The service that's being called may be an HTTP service, but it could also be a gRPC service. So I'd do it slightly different:

class MethodResponse<TResponse>
{
    TResponse Content { get; }
}

class HttpResponse<TResponse> : MethodResponse<TResponse>
{
    IDictionary<string, string> Headers { get; }
}

class DaprClient
{
    abstract Task<MethodResponse<TResponse>> InvokeMethodAsync<TRequest, TResponse>(string appId, string methodName, HTTPExtension requestMetadata, CancellationToken);
}

Alternatively, at that point it may even be better to make separate invoke methods for Http and gRPC, like InvokeHttpMethod and InvokeGrpcMethod. The abstraction is already a bit leaky due to the need for the HttpExtension object.

RELEASE NOTE: ADD HTTP status code for error in method invocation.

[rynowak]

@vinayada1 as well

Agree, we didn't include the ability to expose the underlying HTTP semantics as part of the initial design here, but #399 is causing us to revisit this for the error case, and we should probably also expose it as an API for the successful case.

[vinayada1]

Adding a proposal here:-
Goal: Enable dotnet sdk to call http or gRPC servers with all the necessary fields and be able to receive responses including error messages.

The InvokeRequest and InvokeResponse should include all fields that are specific to http and gRPC protocols. We will treat http as a superset instead of having separate methods to invoke http and gRPC which allows more flexibility to the user.

Service invocation can use the method below:-
public abstract Task InvokeMethodRawAsync(
InvokeRequest request,
CancellationToken cancellationToken);

The Invoke Request will look as below:-

    public class InvokeRequest
    {
        public string AppId { get; set; }
        public string MethodName { get; set; }
        public byte[] Body { get; set; }
        public string ContentType { get; set; }
        public HTTPExtension HttpExtension { get; set; } // Set only in the case of Http
    }

The Invoke Response will look as below:-

    public class InvokeResponse
    {
        public InvokeRequest Request { get; set; }
        public byte[] Body { get; set; }
        public string ContentType { get; set; }
        public Dictionary<string, string> Headers { get; set; }
        public int? HttpStatusCode { get; set; }
        public GrpcStatusInfo GrpcStatusInfo { get; set; } 
    }

The GrpcStatusInfo will look as below:-

    public class GrpcStatusInfo 
    {
        
        public int GrpcStatusCode { get; set; }
        public string GrpcErrorMessage { get; set; }
        public int InnerHttpStatusCode { get; set; }
        public string InnerHttpErrorMessage { get; set; }
    }

For throwing exceptions, we will use the format below:-
public class ServiceInvocationException : Exception
{
public InvokeRequest Request { get; set; }
public InvokeResponse Response { get; set; }
}

[amolenk]

I like having access to the response headers, but I’m not so fond of needing to use a lower level method to achieve it. There really isn’t that much that the SDK adds on top of making the call to the Dapr instance. Using the proposed InvokeMethodRawAsync call, this is even less because now I have to serialize/deserialize the objects myself. If I really want that level of control I can always use an HttpClient directly to call the Dapr instance.

I’d propose to aligning this more to how the state API methods work:

TValue GetState<TValue>(...)
StateEntry<TValue> GetStateEntry<TValue>(...)

For service invocation, this would look like:

// Existing method:
TResponse InvokeMethod<TResponse>(string appId, string methodName, ...)

// Extra method to add:
ServiceInvocationResponse<TResponse> InvokeMethodRaw<TResponse>(
    string appId, string methodName, ...)

The ServiceInvocationResponse<TResponse> would look like:

public class InvokeResponse<TResponse>
{
    public InvokeRequest Request { get; set; }
    public TResponse Body { get; set; }
    public string ContentType { get; set; }
    public Dictionary<string, string> Headers { get; set; }
    public int? HttpStatusCode { get; set; }
    public GrpcStatusInfo GrpcStatusInfo { get; set; } 
}

Both the InvokeMethod and InvokeMethodRaw methods can throw ServiceInvocationException objects if something goes wrong.

Not sure if InvokeMethodRaw would be the best name though. An alternative would be to swap the naming and use:

ServiceInvocationResponse<TResponse> InvokeMethod<TResponse>(...)
TResponse InvokeMethodUnwrap<TResponse>(...)

[rynowak]

@amolenk - can you provide some more info about what you're looking for with headers - which headers? what's the use case?

[amolenk]

@rynowak The main use case for response headers is making calls to services that do not return all information in the response body, but include useful info in response headers as well. For example, a service that returns customer information but also includes an Etag response header to use for optimistic concurrency checks when updating the customer info.
On an earlier IoT project I was involved with there was also a service that provided access to a feed of telemetry data. Each request to the feed returned a response with a custom checkpoint response header to use in the next call.

I think this issue is bigger than only response headers though, because it's equally important to have access to the HTTP status code and error messages. I don't want to have to choose between the SDK doing serialization/deserialization for me and having access to headers/status/error info.

[rynowak]

@amolenk - thanks, that's the kind of details I was looking for.

@vinayada1 is driving the work here, but the last discussion we had was around incorporating your feedback.

The rough plan is to have 3 families of methods:

// These are examples, and not the full set of overloads

public abstract Task<TResponse> InvokeMethodAsync<TRequest, TResponse>(string appId, string methodName, TRequest value, CancellationToken cancellationToken);
public abstract Task<InvokeResponse<TResponse>> InvokeMethodWithResponseAsync<TRequest, TResponse>(string appId, string methodName, TRequest value, CancellationToken cancellationToken);
public abstract Task<InvokeResponse> InvokeMethodRawAsync(InvokeRequest request, CancellationToken cancellationToken);

The first two of these would throw an exception on non-2XX status that includes the InvokeResponse and thus the status code, headers, etc are still available.

Your thoughts? Does this match what you expected?

[amolenk]

@rynowak Yes, looks good to me. I think this would cover all scenarios.