From 9c7c17c1e109ae4e40d5a919c915201faed1928a Mon Sep 17 00:00:00 2001 From: Reginald McDonald <40721169+reggiemcdonald@users.noreply.github.com> Date: Thu, 21 May 2020 17:15:53 -0600 Subject: [PATCH] netty: return status code unavailable when netty channel has unresolved InetSocketAddress (#7023) --- .../io/grpc/netty/NettyChannelBuilder.java | 3 +- netty/src/main/java/io/grpc/netty/Utils.java | 4 ++ .../io/grpc/netty/NettyTransportTest.java | 51 +++++++++++++++++++ .../test/java/io/grpc/netty/UtilsTest.java | 3 ++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java index 84ee4f20dbf5..02a4f4ff3b80 100644 --- a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java +++ b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java @@ -103,7 +103,8 @@ public final class NettyChannelBuilder * Creates a new builder with the given server address. This factory method is primarily intended * for using Netty Channel types other than SocketChannel. {@link #forAddress(String, int)} should * generally be preferred over this method, since that API permits delaying DNS lookups and - * noticing changes to DNS. + * noticing changes to DNS. If an unresolved InetSocketAddress is passed in, then it will remain + * unresolved. */ @CheckReturnValue public static NettyChannelBuilder forAddress(SocketAddress serverAddress) { diff --git a/netty/src/main/java/io/grpc/netty/Utils.java b/netty/src/main/java/io/grpc/netty/Utils.java index c53261856131..a3582b4004d4 100644 --- a/netty/src/main/java/io/grpc/netty/Utils.java +++ b/netty/src/main/java/io/grpc/netty/Utils.java @@ -54,6 +54,7 @@ import java.io.IOException; import java.lang.reflect.Constructor; import java.nio.channels.ClosedChannelException; +import java.nio.channels.UnresolvedAddressException; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.ThreadFactory; @@ -270,6 +271,9 @@ public static Status statusFromThrowable(Throwable t) { if (t instanceof IOException) { return Status.UNAVAILABLE.withDescription("io exception").withCause(t); } + if (t instanceof UnresolvedAddressException) { + return Status.UNAVAILABLE.withDescription("unresolved address").withCause(t); + } if (t instanceof Http2Exception) { return Status.INTERNAL.withDescription("http2 exception").withCause(t); } diff --git a/netty/src/test/java/io/grpc/netty/NettyTransportTest.java b/netty/src/test/java/io/grpc/netty/NettyTransportTest.java index 8d3d356528e9..182fd81d9d85 100644 --- a/netty/src/test/java/io/grpc/netty/NettyTransportTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyTransportTest.java @@ -16,16 +16,24 @@ package io.grpc.netty; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; + +import com.google.common.util.concurrent.SettableFuture; +import io.grpc.ChannelLogger; import io.grpc.ServerStreamTracer; +import io.grpc.Status; import io.grpc.internal.AbstractTransportTest; import io.grpc.internal.ClientTransportFactory; import io.grpc.internal.FakeClock; import io.grpc.internal.InternalServer; import io.grpc.internal.ManagedClientTransport; import java.net.InetSocketAddress; +import java.nio.channels.UnresolvedAddressException; import java.util.List; import java.util.concurrent.TimeUnit; import org.junit.After; +import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -106,4 +114,47 @@ protected ManagedClientTransport newClientTransport(InternalServer server) { public void clientChecksInboundMetadataSize_trailer() throws Exception { // Server-side is flaky due to https://github.com/netty/netty/pull/8332 } + + @Test + public void channelHasUnresolvedHostname() throws Exception { + server = null; + final SettableFuture future = SettableFuture.create(); + ChannelLogger logger = transportLogger(); + ManagedClientTransport transport = clientFactory.newClientTransport( + InetSocketAddress.createUnresolved("invalid", 1234), + new ClientTransportFactory.ClientTransportOptions() + .setChannelLogger(logger), logger); + Runnable runnable = transport.start(new ManagedClientTransport.Listener() { + @Override + public void transportShutdown(Status s) { + future.set(s); + } + + @Override + public void transportTerminated() {} + + @Override + public void transportReady() { + Throwable t = new Throwable("transport should have failed and shutdown but didnt"); + future.setException(t); + } + + @Override + public void transportInUse(boolean inUse) { + Throwable t = new Throwable("transport should have failed and shutdown but didnt"); + future.setException(t); + } + }); + if (runnable != null) { + runnable.run(); + } + try { + Status status = future.get(); + assertEquals(Status.Code.UNAVAILABLE, status.getCode()); + assertThat(status.getCause()).isInstanceOf(UnresolvedAddressException.class); + assertEquals("unresolved address", status.getDescription()); + } finally { + transport.shutdown(Status.UNAVAILABLE.withDescription("test shutdown")); + } + } } diff --git a/netty/src/test/java/io/grpc/netty/UtilsTest.java b/netty/src/test/java/io/grpc/netty/UtilsTest.java index c5d511385059..90330a5fe969 100644 --- a/netty/src/test/java/io/grpc/netty/UtilsTest.java +++ b/netty/src/test/java/io/grpc/netty/UtilsTest.java @@ -42,6 +42,7 @@ import io.netty.handler.codec.http2.Http2Exception; import io.netty.handler.codec.http2.Http2Headers; import io.netty.util.AsciiString; +import java.nio.channels.UnresolvedAddressException; import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -61,6 +62,8 @@ public void testStatusFromThrowable() { Throwable t; t = new ConnectTimeoutException("msg"); assertStatusEquals(Status.UNAVAILABLE.withCause(t), Utils.statusFromThrowable(t)); + t = new UnresolvedAddressException(); + assertStatusEquals(Status.UNAVAILABLE.withCause(t), Utils.statusFromThrowable(t)); t = new Http2Exception(Http2Error.INTERNAL_ERROR, "msg"); assertStatusEquals(Status.INTERNAL.withCause(t), Utils.statusFromThrowable(t)); t = new Exception("msg");