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

Add graceful handling for OperationCanceledException in Client #723

Closed
nothrow opened this issue Jan 16, 2020 · 3 comments · Fixed by #724
Closed

Add graceful handling for OperationCanceledException in Client #723

nothrow opened this issue Jan 16, 2020 · 3 comments · Fixed by #724
Labels
enhancement New feature or request

Comments

@nothrow
Copy link

nothrow commented Jan 16, 2020

Is your feature request related to a problem? Please describe.

When there is cancellation during reading from stream, two log messages of "fail" level are emitted. This happens for any request that returns stream.

var cancellationTokenSource = new CancellationTokenSource(5000);
var client = new MetadataService.MetadataServiceClient(_channel);
using var subscription = client.WatchChanges(new Empty()); // this is just for demonstration, I don't think proto is necessary
await foreach (var change in subscription.ResponseStream.ReadAllAsync(cancellationTokenSource.Token))
                            {
                                Console.WriteLine(change.Elements.Count);
                            }
fail: Grpc.Net.Client.Internal.GrpcCall[17]
      Error reading message.
System.Threading.Tasks.TaskCanceledException: The operation was canceled.
   at System.Net.Http.CancellationHelper.ThrowOperationCanceledException(Exception innerException, CancellationToken cancellationToken)
   at System.Net.Http.CancellationHelper.ThrowIfCancellationRequested(CancellationToken cancellationToken)
   at System.Net.Http.Http2Connection.Http2Stream.GetCancelableWaiterTask(CancellationToken cancellationToken)
   at System.Net.Http.Http2Connection.Http2Stream.ReadDataAsync(Memory`1 buffer, HttpResponseMessage responseMessage, CancellationToken cancellationToken)
   at Grpc.Net.Client.StreamExtensions.ReadMessageAsync[TResponse](Stream responseStream, ILogger logger, Func`2 deserializer, String grpcEncoding, Nullable`1 maximumMessageSize, Dictionary`2 compressionProviders, Boolean singleMessage, CancellationToken cancellationToken)
fail: Grpc.Net.Client.Internal.GrpcCall[3]
      Call failed with gRPC error status. Status code: 'Cancelled', Message: 'gRPC call disposed.'.

Describe the solution you'd like

I believe cancelling should not have 'fail' severity - I suggest debug is enough. This issue floods my logs on shutdown of the service, and I find it annoying.

Fixing this specific instance is pretty easy and I can PR it, I would, however, like, to suggest some more organized effort in graceful handling of cancellation.

Describe alternatives you've considered

Grpc.Core does not suffer from this flaw. I switched, for client, to Grpc.Core for now.

@nothrow nothrow added the enhancement New feature or request label Jan 16, 2020
@JamesNK
Copy link
Member

JamesNK commented Jan 16, 2020

This is already fixed #697

You can wait until the next release or use a nightly package from the NuGet feed. Instructions: https://github.com/grpc/grpc-dotnet#grpc-nuget-feed

@JamesNK JamesNK closed this as completed Jan 16, 2020
@JamesNK JamesNK reopened this Jan 16, 2020
@JamesNK
Copy link
Member

JamesNK commented Jan 16, 2020

Actually, that issue was on the server. Let me test what happens in the client.

@nothrow
Copy link
Author

nothrow commented Jan 16, 2020

https://github.com/grpc/grpc-dotnet/blob/master/src/Grpc.Net.Client/Internal/StreamExtensions.cs#L176

Here it looks like there is no special case for cancellation, and client just log everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants