From e515c772cd81cc4ee1b6f24c1c36afbaac1c2ebb Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 25 Mar 2015 09:36:45 -0700 Subject: [PATCH] netty: Status should be based on GOAWAY code goingAway() is called before onGoAwayRead() in Netty: https://github.com/netty/netty/blob/b7f57223c14b495afb11f1ce0fe32b3d21185a08/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java#L521 The test before checked that the stream went away, but not that the GOAWAY code influenced our Status, as UNAVAILABLE is the default internally. The UNAVAILABLE default has also been changed to include a message so that we can determine where the Status came from in case it is triggered again in the future. --- .../java/io/grpc/transport/netty/NettyClientHandler.java | 8 ++------ .../io/grpc/transport/netty/NettyClientHandlerTest.java | 7 +++++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/netty/src/main/java/io/grpc/transport/netty/NettyClientHandler.java b/netty/src/main/java/io/grpc/transport/netty/NettyClientHandler.java index f7129ebe8c..034418240e 100644 --- a/netty/src/main/java/io/grpc/transport/netty/NettyClientHandler.java +++ b/netty/src/main/java/io/grpc/transport/netty/NettyClientHandler.java @@ -112,11 +112,6 @@ class NettyClientHandler extends Http2ConnectionHandler { // Whenever a stream has been closed, try to create a pending stream to fill its place. createPendingStreams(); } - - @Override - public void goingAway() { - NettyClientHandler.this.goingAway(); - } }); } @@ -215,6 +210,7 @@ class NettyClientHandler extends Http2ConnectionHandler { status = status.augmentDescription(msg); } goAwayStatus(status); + goingAway(); } @Override @@ -404,7 +400,7 @@ class NettyClientHandler extends Http2ConnectionHandler { if (goAwayStatus != null) { return goAwayStatus; } - return Status.UNAVAILABLE; + return Status.UNAVAILABLE.withDescription("Connection going away, but for unknown reason"); } private void goAwayStatus(Status status) { diff --git a/netty/src/test/java/io/grpc/transport/netty/NettyClientHandlerTest.java b/netty/src/test/java/io/grpc/transport/netty/NettyClientHandlerTest.java index 7af262bbe5..2f228efe9f 100644 --- a/netty/src/test/java/io/grpc/transport/netty/NettyClientHandlerTest.java +++ b/netty/src/test/java/io/grpc/transport/netty/NettyClientHandlerTest.java @@ -289,12 +289,15 @@ public class NettyClientHandlerTest extends NettyHandlerTestBase { promise); // Read a GOAWAY that indicates our stream was never processed by the server. - handler.channelRead(ctx, goAwayFrame(0)); + handler.channelRead(ctx, + goAwayFrame(0, 8 /* Cancel */, Unpooled.copiedBuffer("this is a test", UTF_8))); ArgumentCaptor captor = ArgumentCaptor.forClass(Status.class); InOrder inOrder = inOrder(stream); inOrder.verify(stream, calls(1)).transportReportStatus(captor.capture(), eq(false), notNull(Metadata.Trailers.class)); - assertEquals(Status.UNAVAILABLE.getCode(), captor.getValue().getCode()); + assertEquals(Status.CANCELLED.getCode(), captor.getValue().getCode()); + assertEquals("HTTP/2 error code: CANCEL\nthis is a test", + captor.getValue().getDescription()); } @Test