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

Change exceptions to be more consistent with Grpc.Core #1092

Merged
merged 1 commit into from
Nov 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/Grpc.Net.Client/Internal/GrpcCall.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ internal sealed partial class GrpcCall<TRequest, TResponse> : GrpcCall, IDisposa
where TRequest : class
where TResponse : class
{
private const string ErrorStartingCallMessage = "Error starting gRPC call.";

private readonly CancellationTokenSource _callCts;
private readonly TaskCompletionSource<Status> _callTcs;
private readonly DateTime _deadline;
Expand Down Expand Up @@ -249,7 +251,7 @@ private async Task<Metadata> GetResponseHeadersCoreAsync()
}
catch (Exception ex)
{
ResolveException(ex, out _, out var resolvedException);
ResolveException(ErrorStartingCallMessage, ex, out _, out var resolvedException);
throw resolvedException;
}
}
Expand Down Expand Up @@ -592,7 +594,7 @@ private async Task RunCall(HttpRequestMessage request, TimeSpan? timeout)
catch (Exception ex)
{
Exception resolvedException;
ResolveException(ex, out status, out resolvedException);
ResolveException(ErrorStartingCallMessage, ex, out status, out resolvedException);

finished = FinishCall(request, diagnosticSourceEnabled, activity, status.Value);
_responseTcs?.TrySetException(resolvedException);
Expand All @@ -606,7 +608,7 @@ private async Task RunCall(HttpRequestMessage request, TimeSpan? timeout)
}
}

private void ResolveException(Exception ex, [NotNull] out Status? status, out Exception resolvedException)
internal void ResolveException(string summary, Exception ex, [NotNull] out Status? status, out Exception resolvedException)
{
if (ex is OperationCanceledException)
{
Expand All @@ -621,8 +623,9 @@ private void ResolveException(Exception ex, [NotNull] out Status? status, out Ex
else
{
var exceptionMessage = CommonGrpcProtocolHelpers.ConvertToRpcExceptionMessage(ex);
var statusCode = GrpcProtocolHelpers.ResolveRpcExceptionStatusCode(ex);

status = new Status(StatusCode.Internal, "Error starting gRPC call. " + exceptionMessage, ex);
status = new Status(statusCode, summary + " " + exceptionMessage, ex);
resolvedException = CreateRpcException(status.Value);
}
}
Expand Down
26 changes: 26 additions & 0 deletions src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Net.Sockets;
using System.Threading.Tasks;
using Grpc.Core;
using Grpc.Net.Compression;
Expand Down Expand Up @@ -376,5 +378,29 @@ public static bool TryGetStatusCore(HttpResponseHeaders headers, [NotNullWhen(tr
status = new Status((StatusCode)statusValue, grpcMessage);
return true;
}

public static StatusCode ResolveRpcExceptionStatusCode(Exception ex)
{
var current = ex;
do
{
// Grpc.Core tends to return Unavailable if there is a problem establishing the connection.
// Additional changes here are likely required for cases when Unavailable is being returned
// when it shouldn't be.
if (current is SocketException)
{
// SocketError.ConnectionRefused happens when port is not available.
// SocketError.HostNotFound happens when unknown host is specified.
return StatusCode.Unavailable;
}
else if (current is IOException)
{
// IOException happens if there is a protocol mismatch.
return StatusCode.Unavailable;
}
} while ((current = current.InnerException) != null);

return StatusCode.Internal;
}
}
}
6 changes: 6 additions & 0 deletions src/Grpc.Net.Client/Internal/HttpContentClientStreamReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ private async Task<bool> MoveNextCore(CancellationToken cancellationToken)

throw _call.CreateCanceledStatusException();
}
catch (Exception ex)
{
// Throw RpcException from MoveNext. Consistent with Grpc.Core.
_call.ResolveException("Error reading next message.", ex, out _, out var resolvedException);
throw resolvedException;
}
finally
{
ctsRegistration?.Dispose();
Expand Down
42 changes: 42 additions & 0 deletions test/Grpc.Net.Client.Tests/ConnectionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
#endregion

using System;
using System.IO;
using System.Net;
using System.Net.Http;
using System.Net.Sockets;
using System.Threading.Tasks;
using Greet;
using Grpc.Core;
Expand Down Expand Up @@ -50,5 +53,44 @@ public async Task UnaryCall_Http1Response_ThrowError()
Assert.AreEqual(StatusCode.Internal, ex.StatusCode);
Assert.AreEqual("Bad gRPC response. Response protocol downgraded to HTTP/1.1.", ex.Status.Detail);
}

[TestCase(SocketError.HostNotFound, StatusCode.Unavailable)]
[TestCase(SocketError.ConnectionRefused, StatusCode.Unavailable)]
public async Task UnaryCall_SocketException_ThrowCorrectStatus(SocketError socketError, StatusCode statusCode)
{
// Arrange
var httpClient = ClientTestHelpers.CreateTestClient(request =>
{
return Task.FromException<HttpResponseMessage>(new HttpRequestException("Blah", new SocketException((int)socketError)));
});
var invoker = HttpClientCallInvokerFactory.Create(httpClient);

// Act
var call = invoker.AsyncUnaryCall<HelloRequest, HelloReply>(ClientTestHelpers.ServiceMethod, string.Empty, new CallOptions(deadline: invoker.Channel.Clock.UtcNow.AddSeconds(1)), new HelloRequest());

// Assert
var ex = await ExceptionAssert.ThrowsAsync<RpcException>(() => call.ResponseAsync).DefaultTimeout();

Assert.AreEqual(statusCode, ex.StatusCode);
}

[Test]
public async Task UnaryCall_IOException_ThrowCorrectStatus()
{
// Arrange
var httpClient = ClientTestHelpers.CreateTestClient(request =>
{
return Task.FromException<HttpResponseMessage>(new HttpRequestException("Blah", new IOException("")));
});
var invoker = HttpClientCallInvokerFactory.Create(httpClient);

// Act
var call = invoker.AsyncUnaryCall<HelloRequest, HelloReply>(ClientTestHelpers.ServiceMethod, string.Empty, new CallOptions(deadline: invoker.Channel.Clock.UtcNow.AddSeconds(1)), new HelloRequest());

// Assert
var ex = await ExceptionAssert.ThrowsAsync<RpcException>(() => call.ResponseAsync).DefaultTimeout();

Assert.AreEqual(StatusCode.Unavailable, ex.StatusCode);
}
}
}
66 changes: 66 additions & 0 deletions test/Grpc.Net.Client.Tests/HttpContentClientStreamReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#endregion

using System;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
Expand Down Expand Up @@ -201,6 +202,32 @@ public async Task MoveNext_MultipleCallsWithoutAwait_ThrowError()
Assert.AreEqual(ex, write.Exception);
}

[Test]
public async Task MoveNext_StreamThrowsIOException_ThrowErrorUnavailable()
{
// Arrange
var httpClient = ClientTestHelpers.CreateTestClient(request =>
{
var stream = new TestStream();
var content = new StreamContent(stream);
return Task.FromResult(ResponseUtils.CreateResponse(HttpStatusCode.OK, content));
});

var testSink = new TestSink(e => e.LogLevel >= LogLevel.Error);
var testLoggerFactory = new TestLoggerFactory(testSink, enabled: true);

var channel = CreateChannel(httpClient, loggerFactory: testLoggerFactory);
var call = CreateGrpcCall(channel);
call.StartServerStreaming(new HelloRequest());

// Act
var ex = await ExceptionAssert.ThrowsAsync<RpcException>(() => call.ClientStreamReader!.MoveNext(CancellationToken.None)).DefaultTimeout();

// Assert
Assert.AreEqual(StatusCode.Unavailable, ex.StatusCode);
Assert.AreEqual("Error reading next message. IOException: Test", ex.Status.Detail);
}

private static GrpcCall<HelloRequest, HelloReply> CreateGrpcCall(GrpcChannel channel)
{
var uri = new Uri("http://localhost");
Expand All @@ -223,5 +250,44 @@ private static GrpcChannel CreateChannel(HttpClient httpClient, ILoggerFactory?
ThrowOperationCanceledOnCancellation = throwOperationCanceledOnCancellation ?? false
});
}

private class TestStream : Stream
{
public override bool CanRead { get; }
public override bool CanSeek { get; }
public override bool CanWrite { get; }
public override long Length { get; }
public override long Position { get; set; }

public override void Flush()
{
throw new NotImplementedException();
}

public override int Read(byte[] buffer, int offset, int count)
{
throw new NotImplementedException();
}

public override long Seek(long offset, SeekOrigin origin)
{
throw new NotImplementedException();
}

public override void SetLength(long value)
{
throw new NotImplementedException();
}

public override void Write(byte[] buffer, int offset, int count)
{
throw new NotImplementedException();
}

public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default)
{
return new ValueTask<int>(Task.FromException<int>(new IOException("Test")));
}
}
}
}