diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsCache.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsCache.java index 8564f525ed..28e8989e6e 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsCache.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsCache.java @@ -65,6 +65,8 @@ public interface DnsCache { /** * Cache the resolution failure for a given hostname. + * Be aware this won't be called with timeout / cancel / transport exceptions. + * * @param hostname the hostname * @param additionals the additional records * @param cause the resolution failure diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java index da76a8897a..8510488c68 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java @@ -891,6 +891,24 @@ public class DnsNameResolver extends InetNameResolver { return query0(nameServerAddr, question, toArray(additionals, false), promise); } + /** + * Returns {@code true} if the {@link Throwable} was caused by an timeout or transport error. + * These methods can be used on the {@link Future#cause()} that is returned by the various methods exposed by this + * {@link DnsNameResolver}. + */ + public static boolean isTransportOrTimeoutError(Throwable cause) { + return cause != null && cause.getCause() instanceof DnsNameResolverException; + } + + /** + * Returns {@code true} if the {@link Throwable} was caused by an timeout. + * These methods can be used on the {@link Future#cause()} that is returned by the various methods exposed by this + * {@link DnsNameResolver}. + */ + public static boolean isTimeoutError(Throwable cause) { + return cause != null && cause.getCause() instanceof DnsNameResolverTimeoutException; + } + final Future> query0( InetSocketAddress nameServerAddr, DnsQuestion question, DnsRecord[] additionals, diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java index 389693bc40..df3b38a93e 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java @@ -137,14 +137,19 @@ abstract class DnsNameResolverContext { private int searchDomainIdx = initialSearchDomainIdx; @Override public void operationComplete(Future future) throws Exception { - if (future.isSuccess()) { + Throwable cause = future.cause(); + if (cause == null) { promise.trySuccess(future.getNow()); - } else if (searchDomainIdx < searchDomains.length) { - doSearchDomainQuery(hostname + '.' + searchDomains[searchDomainIdx++], this); - } else if (!startWithoutSearchDomain) { - internalResolve(promise); } else { - promise.tryFailure(new SearchDomainUnknownHostException(future.cause(), hostname)); + if (DnsNameResolver.isTransportOrTimeoutError(cause)) { + promise.tryFailure(new SearchDomainUnknownHostException(cause, hostname)); + } else if (searchDomainIdx < searchDomains.length) { + doSearchDomainQuery(hostname + '.' + searchDomains[searchDomainIdx++], this); + } else if (!startWithoutSearchDomain) { + internalResolve(promise); + } else { + promise.tryFailure(new SearchDomainUnknownHostException(cause, hostname)); + } } } }); @@ -166,6 +171,9 @@ abstract class DnsNameResolverContext { SearchDomainUnknownHostException(Throwable cause, String originalHostname) { super("Search domain query failed. Original hostname: '" + originalHostname + "' " + cause.getMessage()); setStackTrace(cause.getStackTrace()); + + // Preserve the cause + initCause(cause.getCause()); } @Override @@ -189,11 +197,11 @@ abstract class DnsNameResolverContext { assert recordTypes.length > 0; final int end = recordTypes.length - 1; for (int i = 0; i < end; ++i) { - if (!query(hostname, recordTypes[i], nameServerAddressStream.duplicate(), promise)) { + if (!query(hostname, recordTypes[i], nameServerAddressStream.duplicate(), promise, null)) { return; } } - query(hostname, recordTypes[end], nameServerAddressStream, promise); + query(hostname, recordTypes[end], nameServerAddressStream, promise, null); } /** @@ -283,19 +291,20 @@ abstract class DnsNameResolverContext { private void query(final DnsServerAddressStream nameServerAddrStream, final int nameServerAddrStreamIndex, final DnsQuestion question, - final Promise promise) { + final Promise promise, Throwable cause) { query(nameServerAddrStream, nameServerAddrStreamIndex, question, - parent.dnsQueryLifecycleObserverFactory().newDnsQueryLifecycleObserver(question), promise); + parent.dnsQueryLifecycleObserverFactory().newDnsQueryLifecycleObserver(question), promise, cause); } private void query(final DnsServerAddressStream nameServerAddrStream, final int nameServerAddrStreamIndex, final DnsQuestion question, final DnsQueryLifecycleObserver queryLifecycleObserver, - final Promise promise) { + final Promise promise, + final Throwable cause) { if (nameServerAddrStreamIndex >= nameServerAddrStream.size() || allowedQueries == 0 || promise.isCancelled()) { tryToFinishResolve(nameServerAddrStream, nameServerAddrStreamIndex, question, queryLifecycleObserver, - promise); + promise, cause); return; } @@ -319,21 +328,22 @@ abstract class DnsNameResolverContext { return; } + final Throwable queryCause = future.cause(); try { - if (future.isSuccess()) { + if (queryCause == null) { onResponse(nameServerAddrStream, nameServerAddrStreamIndex, question, future.getNow(), queryLifecycleObserver, promise); } else { // Server did not respond or I/O error occurred; try again. - queryLifecycleObserver.queryFailed(future.cause()); - query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, promise); + queryLifecycleObserver.queryFailed(queryCause); + query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, promise, queryCause); } } finally { tryToFinishResolve(nameServerAddrStream, nameServerAddrStreamIndex, question, // queryLifecycleObserver has already been terminated at this point so we must // not allow it to be terminated again by tryToFinishResolve. NoopDnsQueryLifecycleObserver.INSTANCE, - promise); + promise, queryCause); } } }); @@ -366,7 +376,7 @@ abstract class DnsNameResolverContext { // Retry with the next server if the server did not tell us that the domain does not exist. if (code != DnsResponseCode.NXDOMAIN) { query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, - queryLifecycleObserver.queryNoAnswer(code), promise); + queryLifecycleObserver.queryNoAnswer(code), promise, null); } else { queryLifecycleObserver.queryFailed(NXDOMAIN_QUERY_FAILED_EXCEPTION); } @@ -420,7 +430,7 @@ abstract class DnsNameResolverContext { if (!nameServers.isEmpty()) { query(parent.uncachedRedirectDnsServerStream(nameServers), 0, question, - queryLifecycleObserver.queryRedirected(unmodifiableList(nameServers)), promise); + queryLifecycleObserver.queryRedirected(unmodifiableList(nameServers)), promise, null); return true; } } @@ -601,7 +611,8 @@ abstract class DnsNameResolverContext { final int nameServerAddrStreamIndex, final DnsQuestion question, final DnsQueryLifecycleObserver queryLifecycleObserver, - final Promise promise) { + final Promise promise, + final Throwable cause) { // There are no queries left to try. if (!queriesInProgress.isEmpty()) { queryLifecycleObserver.queryCancelled(allowedQueries); @@ -609,7 +620,7 @@ abstract class DnsNameResolverContext { // There are still some queries we did not receive responses for. if (gotPreferredAddress()) { // But it's OK to finish the resolution process if we got a resolved address of the preferred type. - finishResolve(promise); + finishResolve(promise, cause); } // We did not get any resolved address of the preferred type, so we can't finish the resolution process. @@ -622,10 +633,10 @@ abstract class DnsNameResolverContext { if (queryLifecycleObserver == NoopDnsQueryLifecycleObserver.INSTANCE) { // If the queryLifecycleObserver has already been terminated we should create a new one for this // fresh query. - query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, promise); + query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, promise, cause); } else { query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, queryLifecycleObserver, - promise); + promise, cause); } return; } @@ -633,11 +644,15 @@ abstract class DnsNameResolverContext { queryLifecycleObserver.queryFailed(NAME_SERVERS_EXHAUSTED_EXCEPTION); // .. and we could not find any A/AAAA records. - if (!triedCNAME) { + + // If cause != null we know this was caused by a timeout / cancel / transport exception. In this case we + // won't try to resolve the CNAME as we only should do this if we could not get the A/AAAA records because + // these not exists and the DNS server did probably signal it. + if (cause == null && !triedCNAME) { // As the last resort, try to query CNAME, just in case the name server has it. triedCNAME = true; - query(hostname, DnsRecordType.CNAME, getNameServers(hostname), promise); + query(hostname, DnsRecordType.CNAME, getNameServers(hostname), promise, null); return; } } else { @@ -645,7 +660,7 @@ abstract class DnsNameResolverContext { } // We have at least one resolved address or tried CNAME as the last resort.. - finishResolve(promise); + finishResolve(promise, cause); } private boolean gotPreferredAddress() { @@ -664,7 +679,7 @@ abstract class DnsNameResolverContext { return false; } - private void finishResolve(Promise promise) { + private void finishResolve(Promise promise, Throwable cause) { if (!queriesInProgress.isEmpty()) { // If there are queries in progress, we should cancel it because we already finished the resolution. for (Iterator>> i = queriesInProgress.iterator(); @@ -703,10 +718,15 @@ abstract class DnsNameResolverContext { .append(' '); } } - final UnknownHostException cause = new UnknownHostException(buf.toString()); - - resolveCache.cache(hostname, additionals, cause, parent.ch.eventLoop()); - promise.tryFailure(cause); + final UnknownHostException unknownHostException = new UnknownHostException(buf.toString()); + if (cause == null) { + // Only cache if the failure was not because of an IO error / timeout that was caused by the query + // itself. + resolveCache.cache(hostname, additionals, unknownHostException, parent.ch.eventLoop()); + } else { + unknownHostException.initCause(cause); + } + promise.tryFailure(unknownHostException); } abstract boolean finishResolve(Class addressType, List resolvedEntries, @@ -747,7 +767,7 @@ abstract class DnsNameResolverContext { queryLifecycleObserver.queryFailed(cause); PlatformDependent.throwException(cause); } - query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise); + query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise, null); } if (parent.supportsAAAARecords()) { try { @@ -758,17 +778,17 @@ abstract class DnsNameResolverContext { queryLifecycleObserver.queryFailed(cause); PlatformDependent.throwException(cause); } - query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise); + query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise, null); } } private boolean query(String hostname, DnsRecordType type, DnsServerAddressStream dnsServerAddressStream, - Promise promise) { + Promise promise, Throwable cause) { final DnsQuestion question = newQuestion(hostname, type); if (question == null) { return false; } - query(dnsServerAddressStream, 0, question, promise); + query(dnsServerAddressStream, 0, question, promise, cause); return true; } diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverException.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverException.java index 07eb32e63c..77e347449b 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverException.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverException.java @@ -26,7 +26,7 @@ import java.net.InetSocketAddress; * A {@link RuntimeException} raised when {@link DnsNameResolver} failed to perform a successful query. */ @UnstableApi -public final class DnsNameResolverException extends RuntimeException { +public class DnsNameResolverException extends RuntimeException { private static final long serialVersionUID = -8826717909627131850L; diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverTimeoutException.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverTimeoutException.java new file mode 100644 index 0000000000..e1ad13de4d --- /dev/null +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverTimeoutException.java @@ -0,0 +1,35 @@ +/* + * Copyright 2017 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.resolver.dns; + +import io.netty.handler.codec.dns.DnsQuestion; +import io.netty.util.internal.UnstableApi; + +import java.net.InetSocketAddress; + +/** + * A {@link DnsNameResolverException} raised when {@link DnsNameResolver} failed to perform a successful query because + * of an timeout. In this case you may want to retry the operation. + */ +@UnstableApi +public final class DnsNameResolverTimeoutException extends DnsNameResolverException { + private static final long serialVersionUID = -8826717969627131854L; + + public DnsNameResolverTimeoutException( + InetSocketAddress remoteAddress, DnsQuestion question, String message) { + super(remoteAddress, question, message); + } +} diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java index 4f8707701f..12196f184c 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java @@ -213,12 +213,13 @@ final class DnsQueryContext { .append(" (no stack trace available)"); final DnsNameResolverException e; - if (cause != null) { - e = new DnsNameResolverException(nameServerAddr, question(), buf.toString(), cause); + if (cause == null) { + // This was caused by an timeout so use DnsNameResolverTimeoutException to allow the user to + // handle it special (like retry the query). + e = new DnsNameResolverTimeoutException(nameServerAddr, question(), buf.toString()); } else { - e = new DnsNameResolverException(nameServerAddr, question(), buf.toString()); + e = new DnsNameResolverException(nameServerAddr, question(), buf.toString(), cause); } - promise.tryFailure(e); } } diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java index 2e99308970..707a061d56 100644 --- a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java +++ b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java @@ -1322,4 +1322,51 @@ public class DnsNameResolverTest { return rm.getEntry(); } } + + @Test(timeout = 3000) + public void testTimeoutNotCached() { + DnsCache cache = new DnsCache() { + @Override + public void clear() { + // NOOP + } + + @Override + public boolean clear(String hostname) { + return false; + } + + @Override + public List get(String hostname, DnsRecord[] additionals) { + return Collections.emptyList(); + } + + @Override + public DnsCacheEntry cache(String hostname, DnsRecord[] additionals, InetAddress address, + long originalTtl, EventLoop loop) { + fail("Should not be cached"); + return null; + } + + @Override + public DnsCacheEntry cache(String hostname, DnsRecord[] additionals, Throwable cause, EventLoop loop) { + fail("Should not be cached"); + return null; + } + }; + DnsNameResolverBuilder builder = newResolver(); + builder.queryTimeoutMillis(100) + .authoritativeDnsServerCache(cache) + .resolveCache(cache) + .nameServerProvider(new SingletonDnsServerAddressStreamProvider( + new InetSocketAddress(NetUtil.LOCALHOST, 12345))); + DnsNameResolver resolver = builder.build(); + Future result = resolver.resolve("doesnotexist.netty.io").awaitUninterruptibly(); + Throwable cause = result.cause(); + assertTrue(cause instanceof UnknownHostException); + assertTrue(cause.getCause() instanceof DnsNameResolverTimeoutException); + assertTrue(DnsNameResolver.isTimeoutError(cause)); + assertTrue(DnsNameResolver.isTransportOrTimeoutError(cause)); + resolver.close(); + } }