From 2385cc449d86f62a59c7a9f848bec53ee436172f Mon Sep 17 00:00:00 2001 From: Abhishek Agrawal <81427947+AgraVator@users.noreply.github.com> Date: Tue, 8 Sep 2026 19:56:49 +0530 Subject: [PATCH] core: Gracefully handle GOAWAY frames with unrecognized error codes (#13035) Fixes #13031 --- .../grpc/internal/GoAwayDisconnectError.java | 7 +-- .../java/io/grpc/internal/GrpcUtilTest.java | 52 +++++++++++++++++++ .../io/grpc/netty/NettyClientHandlerTest.java | 51 ++++++++++++++++++ 3 files changed, 105 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/GoAwayDisconnectError.java b/core/src/main/java/io/grpc/internal/GoAwayDisconnectError.java index 20c8c709932..4a03bf001c3 100644 --- a/core/src/main/java/io/grpc/internal/GoAwayDisconnectError.java +++ b/core/src/main/java/io/grpc/internal/GoAwayDisconnectError.java @@ -31,13 +31,10 @@ public final class GoAwayDisconnectError implements DisconnectError { /** * Creates a GoAway reason. * - * @param errorCode The specific, non-null HTTP/2 error code (e.g., "NO_ERROR"). + * @param errorCode The specific HTTP/2 error code (e.g., "NO_ERROR"). */ public GoAwayDisconnectError(GrpcUtil.Http2Error errorCode) { - if (errorCode == null) { - throw new NullPointerException("Http2Error cannot be null for GOAWAY"); - } - this.errorCode = errorCode; + this.errorCode = errorCode != null ? errorCode : GrpcUtil.Http2Error.INTERNAL_ERROR; } @Override diff --git a/core/src/test/java/io/grpc/internal/GrpcUtilTest.java b/core/src/test/java/io/grpc/internal/GrpcUtilTest.java index c243790028c..dbc3051628f 100644 --- a/core/src/test/java/io/grpc/internal/GrpcUtilTest.java +++ b/core/src/test/java/io/grpc/internal/GrpcUtilTest.java @@ -391,4 +391,56 @@ public void getTransportFromPickResult_dropPickResult_failFast() { verify(listener).closed(eq(status), eq(RpcProgress.DROPPED), any(Metadata.class)); } + + @Test + public void goAwayDisconnectError_standardErrorCode() { + GoAwayDisconnectError error = new GoAwayDisconnectError(GrpcUtil.Http2Error.NO_ERROR); + assertEquals("GOAWAY NO_ERROR", error.toErrorString()); + } + + @Test + public void goAwayDisconnectError_nullErrorCodeFallsBackToInternalError() { + GoAwayDisconnectError error = new GoAwayDisconnectError(null); + assertEquals("GOAWAY INTERNAL_ERROR", error.toErrorString()); + assertEquals(new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), error); + } + + @Test + public void goAwayDisconnectError_unrecognizedErrorCodeFallsBackToInternalError() { + // Apache httpd APR_TIMEUP error code + GoAwayDisconnectError apacheError = + new GoAwayDisconnectError(GrpcUtil.Http2Error.forCode(70007L)); + assertEquals("GOAWAY INTERNAL_ERROR", apacheError.toErrorString()); + assertEquals(new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), apacheError); + + // Arbitrary unknown error code + GoAwayDisconnectError unknownError = + new GoAwayDisconnectError(GrpcUtil.Http2Error.forCode(0x12345678L)); + assertEquals("GOAWAY INTERNAL_ERROR", unknownError.toErrorString()); + assertEquals(new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), unknownError); + + // Negative code + GoAwayDisconnectError negativeError = + new GoAwayDisconnectError(GrpcUtil.Http2Error.forCode(-1L)); + assertEquals("GOAWAY INTERNAL_ERROR", negativeError.toErrorString()); + assertEquals(new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), negativeError); + } + + @Test + public void goAwayDisconnectError_equalsAndHashCode() { + GoAwayDisconnectError err1 = + new GoAwayDisconnectError(GrpcUtil.Http2Error.forCode(70007L)); + GoAwayDisconnectError err2 = new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR); + GoAwayDisconnectError err3 = new GoAwayDisconnectError(null); + GoAwayDisconnectError err4 = new GoAwayDisconnectError(GrpcUtil.Http2Error.NO_ERROR); + + assertEquals(err1, err2); + assertEquals(err2, err3); + assertEquals(err1.hashCode(), err2.hashCode()); + assertEquals(err2.hashCode(), err3.hashCode()); + + assertFalse(err1.equals(err4)); + assertFalse(err1.hashCode() == err4.hashCode()); + } } + diff --git a/netty/src/test/java/io/grpc/netty/NettyClientHandlerTest.java b/netty/src/test/java/io/grpc/netty/NettyClientHandlerTest.java index 9f6be9a2f3e..73050bc93d7 100644 --- a/netty/src/test/java/io/grpc/netty/NettyClientHandlerTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyClientHandlerTest.java @@ -64,6 +64,8 @@ import io.grpc.internal.ClientStreamListener.RpcProgress; import io.grpc.internal.ClientTransport; import io.grpc.internal.ClientTransport.PingCallback; +import io.grpc.internal.DisconnectError; +import io.grpc.internal.GoAwayDisconnectError; import io.grpc.internal.GrpcAttributes; import io.grpc.internal.GrpcUtil; import io.grpc.internal.KeepAliveManager; @@ -588,6 +590,55 @@ public void receivedGoAwayShouldFailNewStreams() throws Exception { status.getDescription()); } + @Test + public void receivedGoAway_unrecognizedErrorCode_shouldFailNewStreamsAndReportMetric() + throws Exception { + // Read a GOAWAY with an unrecognized error code (e.g. Apache httpd error code 70007). + channelRead(goAwayFrame(0, 70007, Unpooled.copiedBuffer("apache error", UTF_8))); + + ArgumentCaptor statusCaptor = ArgumentCaptor.forClass(Status.class); + ArgumentCaptor disconnectErrorCaptor = + ArgumentCaptor.forClass(DisconnectError.class); + verify(listener).transportShutdown(statusCaptor.capture(), disconnectErrorCaptor.capture()); + + assertEquals(Status.UNAVAILABLE.getCode(), statusCaptor.getValue().getCode()); + assertEquals( + "GOAWAY shut down transport. Unrecognized HTTP/2 error code: 70007, " + + "debug data: apache error", + statusCaptor.getValue().getDescription()); + + DisconnectError disconnectError = disconnectErrorCaptor.getValue(); + assertEquals( + new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), disconnectError); + + // Creating new stream must fail immediately with UNAVAILABLE + ChannelFuture future = enqueue(newCreateStreamCommand(grpcHeaders, streamTransportState)); + assertTrue(future.isDone()); + assertFalse(future.isSuccess()); + Status status = Status.fromThrowable(future.cause()); + assertEquals(Status.UNAVAILABLE.getCode(), status.getCode()); + assertEquals( + "GOAWAY shut down transport. Unrecognized HTTP/2 error code: 70007, " + + "debug data: apache error", + status.getDescription()); + } + + @Test + public void receivedGoAway_unrecognizedErrorCode_shouldCloseActiveStreams() throws Exception { + createStream(); + + // Read a GOAWAY indicating our stream (id 3) was not processed. + channelRead(goAwayFrame(0, 70007, Unpooled.copiedBuffer("apache error", UTF_8))); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Status.class); + verify(streamListener).closed(captor.capture(), eq(PROCESSED), ArgumentMatchers.notNull()); + assertEquals(Status.INTERNAL.getCode(), captor.getValue().getCode()); + assertEquals( + "Abrupt GOAWAY closed sent stream. Unrecognized HTTP/2 error code: 70007, " + + "debug data: apache error", + captor.getValue().getDescription()); + } + // This test is not as useful as it looks, because the HTTP/2 Netty code catches and doesn't // propagate exceptions during the onGoAwayReceived callback. @Test