Skip to content

Commit 09a6e2e

Browse files
committed
Revert "netty: Preserve early server handshake failure cause in logs"
This reverts commit 50ead96. Some users are seeing errors during app development: cl/876319409 b/488103229. Seems related to some internal Google-specific stuff.
1 parent 31fdb6c commit 09a6e2e

File tree

4 files changed

+0
-76
lines changed

4 files changed

+0
-76
lines changed

netty/src/main/java/io/grpc/netty/NettyServerHandler.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,6 @@ class NettyServerHandler extends AbstractNettyHandler {
124124
private static final boolean DISABLE_CONNECTION_HEADER_CHECK = Boolean.parseBoolean(
125125
System.getProperty("io.grpc.netty.disableConnectionHeaderCheck", "false"));
126126

127-
/**
128-
* A message that simply passes through the channel without any real processing. It is useful to
129-
* check if buffers have been drained and test the health of the channel in a single operation.
130-
*/
131-
static final Object NOOP_MESSAGE = new Object();
132-
133127
private final Http2Connection.PropertyKey streamKey;
134128
private final ServerTransportListener transportListener;
135129
private final int maxMessageSize;
@@ -715,8 +709,6 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
715709
gracefulClose(ctx, (GracefulServerCloseCommand) msg, promise);
716710
} else if (msg instanceof ForcefulCloseCommand) {
717711
forcefulClose(ctx, (ForcefulCloseCommand) msg, promise);
718-
} else if (msg == NOOP_MESSAGE) {
719-
ctx.write(Unpooled.EMPTY_BUFFER, promise);
720712
} else {
721713
AssertionError e =
722714
new AssertionError("Write called for unexpected type: " + msg.getClass().getName());

netty/src/main/java/io/grpc/netty/NettyServerTransport.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -160,18 +160,6 @@ public void operationComplete(ChannelFuture future) throws Exception {
160160
channel.closeFuture().addListener(terminationNotifier);
161161

162162
channel.pipeline().addLast(bufferingHandler);
163-
164-
channel.writeAndFlush(NettyServerHandler.NOOP_MESSAGE).addListener(new ChannelFutureListener() {
165-
@Override
166-
public void operationComplete(ChannelFuture future) throws Exception {
167-
if (!future.isSuccess()) {
168-
// grpcHandler (NettyServerHandler) may not be in the pipeline yet on early negotiation
169-
// failure, so connectionError() can remain null. Notify termination here with the write
170-
// failure cause to preserve/log the original transport termination reason.
171-
notifyTerminated(future.cause());
172-
}
173-
}
174-
});
175163
}
176164

177165
@Override

netty/src/test/java/io/grpc/netty/NettyServerHandlerTest.java

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,25 +1347,6 @@ public void maxRstCountSent_exceedsLimit_fails() throws Exception {
13471347
assertFalse(channel().isOpen());
13481348
}
13491349

1350-
@Test
1351-
public void write_noopMessage_writesEmptyBuffer() throws Exception {
1352-
manualSetUp();
1353-
1354-
ChannelPromise promise = newPromise();
1355-
handler().write(ctx(), NettyServerHandler.NOOP_MESSAGE, promise);
1356-
channel().flush();
1357-
1358-
assertTrue(promise.isSuccess());
1359-
1360-
Object outbound = channel().readOutbound();
1361-
assertTrue(outbound instanceof ByteBuf);
1362-
ByteBuf buf = (ByteBuf) outbound;
1363-
assertEquals(0, buf.readableBytes());
1364-
buf.release();
1365-
1366-
assertNull(channel().readOutbound());
1367-
}
1368-
13691350
private void madeYouReset(int burstSize) throws Exception {
13701351
when(streamTracerFactory.newServerStreamTracer(anyString(), any(Metadata.class)))
13711352
.thenAnswer((args) -> new TestServerStreamTracer());

netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@
133133
import java.util.concurrent.TimeUnit;
134134
import java.util.concurrent.atomic.AtomicReference;
135135
import java.util.logging.Filter;
136-
import java.util.logging.Handler;
137136
import java.util.logging.Level;
138137
import java.util.logging.LogRecord;
139138
import java.util.logging.Logger;
@@ -473,42 +472,6 @@ public void from_tls_clientAuthRequire_noClientCert() throws Exception {
473472
}
474473
}
475474

476-
@Test
477-
public void from_plaintextClient_toTlsServer_logsTransportFailureCause() throws Exception {
478-
final AtomicReference<LogRecord> transportFailureLog = new AtomicReference<>();
479-
Handler handler = new Handler() {
480-
@Override
481-
public void publish(LogRecord record) {
482-
if ("Transport failed".equals(record.getMessage()) && record.getThrown() != null) {
483-
transportFailureLog.compareAndSet(null, record);
484-
}
485-
}
486-
487-
@Override
488-
public void flush() {}
489-
490-
@Override
491-
public void close() throws SecurityException {}
492-
};
493-
Logger logger =
494-
Logger.getLogger(String.format("%s.connections", NettyServerTransport.class.getName()));
495-
Level oldLevel = logger.getLevel();
496-
try {
497-
logger.addHandler(handler);
498-
logger.setLevel(Level.ALL);
499-
500-
ServerCredentials serverCreds = TlsServerCredentials.create(server1Cert, server1Key);
501-
ChannelCredentials channelCreds = InsecureChannelCredentials.create();
502-
Status status = expectFailedHandshake(channelCreds, serverCreds);
503-
504-
assertEquals(Status.Code.UNAVAILABLE, status.getCode());
505-
assertThat(transportFailureLog.get()).isNotNull();
506-
} finally {
507-
logger.removeHandler(handler);
508-
logger.setLevel(oldLevel);
509-
}
510-
}
511-
512475
@Test
513476
public void from_tls_clientAuthRequire_clientCert() throws Exception {
514477
ServerCredentials serverCreds = TlsServerCredentials.newBuilder()

0 commit comments

Comments
 (0)