From 28b13489cc84f32c025d4c9a62e410dd5159fffc Mon Sep 17 00:00:00 2001 From: hidingmyname Date: Mon, 2 Nov 2026 21:11:27 +0800 Subject: [PATCH 1/2] okhttp: Don't log a WARNING for EOF before the HTTP/2 preface L4 load balancers and TCP health probes open a connection and close it without sending any bytes. OkHttpServerTransport then hit an EOFException in readConnectionPreface(), which went to the generic catch (Throwable) and was logged at WARNING with a stack trace for every probe. Treat a clean EOF while reading the preface as a normal close: log it at FINE and shut down the transport as before. Fixes #13152 --- .../io/grpc/okhttp/OkHttpServerTransport.java | 13 ++++++- .../okhttp/OkHttpServerTransportTest.java | 34 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java index 8b5d8e6405f..c98ca1289fa 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java @@ -50,6 +50,7 @@ import io.grpc.okhttp.internal.framed.Http2; import io.grpc.okhttp.internal.framed.Settings; import io.grpc.okhttp.internal.framed.Variant; +import java.io.EOFException; import java.io.IOException; import java.net.Socket; import java.net.SocketException; @@ -583,7 +584,17 @@ public void run() { String threadName = Thread.currentThread().getName(); Thread.currentThread().setName("OkHttpServerTransport"); try { - frameReader.readConnectionPreface(); + try { + frameReader.readConnectionPreface(); + } catch (EOFException e) { + // The client closed the connection before sending the preface, e.g. a TCP health check + // from a load balancer. This is expected, so don't log it as a warning. + log.log(Level.FINE, "Connection closed before receiving the connection preface", e); + abruptShutdown(ErrorCode.INTERNAL_ERROR, "EOF while reading connection preface", + Status.UNAVAILABLE.withDescription("Connection closed before connection preface"), + false); + return; + } if (!frameReader.nextFrame(this)) { connectionError(ErrorCode.INTERNAL_ERROR, "Failed to read initial SETTINGS"); return; diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java index ae7c53c7151..1bb61a9bb57 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java @@ -72,6 +72,10 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; import okio.Buffer; import okio.BufferedSink; import okio.BufferedSource; @@ -314,6 +318,36 @@ public void closeDuringHttp2Preface() throws Exception { verify(transportListener, never()).transportReady(any(Attributes.class)); } + @Test + public void closeDuringHttp2Preface_doesNotLogWarning() throws Exception { + Logger logger = Logger.getLogger(OkHttpServerTransport.class.getName()); + List warnings = Collections.synchronizedList(new ArrayList<>()); + Handler handler = new Handler() { + @Override + public void publish(LogRecord record) { + if (record.getLevel().intValue() >= Level.WARNING.intValue()) { + warnings.add(record.getMessage() + ": " + record.getThrown()); + } + } + + @Override + public void flush() {} + + @Override + public void close() {} + }; + logger.addHandler(handler); + try { + initTransport(); + socketPair.client.close(); + + verify(transportListener, timeout(TIME_OUT_MS)).transportTerminated(); + assertThat(warnings).isEmpty(); + } finally { + logger.removeHandler(handler); + } + } + @Test public void noSettingsDuringHttp2HandshakeSettings() throws Exception { initTransport(); From 772842193222a22a495ef59e5331243e1fc9e3e3 Mon Sep 17 00:00:00 2001 From: hidingmyname Date: Tue, 3 Nov 2026 01:22:41 +0800 Subject: [PATCH 2/2] okhttp: Also treat a connection reset before the preface as a normal close A client that closes without reading the server's SETTINGS can reset the connection, so readConnectionPreface() may throw SocketException (Connection reset) instead of EOFException. CI hit this on JDK 8 and 11. Handle both the same way, and make the existing test close cleanly with shutdownOutput() plus add a test that forces a reset with SO_LINGER 0. --- .../io/grpc/okhttp/OkHttpServerTransport.java | 9 +++++---- .../okhttp/OkHttpServerTransportTest.java | 20 ++++++++++++++++++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java index c98ca1289fa..e33254eb8cf 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java @@ -586,11 +586,12 @@ public void run() { try { try { frameReader.readConnectionPreface(); - } catch (EOFException e) { - // The client closed the connection before sending the preface, e.g. a TCP health check - // from a load balancer. This is expected, so don't log it as a warning. + } catch (EOFException | SocketException e) { + // The client went away before sending the preface, e.g. a TCP health check from a load + // balancer. Closing without reading the server's SETTINGS resets the connection, so this + // can be a SocketException as well as an EOF. This is expected, so don't log a warning. log.log(Level.FINE, "Connection closed before receiving the connection preface", e); - abruptShutdown(ErrorCode.INTERNAL_ERROR, "EOF while reading connection preface", + abruptShutdown(ErrorCode.INTERNAL_ERROR, "Connection closed reading connection preface", Status.UNAVAILABLE.withDescription("Connection closed before connection preface"), false); return; diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java index 1bb61a9bb57..4b3e744a6eb 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java @@ -320,6 +320,20 @@ public void closeDuringHttp2Preface() throws Exception { @Test public void closeDuringHttp2Preface_doesNotLogWarning() throws Exception { + assertNoWarningLoggedAfter(() -> socketPair.client.shutdownOutput()); + } + + @Test + public void resetDuringHttp2Preface_doesNotLogWarning() throws Exception { + // A client that closes without reading the server's SETTINGS resets the connection instead of + // closing it cleanly. A linger of 0 makes the reset deterministic. + assertNoWarningLoggedAfter(() -> { + socketPair.client.setSoLinger(true, 0); + socketPair.client.close(); + }); + } + + private void assertNoWarningLoggedAfter(ClientDisconnect disconnect) throws Exception { Logger logger = Logger.getLogger(OkHttpServerTransport.class.getName()); List warnings = Collections.synchronizedList(new ArrayList<>()); Handler handler = new Handler() { @@ -339,7 +353,7 @@ public void close() {} logger.addHandler(handler); try { initTransport(); - socketPair.client.close(); + disconnect.run(); verify(transportListener, timeout(TIME_OUT_MS)).transportTerminated(); assertThat(warnings).isEmpty(); @@ -1609,6 +1623,10 @@ public static SocketPair create(ExecutorService threadPool) { } } + private interface ClientDisconnect { + void run() throws IOException; + } + private interface DataFrameHandler { void data(boolean inFinished, int streamId, Buffer payload); }