diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java index 8b5d8e6405f..e33254eb8cf 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,18 @@ public void run() { String threadName = Thread.currentThread().getName(); Thread.currentThread().setName("OkHttpServerTransport"); try { - frameReader.readConnectionPreface(); + try { + frameReader.readConnectionPreface(); + } 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, "Connection closed 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..4b3e744a6eb 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,50 @@ public void closeDuringHttp2Preface() throws Exception { verify(transportListener, never()).transportReady(any(Attributes.class)); } + @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() { + @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(); + disconnect.run(); + + verify(transportListener, timeout(TIME_OUT_MS)).transportTerminated(); + assertThat(warnings).isEmpty(); + } finally { + logger.removeHandler(handler); + } + } + @Test public void noSettingsDuringHttp2HandshakeSettings() throws Exception { initTransport(); @@ -1575,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); }