From 3ead41141e2eb65b1eba2896530aab00c5ef6b9c Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 24 Mar 2016 10:41:04 -0700 Subject: [PATCH] Use stream.cancel() instead of cancel() in ClientCallImpl Using cancel() sets cancelCalled=true which causes methods like sendMessage() to throw IllegalStateException. This results in spurious exceptions that are impossible to avoid. The API was designed to only throw IllegalStateException when the caller called methods in the wrong order, which is not the case here. Fixes #1531 --- .../src/main/java/io/grpc/internal/ClientCallImpl.java | 8 ++++---- .../test/java/io/grpc/internal/ClientCallImplTest.java | 10 ++-------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/ClientCallImpl.java b/core/src/main/java/io/grpc/internal/ClientCallImpl.java index 26f664fbe8..eab9631718 100644 --- a/core/src/main/java/io/grpc/internal/ClientCallImpl.java +++ b/core/src/main/java/io/grpc/internal/ClientCallImpl.java @@ -108,7 +108,7 @@ final class ClientCallImpl extends ClientCall @Override public void cancelled(Context context) { - cancel(); + stream.cancel(Status.CANCELLED.withCause(context.cancellationCause())); } /** @@ -316,7 +316,7 @@ final class ClientCallImpl extends ClientCall InputStream messageIs = method.streamRequest(message); stream.writeMessage(messageIs); } catch (Throwable e) { - cancel(Status.CANCELLED.withCause(e).withDescription("Failed to stream message")); + stream.cancel(Status.CANCELLED.withCause(e).withDescription("Failed to stream message")); return; } // For unary requests, we don't flush since we know that halfClose should be coming soon. This @@ -379,7 +379,7 @@ final class ClientCallImpl extends ClientCall observer.onHeaders(headers); } catch (Throwable t) { - cancel(Status.CANCELLED.withCause(t).withDescription("Failed to read headers")); + stream.cancel(Status.CANCELLED.withCause(t).withDescription("Failed to read headers")); return; } } @@ -402,7 +402,7 @@ final class ClientCallImpl extends ClientCall message.close(); } } catch (Throwable t) { - cancel(Status.CANCELLED.withCause(t).withDescription("Failed to read message.")); + stream.cancel(Status.CANCELLED.withCause(t).withDescription("Failed to read message.")); return; } } diff --git a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java index da059b9692..8c82be2f98 100644 --- a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java +++ b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java @@ -364,14 +364,8 @@ public class ClientCallImplTest { cancellableContext.cancel(new Throwable()); - verify(stream, times(1)).cancel(Status.CANCELLED); - - try { - call.sendMessage(null); - fail("Call has been cancelled"); - } catch (IllegalStateException ise) { - // expected - } + verify(stream, times(1)).cancel(statusCaptor.capture()); + assertEquals(Status.Code.CANCELLED, statusCaptor.getValue().getCode()); } @Test