grpc / grpc-dotnet

gRPC for .NET
Apache License 2.0
4.17k stars 766 forks source link

Can we set custom grpc settings? #1842

Open AnashOommen opened 2 years ago

AnashOommen commented 2 years ago

I'm trying to address https://github.com/googleads/google-ads-dotnet/issues/445 and noticed that there's no support for custom grpc settings.

How do I set the the following settings?

* grpc.max_metadata_size
* grpc.http_proxy

@jtattermusch @jskeet FYI.

jskeet commented 2 years ago

I'm on holiday at the moment. I'll respond to this when I'm back.

jskeet commented 2 years ago

(There is support for custom settings via GrpcChannelOptions, but I don't know about these specific options in different implementations.)

AnashOommen commented 2 years ago

I thought we ignored custom options in Gax? https://github.com/googleapis/gax-dotnet/blob/main/Google.Api.Gax.Grpc/GrpcNetClientAdapter.cs#L67

AnashOommen commented 2 years ago

Btw, here's why we need grpc.max_metadata_size: https://github.com/googleads/google-ads-dotnet/issues/94

jskeet commented 2 years ago

Yes, quite possibly. I'm on holiday, so haven't actually checked. The proxy is normally set in other ways - I don't know if Grpc.Net has support for maximum metadata size

JamesNK commented 2 years ago

The proxy is customized on SocketsHttpHandler which is set on GrpcChannelOptions.HttpHandler.

I don't think SocketsHttpHandler has the equivalent of grpc.max_metadata_size. Are large headers/trailers a problem with Grpc.Net.Client?

AnashOommen commented 2 years ago

@JamesNK yes, it is. I can replicate this at my end by uploading ~200 keywords with policy errors in Google Ads API.

If you can reach out to me at anash@google.com, I'd be happy to share some source code to replicate the issue. This requires credentials to make call to the Google Ads API server, so I can't post it here.

JamesNK commented 2 years ago

What is the exception? Can you add the message + stack trace to this issue?

gRPC removes some of the exception detail. To get the original error you might need to configure logging: https://docs.microsoft.com/en-us/aspnet/core/grpc/diagnostics?view=aspnetcore-6.0#grpc-client-logging

AnashOommen commented 2 years ago

Exception & Stack trace

 {"Can't get the call trailers because the call has not completed successfully."} | System.InvalidOperationException

   at Grpc.Net.Client.Internal.GrpcCall`2.GetTrailers()
   at Grpc.Net.Client.Internal.HttpClientCallInvoker.Callbacks`2.<>c.<.cctor>b__4_2(Object state)
   at Grpc.Core.AsyncCallState.GetTrailers() in /var/local/git/grpc/src/csharp/Grpc.Core.Api/AsyncCallState.cs:line 81
   at Grpc.Core.AsyncUnaryCall`1.GetTrailers() in /var/local/git/grpc/src/csharp/Grpc.Core.Api/AsyncUnaryCall.cs:line 132
   at Google.Ads.GoogleAds.Logging.LoggingHandler.<HandleAsyncUnaryLogging>d__12`2.MoveNext() in C:\src\MyProjects\Common Library Base\simply-live\Google.Ads.GoogleAds.Core\src\Logging\LoggingHandler.cs:line 120
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionDispatchInfo.cs:line 56
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__140_1(Object state) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:line 1898
   at System.Threading.QueueUserWorkItemCallbackDefaultContext.Execute() in /_/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs:line 856
   at System.Threading.ThreadPoolWorkQueue.Dispatch() in /_/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs:line 655
   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback() in /_/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs:line 29

Logging

info: Grpc.Net.Client.Internal.GrpcCall[17]
      Error reading message.
      System.IO.IOException: The request was aborted.
       ---> System.Net.Http.HPack.HPackDecodingException: The HTTP headers length exceeded the set limit of 65536 bytes.         at System.Net.Http.HPack.HPackDecoder.OnStringLength(Int32 length, State nextState)
         at System.Net.Http.HPack.HPackDecoder.ParseHeaderValueLength(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.ParseHeaderName(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.ParseHeaderNameLength(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.Parse(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.DecodeInternal(ReadOnlySpan`1 data, IHttpHeadersHandler handler)
         at System.Net.Http.Http2Connection.ProcessHeadersFrame(FrameHeader frameHeader)
         at System.Net.Http.Http2Connection.ProcessIncomingFramesAsync()
         --- End of inner exception stack trace ---
         at System.Net.Http.Http2Connection.ThrowRequestAborted(Exception innerException)
         at System.Net.Http.Http2Connection.Http2Stream.CheckResponseBodyState()
         at System.Net.Http.Http2Connection.Http2Stream.TryReadFromBuffer(Span`1 buffer, Boolean partOfSyncRead)
         at System.Net.Http.Http2Connection.Http2Stream.ReadDataAsync(Memory`1 buffer, HttpResponseMessage responseMessage, CancellationToken cancellationToken)
         at Grpc.Net.Client.StreamExtensions.ReadMessageAsync[TResponse](Stream responseStream, GrpcCall call, Func`2 deserializer, String grpcEncoding, Boolean singleMessage, CancellationToken cancellationToken)
info: Grpc.Net.Client.Internal.GrpcCall[3]
      Call failed with gRPC error status. Status code: 'Unavailable', Message: 'Error starting gRPC call. IOException: The request was aborted. HPackDecodingException: The HTTP headers length exceeded the set limit of 65536 bytes.'.
JamesNK commented 2 years ago

Ok. I think this can be increased by setting SocketsHttpHandler.MaxResponseHeadersLength.

AnashOommen commented 2 years ago

Out of curiosity, I tried this out:

GrpcChannelOptions options = new GrpcChannelOptions();
options.MaxReceiveMessageSize = 256 * 1024 * 1204;
options.Credentials = credentials;

ILoggerFactory factory = LoggerFactory.Create(builder =>builder.AddConsole());

options.LoggerFactory = factory;
#if NET5_0_OR_GREATER
  options.HttpHandler = new SocketsHttpHandler()
  {
    MaxResponseHeadersLength = 20 * 1024 * 1024
  };
#endif
return GrpcChannel.ForAddress(uri, options);

And I get this log

fail: Grpc.Net.Client.Internal.GrpcCall[6]
      Error starting gRPC call.
      System.Net.Http.HttpRequestException: An error occurred while sending the request.
       ---> System.IO.IOException: The request was aborted.
       ---> System.Net.Http.HttpRequestException: The HTTP response headers length exceeded the set limit of 21474836480 bytes.
         at System.Net.Http.Http2Connection.Http2Stream.OnHeader(ReadOnlySpan`1 name, ReadOnlySpan`1 value)
         at System.Net.Http.Http2Connection.Http2Stream.System.Net.Http.IHttpHeadersHandler.OnStaticIndexedHeader(Int32 index)
         at System.Net.Http.HPack.HPackDecoder.OnIndexedHeaderField(Int32 index, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.Parse(ReadOnlySpan`1 data, Int32& currentIndex, IHttpHeadersHandler handler)
         at System.Net.Http.HPack.HPackDecoder.DecodeInternal(ReadOnlySpan`1 data, IHttpHeadersHandler handler)
         at System.Net.Http.Http2Connection.ProcessHeadersFrame(FrameHeader frameHeader)
         at System.Net.Http.Http2Connection.ProcessIncomingFramesAsync()
         --- End of inner exception stack trace ---
         at System.Net.Http.Http2Connection.ThrowRequestAborted(Exception innerException)
         at System.Net.Http.Http2Connection.Http2Stream.CheckResponseBodyState()
         at System.Net.Http.Http2Connection.Http2Stream.TryEnsureHeaders()
         at System.Net.Http.Http2Connection.Http2Stream.ReadResponseHeadersAsync(CancellationToken cancellationToken)
         at System.Net.Http.Http2Connection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         --- End of inner exception stack trace ---
         at System.Net.Http.Http2Connection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
         at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         at Grpc.Shared.TelemetryHeaderHandler.SendAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
         at Grpc.Net.Client.Balancer.Internal.BalancerHttpHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         at Grpc.Net.Client.Internal.GrpcCall`2.RunCall(HttpRequestMessage request, Nullable`1 timeout)
info: Grpc.Net.Client.Internal.GrpcCall[3]
      Call failed with gRPC error status. Status code: 'Unavailable', Message: 'Error starting gRPC call. HttpRequestException: An error occurred while sending the request. IOException: The request was aborted. HttpRequestException: The HTTP response headers length exceeded the set limit of 21474836480 bytes.'.

I have no idea why the library is complaining about a 20GB limit.

JamesNK commented 2 years ago

I think there is a 32-bit overflow inside the HTTP client.

MaxResponseHeadersLength is kilobytes so 20 megs (20 * 1024) is enough.

JamesNK commented 2 years ago

Logged a bug with maintainers of HTTP/2 client - https://github.com/dotnet/runtime/issues/73848

Use a smaller value for now.

AnashOommen commented 2 years ago

Thanks @JamesNK. I wonder, is there a chance to expose this in GrpcChannelOptions instead of doing this on GrpcChannelOptions::HttpHandler? Seems there's some logic being done here to create different types of handlers. I'd prefer if I don't have to replicate all that logic in my code just to be able to set a few custom settings.

Any other alternatives if this isn't an option?

JamesNK commented 2 years ago

There are too many settings on SocketsHttpHandler to expose them all. And settings don't apply to handler instances. e.g. the handler in WASM either doesn't have MaxResponseHeadersLength or Proxy, or will ignore them if set.

jtattermusch commented 2 years ago

We should probably document better what are the Grpc.Net.Client alternatives for well-known channel arguments (e.g. see https://github.com/grpc/grpc/blob/f268659bf183a39857064593b141f313845282d9/include/grpc/impl/codegen/grpc_types.h) when configuring the client/server. I know there's https://docs.microsoft.com/en-us/aspnet/core/grpc/configuration?view=aspnetcore-6.0#configure-client-options, which is a good start, but I don't think we document mapping from gRPC channel arg names (e.g. grpc.max_receive_message_length to the GrpcChannelOptions field names - in this case it's MaxReceiveMessageSize). If we had such mapping, it would be clear which arguments are supported / unsupported / partially supported (and we could document the workarounds). I don't think we want to add every possible field to GrpcChannelOptions class, but if there isn't a corresponding field there, we could documents alternative ways of setting it.

@JamesNK WDYT?

JamesNK commented 2 years ago

Adding a mapping between channel arg names and settings on HttpClientHandler/SocketsHttpHandler makes sense. It could be added here: https://docs.microsoft.com/en-us/aspnet/core/grpc/migration?view=aspnetcore-6.0

AnashOommen commented 2 years ago

@JamesNK The reason why I asked to expose grpc.max_metadata_size and grpc.http_proxy on GrpcChannelOptions is because those are well-known gRPC channel settings and in my mind, the ideal place to expose those settings would be in GrpcChannelOptions.

Of course, there are situations where those settings are not supported at all / there are alternate ways to achieve the same effect, and the documentation will go a long way towards achieving that goal. Then we can have discussions around whether the alternative workaround is too implementation-specific (and a burden on end user) and whether it should be promoted to GrpcChannelOptions instead. Even the Gax implementation isn't there yet in that sense.

The goal is definitely not to expose every setting on SocketHttpHandler on GrpcChannelOptions.

Thanks @jtattermusch for linking the correct documentation pages.

halter73 commented 2 years ago

Triage decision:

dkapellusch commented 3 days ago

Hey all, I'm running into a problem using a local proxy (ProxyMan) that I think may be related to this.

According to the result of this issue, replacing the handler in the channel options should resolve the problem, but I cannot find a way to specify that using Google.Api.Gax.Grpc.

Any suggestions or workarounds?

jskeet commented 2 days ago

@dkapellusch: See https://github.com/googleapis/google-cloud-dotnet/blob/main/issues/Issue12318/Workaround/Program.cs for an example of replacing the handler when working with GrpcNetClientAdapter.

dkapellusch commented 1 day ago

Thank you @jskeet!
I ended up putting a local handler in place, something like this:


internal static void ModifyGrpcChannelOptions(Grpc.Net.Client.GrpcChannelOptions options)
{
    options.HttpHandler = new LocalBypassProxyHandler(new SocketsHttpHandler
    {
        EnableMultipleHttp2Connections = true,
        Proxy = null,
        UseProxy = false,
    });
}