Skip to content

Commit 4d5ccfc

Browse files
authored
core: Gracefully handle GOAWAY frames with unrecognized error codes (v1.79.x backport) (#13048)
Backport of #13035 to v1.79.x.
1 parent 2b746c0 commit 4d5ccfc

3 files changed

Lines changed: 105 additions & 5 deletions

File tree

‎core/src/main/java/io/grpc/internal/GoAwayDisconnectError.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,10 @@ public final class GoAwayDisconnectError implements DisconnectError {
3131
/**
3232
* Creates a GoAway reason.
3333
*
34-
* @param errorCode The specific, non-null HTTP/2 error code (e.g., "NO_ERROR").
34+
* @param errorCode The specific HTTP/2 error code (e.g., "NO_ERROR").
3535
*/
3636
public GoAwayDisconnectError(GrpcUtil.Http2Error errorCode) {
37-
if (errorCode == null) {
38-
throw new NullPointerException("Http2Error cannot be null for GOAWAY");
39-
}
40-
this.errorCode = errorCode;
37+
this.errorCode = errorCode != null ? errorCode : GrpcUtil.Http2Error.INTERNAL_ERROR;
4138
}
4239

4340
@Override

‎core/src/test/java/io/grpc/internal/GrpcUtilTest.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,4 +391,56 @@ public void getTransportFromPickResult_dropPickResult_failFast() {
391391

392392
verify(listener).closed(eq(status), eq(RpcProgress.DROPPED), any(Metadata.class));
393393
}
394+
395+
@Test
396+
public void goAwayDisconnectError_standardErrorCode() {
397+
GoAwayDisconnectError error = new GoAwayDisconnectError(GrpcUtil.Http2Error.NO_ERROR);
398+
assertEquals("GOAWAY NO_ERROR", error.toErrorString());
399+
}
400+
401+
@Test
402+
public void goAwayDisconnectError_nullErrorCodeFallsBackToInternalError() {
403+
GoAwayDisconnectError error = new GoAwayDisconnectError(null);
404+
assertEquals("GOAWAY INTERNAL_ERROR", error.toErrorString());
405+
assertEquals(new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), error);
406+
}
407+
408+
@Test
409+
public void goAwayDisconnectError_unrecognizedErrorCodeFallsBackToInternalError() {
410+
// Apache httpd APR_TIMEUP error code
411+
GoAwayDisconnectError apacheError =
412+
new GoAwayDisconnectError(GrpcUtil.Http2Error.forCode(70007L));
413+
assertEquals("GOAWAY INTERNAL_ERROR", apacheError.toErrorString());
414+
assertEquals(new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), apacheError);
415+
416+
// Arbitrary unknown error code
417+
GoAwayDisconnectError unknownError =
418+
new GoAwayDisconnectError(GrpcUtil.Http2Error.forCode(0x12345678L));
419+
assertEquals("GOAWAY INTERNAL_ERROR", unknownError.toErrorString());
420+
assertEquals(new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), unknownError);
421+
422+
// Negative code
423+
GoAwayDisconnectError negativeError =
424+
new GoAwayDisconnectError(GrpcUtil.Http2Error.forCode(-1L));
425+
assertEquals("GOAWAY INTERNAL_ERROR", negativeError.toErrorString());
426+
assertEquals(new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), negativeError);
427+
}
428+
429+
@Test
430+
public void goAwayDisconnectError_equalsAndHashCode() {
431+
GoAwayDisconnectError err1 =
432+
new GoAwayDisconnectError(GrpcUtil.Http2Error.forCode(70007L));
433+
GoAwayDisconnectError err2 = new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR);
434+
GoAwayDisconnectError err3 = new GoAwayDisconnectError(null);
435+
GoAwayDisconnectError err4 = new GoAwayDisconnectError(GrpcUtil.Http2Error.NO_ERROR);
436+
437+
assertEquals(err1, err2);
438+
assertEquals(err2, err3);
439+
assertEquals(err1.hashCode(), err2.hashCode());
440+
assertEquals(err2.hashCode(), err3.hashCode());
441+
442+
assertFalse(err1.equals(err4));
443+
assertFalse(err1.hashCode() == err4.hashCode());
444+
}
394445
}
446+

‎netty/src/test/java/io/grpc/netty/NettyClientHandlerTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@
6363
import io.grpc.internal.ClientStreamListener.RpcProgress;
6464
import io.grpc.internal.ClientTransport;
6565
import io.grpc.internal.ClientTransport.PingCallback;
66+
import io.grpc.internal.DisconnectError;
67+
import io.grpc.internal.GoAwayDisconnectError;
6668
import io.grpc.internal.GrpcAttributes;
6769
import io.grpc.internal.GrpcUtil;
6870
import io.grpc.internal.KeepAliveManager;
@@ -587,6 +589,55 @@ public void receivedGoAwayShouldFailNewStreams() throws Exception {
587589
status.getDescription());
588590
}
589591

592+
@Test
593+
public void receivedGoAway_unrecognizedErrorCode_shouldFailNewStreamsAndReportMetric()
594+
throws Exception {
595+
// Read a GOAWAY with an unrecognized error code (e.g. Apache httpd error code 70007).
596+
channelRead(goAwayFrame(0, 70007, Unpooled.copiedBuffer("apache error", UTF_8)));
597+
598+
ArgumentCaptor<Status> statusCaptor = ArgumentCaptor.forClass(Status.class);
599+
ArgumentCaptor<DisconnectError> disconnectErrorCaptor =
600+
ArgumentCaptor.forClass(DisconnectError.class);
601+
verify(listener).transportShutdown(statusCaptor.capture(), disconnectErrorCaptor.capture());
602+
603+
assertEquals(Status.UNAVAILABLE.getCode(), statusCaptor.getValue().getCode());
604+
assertEquals(
605+
"GOAWAY shut down transport. Unrecognized HTTP/2 error code: 70007, "
606+
+ "debug data: apache error",
607+
statusCaptor.getValue().getDescription());
608+
609+
DisconnectError disconnectError = disconnectErrorCaptor.getValue();
610+
assertEquals(
611+
new GoAwayDisconnectError(GrpcUtil.Http2Error.INTERNAL_ERROR), disconnectError);
612+
613+
// Creating new stream must fail immediately with UNAVAILABLE
614+
ChannelFuture future = enqueue(newCreateStreamCommand(grpcHeaders, streamTransportState));
615+
assertTrue(future.isDone());
616+
assertFalse(future.isSuccess());
617+
Status status = Status.fromThrowable(future.cause());
618+
assertEquals(Status.UNAVAILABLE.getCode(), status.getCode());
619+
assertEquals(
620+
"GOAWAY shut down transport. Unrecognized HTTP/2 error code: 70007, "
621+
+ "debug data: apache error",
622+
status.getDescription());
623+
}
624+
625+
@Test
626+
public void receivedGoAway_unrecognizedErrorCode_shouldCloseActiveStreams() throws Exception {
627+
createStream();
628+
629+
// Read a GOAWAY indicating our stream (id 3) was not processed.
630+
channelRead(goAwayFrame(0, 70007, Unpooled.copiedBuffer("apache error", UTF_8)));
631+
632+
ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
633+
verify(streamListener).closed(captor.capture(), eq(PROCESSED), ArgumentMatchers.notNull());
634+
assertEquals(Status.INTERNAL.getCode(), captor.getValue().getCode());
635+
assertEquals(
636+
"Abrupt GOAWAY closed sent stream. Unrecognized HTTP/2 error code: 70007, "
637+
+ "debug data: apache error",
638+
captor.getValue().getDescription());
639+
}
640+
590641
// This test is not as useful as it looks, because the HTTP/2 Netty code catches and doesn't
591642
// propagate exceptions during the onGoAwayReceived callback.
592643
@Test

0 commit comments

Comments
 (0)