Skip to content

Commit

Permalink
netty: return status code unavailable when netty channel has unresolv…
Browse files Browse the repository at this point in the history
…ed InetSocketAddress (grpc#7023)
  • Loading branch information
reggiemcdonald authored and dfawley committed Jan 15, 2021
1 parent 5ffb760 commit 9c7c17c
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 1 deletion.
3 changes: 2 additions & 1 deletion netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions netty/src/main/java/io/grpc/netty/Utils.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
51 changes: 51 additions & 0 deletions netty/src/test/java/io/grpc/netty/NettyTransportTest.java
Expand Up @@ -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;

Expand Down Expand Up @@ -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<Status> 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"));
}
}
}
3 changes: 3 additions & 0 deletions netty/src/test/java/io/grpc/netty/UtilsTest.java
Expand Up @@ -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;
Expand All @@ -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");
Expand Down

0 comments on commit 9c7c17c

Please sign in to comment.