From b293c953d9238d157279f97ef477c5ae7da8df0c Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 9 May 2016 12:44:11 -0700 Subject: [PATCH 1/5] add more tests --- .../Grpc.Core.Tests/Internal/AsyncCallTest.cs | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs index abe9d4a2e62..6f8668d143e 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs @@ -181,13 +181,14 @@ namespace Grpc.Core.Internal.Tests } [Test] - public void ClientStreaming_WriteFailure() + public void ClientStreaming_WriteCompletionFailure() { var resultTask = asyncCall.ClientStreamingCallAsync(); var requestStream = new ClientRequestStream(asyncCall); var writeTask = requestStream.WriteAsync("request1"); fakeCall.SendCompletionHandler(false); + // TODO: maybe IOException or waiting for RPCException is more appropriate here. Assert.ThrowsAsync(typeof(InvalidOperationException), async () => await writeTask); fakeCall.UnaryResponseClientHandler(true, @@ -199,7 +200,7 @@ namespace Grpc.Core.Internal.Tests } [Test] - public void ClientStreaming_WriteAfterReceivingStatusFails() + public void ClientStreaming_WriteAfterReceivingStatusThrowsRpcException() { var resultTask = asyncCall.ClientStreamingCallAsync(); var requestStream = new ClientRequestStream(asyncCall); @@ -210,7 +211,28 @@ namespace Grpc.Core.Internal.Tests new Metadata()); AssertUnaryResponseSuccess(asyncCall, fakeCall, resultTask); + var ex = Assert.Throws(() => requestStream.WriteAsync("request1")); + //TODO: add assert. + } + + [Test] + public void ClientStreaming_WriteAfterCompleteThrowsInvalidOperationException() + { + var resultTask = asyncCall.ClientStreamingCallAsync(); + var requestStream = new ClientRequestStream(asyncCall); + + requestStream.CompleteAsync(); + Assert.Throws(typeof(InvalidOperationException), () => requestStream.WriteAsync("request1")); + + fakeCall.SendCompletionHandler(true); + + fakeCall.UnaryResponseClientHandler(true, + new ClientSideStatus(Status.DefaultSuccess, new Metadata()), + CreateResponsePayload(), + new Metadata()); + + AssertUnaryResponseSuccess(asyncCall, fakeCall, resultTask); } [Test] @@ -229,7 +251,7 @@ namespace Grpc.Core.Internal.Tests } [Test] - public void ClientStreaming_WriteAfterCancellationRequestFails() + public void ClientStreaming_WriteAfterCancellationRequestThrowsOperationCancelledException() { var resultTask = asyncCall.ClientStreamingCallAsync(); var requestStream = new ClientRequestStream(asyncCall); @@ -340,7 +362,7 @@ namespace Grpc.Core.Internal.Tests } [Test] - public void DuplexStreaming_WriteAfterReceivingStatusFails() + public void DuplexStreaming_WriteAfterReceivingStatusThrowsRpcException() { asyncCall.StartDuplexStreamingCall(); var requestStream = new ClientRequestStream(asyncCall); @@ -352,7 +374,8 @@ namespace Grpc.Core.Internal.Tests AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); - Assert.ThrowsAsync(typeof(InvalidOperationException), async () => await requestStream.WriteAsync("request1")); + var ex = Assert.ThrowsAsync(async () => await requestStream.WriteAsync("request1")); + //TODO: add assert. } [Test] @@ -372,7 +395,7 @@ namespace Grpc.Core.Internal.Tests } [Test] - public void DuplexStreaming_WriteAfterCancellationRequestFails() + public void DuplexStreaming_WriteAfterCancellationRequestThrowsOperationCancelledException() { asyncCall.StartDuplexStreamingCall(); var requestStream = new ClientRequestStream(asyncCall); From 19c7bf7871aa86814a56b6eb7053b55920b8e0c6 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 9 May 2016 13:03:59 -0700 Subject: [PATCH 2/5] fixup tests --- .../Grpc.Core.Tests/Internal/AsyncCallTest.cs | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs index 6f8668d143e..777a1c8c500 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs @@ -212,7 +212,23 @@ namespace Grpc.Core.Internal.Tests AssertUnaryResponseSuccess(asyncCall, fakeCall, resultTask); var ex = Assert.Throws(() => requestStream.WriteAsync("request1")); - //TODO: add assert. + Assert.AreEqual(Status.DefaultSuccess, ex.Status); + } + + [Test] + public void ClientStreaming_WriteAfterReceivingStatusThrowsRpcException2() + { + var resultTask = asyncCall.ClientStreamingCallAsync(); + var requestStream = new ClientRequestStream(asyncCall); + + fakeCall.UnaryResponseClientHandler(true, + new ClientSideStatus(new Status(StatusCode.OutOfRange, ""), new Metadata()), + CreateResponsePayload(), + new Metadata()); + + AssertUnaryResponseError(asyncCall, fakeCall, resultTask, StatusCode.OutOfRange); + var ex = Assert.Throws(() => requestStream.WriteAsync("request1")); + Assert.AreEqual(StatusCode.OutOfRange, ex.Status.StatusCode); } [Test] @@ -375,7 +391,7 @@ namespace Grpc.Core.Internal.Tests AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); var ex = Assert.ThrowsAsync(async () => await requestStream.WriteAsync("request1")); - //TODO: add assert. + Assert.AreEqual(Status.DefaultSuccess, ex.Status); } [Test] From 6220033e7df811e7b38afb7c9bd39887dd549e23 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 9 May 2016 12:45:27 -0700 Subject: [PATCH 3/5] change typo in the comment --- src/csharp/Grpc.Core/Internal/AsyncCall.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs index f522174bd0f..da1e6592d19 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs @@ -57,7 +57,7 @@ namespace Grpc.Core.Internal // Completion of a pending unary response if not null. TaskCompletionSource unaryResponseTcs; - // Indicates that steaming call has finished. + // Indicates that response streaming call has finished. TaskCompletionSource streamingCallFinishedTcs = new TaskCompletionSource(); // Response headers set here once received. From 98f2430d2d02ceff46b42f4e7e88786e04deb2d6 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 9 May 2016 13:04:30 -0700 Subject: [PATCH 4/5] throw RpcException from writes after finishing --- src/csharp/Grpc.Core/Internal/AsyncCall.cs | 13 +++++++++++++ src/csharp/Grpc.Core/Internal/AsyncCallBase.cs | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs index da1e6592d19..55351869b5c 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs @@ -443,6 +443,19 @@ namespace Grpc.Core.Internal } } + protected override void CheckSendingAllowed(bool allowFinished) + { + base.CheckSendingAllowed(true); + + // throwing RpcException if we already received status on client + // side makes the most sense. + // Note that this throws even for StatusCode.OK. + if (!allowFinished && finishedStatus.HasValue) + { + throw new RpcException(finishedStatus.Value.Status); + } + } + /// /// Handles receive status completion for calls with streaming response. /// diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs index 42234dcac21..4de23706b28 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs @@ -213,7 +213,7 @@ namespace Grpc.Core.Internal { } - protected void CheckSendingAllowed(bool allowFinished) + protected virtual void CheckSendingAllowed(bool allowFinished) { GrpcPreconditions.CheckState(started); CheckNotCancelled(); From a46d21de37c60554dd455104dbbec37854515253 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 10 May 2016 09:57:51 -0700 Subject: [PATCH 5/5] fix TimeoutOnSleepingServer interop test --- src/csharp/Grpc.IntegrationTesting/InteropClient.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/csharp/Grpc.IntegrationTesting/InteropClient.cs b/src/csharp/Grpc.IntegrationTesting/InteropClient.cs index b3b1abf1bca..cff85086310 100644 --- a/src/csharp/Grpc.IntegrationTesting/InteropClient.cs +++ b/src/csharp/Grpc.IntegrationTesting/InteropClient.cs @@ -492,6 +492,10 @@ namespace Grpc.IntegrationTesting { // Deadline was reached before write has started. Eat the exception and continue. } + catch (RpcException) + { + // Deadline was reached before write has started. Eat the exception and continue. + } var ex = Assert.ThrowsAsync(async () => await call.ResponseStream.MoveNext()); // We can't guarantee the status code always DeadlineExceeded. See issue #2685.