Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP resiliency features don't work with the .NET gRPC client #4923

Open
DamianEdwards opened this issue Feb 6, 2024 · 7 comments
Open

HTTP resiliency features don't work with the .NET gRPC client #4923

DamianEdwards opened this issue Feb 6, 2024 · 7 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-resilience

Comments

@DamianEdwards
Copy link
Member

The HTTP resiliency features, including those added by the IHttpClientBuilder.AddStandardResilienceHandler method, don't apply to gRPC calls despite them going through configured HttpClient instances. This is due to the gRPC stack not exposing error details at the HTTP request level in the way that the resiliency features expect (e.g. using HTTP status codes).

The following code example, typical of setting up a gRPC client in a .NET server application, will not actually result in the standard resiliency features being applied to gRPC calls:

builder.Services.AddGrpcClient<Basket.BasketClient>(o => o.Address = new("http://basket-api"))
    .AddStandardResilienceHandler();

Consider adding support for the standard resiliency patterns to the .NET gRPC client stack in a similar fashion to those added to the HttpClient stack so that resiliency features like Circuit Breaker can be easily added by default.

/Cc @JamesNK @davidfowl

@joperezr
Copy link
Member

joperezr commented Feb 6, 2024

@martintmk
Copy link
Contributor

The easy enhancement is to improve the HttpClientResiliencePredicates to also detect gRPC calls and handle retriable status codes:

internal static bool IsTransientHttpFailure(HttpResponseMessage response)

This should make both retry and circuit breaker strategy work for gRPC. The other issue is handling of streamed calls, which I am not sure how to address.

@JamesNK
Copy link
Member

JamesNK commented Feb 7, 2024

gRPC always return 200 status code. Failure is communicated in grpc-status trailer.

I haven't looked at how resilience works, but I'm guessing the retry happens inside a HTTP handler's SendAsync. gRPC supports streaming an error can occur long after response status is returned and SendAsync has run.

I think a known limitation will be that streaming gRPC calls won't be retried. However, failing unary calls should be detectable. Look for a 200 status code and also check the response headers for grpc-status. They will both be available in SendAsync.

@martintmk
Copy link
Contributor

Failure is communicated in grpc-status trailer.

The trailer is available only after the response body is finished reading, is that correct? I am wondering how we can ensure that trailer is available for gRPC calls. Otherwise, the retries won't work.

Will buffering the content work?

@JamesNK
Copy link
Member

JamesNK commented Feb 7, 2024

Will buffering the content work?

No.

If an error happens before any content is returned by the server, then grpc-status is in the headers. That is the scenario that will work. It's confusingly named Trailers-Only in the spec - https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses

@geeknoid geeknoid added area-resilience api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Feb 7, 2024
@soroshsabz
Copy link

ITNOA

Any plan to implement specific extensions for support gRPC in Microsoft.Extensions.Resilience?

thanks

@joperezr
Copy link
Member

joperezr commented Apr 2, 2024

Any plan to implement specific extensions for support gRPC in Microsoft.Extensions.Resilience?

That is what this issue is tracking. No committed timelines yet, so for now we just want to continue this discussion.

@iliar-turdushev iliar-turdushev added bug This issue describes a behavior which is not expected - a bug. and removed bug This issue describes a behavior which is not expected - a bug. labels Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-resilience
Projects
None yet
Development

No branches or pull requests

7 participants