Revert "Allow to detect failed query caused by an Timeout / IO error …

…and also not cache these."

This reverts commit 12a413b as it needs some more changes due some changes that were merged into 4.1 before.

resolver-dns/src/main/java/io/netty/resolver/dns/DnsCache.java

@@ -65,8 +65,6 @@ DnsCacheEntry cache(String hostname, DnsRecord[] additionals, InetAddress addres
 
     /**
      * Cache the resolution failure for a given hostname.
-     * Be aware this <strong>won't</strong> be called with timeout / cancel / transport exceptions.
-      *
      * @param hostname the hostname
      * @param additionals the additional records
      * @param cause the resolution failure

resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java

@@ -891,24 +891,6 @@ public Future<AddressedEnvelope<DnsResponse, InetSocketAddress>> query(
         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<AddressedEnvelope<DnsResponse, InetSocketAddress>> query0(
             InetSocketAddress nameServerAddr, DnsQuestion question,
             DnsRecord[] additionals,

resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java

@@ -189,11 +189,11 @@ private void internalResolve(Promise<T> promise) {
         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, null)) {
+            if (!query(hostname, recordTypes[i], nameServerAddressStream.duplicate(), promise)) {
                 return;
             }
         }
-        query(hostname, recordTypes[end], nameServerAddressStream, promise, null);
+        query(hostname, recordTypes[end], nameServerAddressStream, promise);
     }
 
     /**
@@ -283,20 +283,19 @@ public void remove() {
 
     private void query(final DnsServerAddressStream nameServerAddrStream, final int nameServerAddrStreamIndex,
                        final DnsQuestion question,
-                       final Promise<T> promise, Throwable cause) {
+                       final Promise<T> promise) {
         query(nameServerAddrStream, nameServerAddrStreamIndex, question,
-                parent.dnsQueryLifecycleObserverFactory().newDnsQueryLifecycleObserver(question), promise, cause);
+                parent.dnsQueryLifecycleObserverFactory().newDnsQueryLifecycleObserver(question), promise);
     }
 
     private void query(final DnsServerAddressStream nameServerAddrStream,
                        final int nameServerAddrStreamIndex,
                        final DnsQuestion question,
                        final DnsQueryLifecycleObserver queryLifecycleObserver,
-                       final Promise<T> promise,
-                       final Throwable cause) {
+                       final Promise<T> promise) {
         if (nameServerAddrStreamIndex >= nameServerAddrStream.size() || allowedQueries == 0 || promise.isCancelled()) {
             tryToFinishResolve(nameServerAddrStream, nameServerAddrStreamIndex, question, queryLifecycleObserver,
-                               promise, cause);
+                               promise);
             return;
         }
 
@@ -320,22 +319,21 @@ public void operationComplete(Future<AddressedEnvelope<DnsResponse, InetSocketAd
                     return;
                 }
 
-                final Throwable queryCause = future.cause();
                 try {
-                    if (queryCause == null) {
+                    if (future.isSuccess()) {
                         onResponse(nameServerAddrStream, nameServerAddrStreamIndex, question, future.getNow(),
                                    queryLifecycleObserver, promise);
                     } else {
                         // Server did not respond or I/O error occurred; try again.
-                        queryLifecycleObserver.queryFailed(queryCause);
-                        query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, promise, queryCause);
+                        queryLifecycleObserver.queryFailed(future.cause());
+                        query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, promise);
                     }
                 } 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, queryCause);
+                                       promise);
                 }
             }
         });
@@ -368,7 +366,7 @@ void onResponse(final DnsServerAddressStream nameServerAddrStream, final int nam
             // 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, null);
+                      queryLifecycleObserver.queryNoAnswer(code), promise);
             } else {
                 queryLifecycleObserver.queryFailed(NXDOMAIN_QUERY_FAILED_EXCEPTION);
             }
@@ -422,7 +420,7 @@ private boolean handleRedirect(
 
                 if (!nameServers.isEmpty()) {
                     query(parent.uncachedRedirectDnsServerStream(nameServers), 0, question,
-                          queryLifecycleObserver.queryRedirected(unmodifiableList(nameServers)), promise, null);
+                          queryLifecycleObserver.queryRedirected(unmodifiableList(nameServers)), promise);
                     return true;
                 }
             }
@@ -603,16 +601,15 @@ void tryToFinishResolve(final DnsServerAddressStream nameServerAddrStream,
                             final int nameServerAddrStreamIndex,
                             final DnsQuestion question,
                             final DnsQueryLifecycleObserver queryLifecycleObserver,
-                            final Promise<T> promise,
-                            final Throwable cause) {
+                            final Promise<T> promise) {
         // There are no queries left to try.
         if (!queriesInProgress.isEmpty()) {
             queryLifecycleObserver.queryCancelled(allowedQueries);
 
             // 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, cause);
+                finishResolve(promise);
             }
 
             // We did not get any resolved address of the preferred type, so we can't finish the resolution process.
@@ -625,34 +622,30 @@ void tryToFinishResolve(final DnsServerAddressStream nameServerAddrStream,
                 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, cause);
+                    query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, promise);
                 } else {
                     query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, queryLifecycleObserver,
-                          promise, cause);
+                          promise);
                 }
                 return;
             }
 
             queryLifecycleObserver.queryFailed(NAME_SERVERS_EXHAUSTED_EXCEPTION);
 
             // .. and we could not find any A/AAAA records.
-
-            // 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) {
+            if (!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, null);
+                query(hostname, DnsRecordType.CNAME, getNameServers(hostname), promise);
                 return;
             }
         } else {
             queryLifecycleObserver.queryCancelled(allowedQueries);
         }
 
         // We have at least one resolved address or tried CNAME as the last resort..
-        finishResolve(promise, cause);
+        finishResolve(promise);
     }
 
     private boolean gotPreferredAddress() {
@@ -671,7 +664,7 @@ private boolean gotPreferredAddress() {
         return false;
     }
 
-    private void finishResolve(Promise<T> promise, Throwable cause) {
+    private void finishResolve(Promise<T> promise) {
         if (!queriesInProgress.isEmpty()) {
             // If there are queries in progress, we should cancel it because we already finished the resolution.
             for (Iterator<Future<AddressedEnvelope<DnsResponse, InetSocketAddress>>> i = queriesInProgress.iterator();
@@ -710,15 +703,10 @@ private void finishResolve(Promise<T> promise, Throwable cause) {
                 .append(' ');
             }
         }
-        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);
+        final UnknownHostException cause = new UnknownHostException(buf.toString());
+
+        resolveCache.cache(hostname, additionals, cause, parent.ch.eventLoop());
+        promise.tryFailure(cause);
     }
 
     abstract boolean finishResolve(Class<? extends InetAddress> addressType, List<DnsCacheEntry> resolvedEntries,
@@ -759,7 +747,7 @@ private void followCname(String cname, final DnsQueryLifecycleObserver queryLife
                 queryLifecycleObserver.queryFailed(cause);
                 PlatformDependent.throwException(cause);
             }
-            query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise, null);
+            query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise);
         }
         if (parent.supportsAAAARecords()) {
             try {
@@ -770,17 +758,17 @@ private void followCname(String cname, final DnsQueryLifecycleObserver queryLife
                 queryLifecycleObserver.queryFailed(cause);
                 PlatformDependent.throwException(cause);
             }
-            query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise, null);
+            query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise);
         }
     }
 
     private boolean query(String hostname, DnsRecordType type, DnsServerAddressStream dnsServerAddressStream,
-                          Promise<T> promise, Throwable cause) {
+                          Promise<T> promise) {
         final DnsQuestion question = newQuestion(hostname, type);
         if (question == null) {
             return false;
         }
-        query(dnsServerAddressStream, 0, question, promise, cause);
+        query(dnsServerAddressStream, 0, question, promise);
         return true;
     }
 

resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverException.java

@@ -26,7 +26,7 @@
  * A {@link RuntimeException} raised when {@link DnsNameResolver} failed to perform a successful query.
  */
 @UnstableApi
-public class DnsNameResolverException extends RuntimeException {
+public final class DnsNameResolverException extends RuntimeException {
 
     private static final long serialVersionUID = -8826717909627131850L;
 

resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java

@@ -213,13 +213,12 @@ private void setFailure(String message, Throwable cause) {
            .append(" (no stack trace available)");
 
         final DnsNameResolverException e;
-        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 {
+        if (cause != null) {
             e = new DnsNameResolverException(nameServerAddr, question(), buf.toString(), cause);
+        } else {
+            e = new DnsNameResolverException(nameServerAddr, question(), buf.toString());
         }
+
         promise.tryFailure(e);
     }
 }

resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java

@@ -1322,51 +1322,4 @@ private static ResourceRecord newNsRecord(String dnsname, String domainName) {
             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<? extends DnsCacheEntry> 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<InetAddress> 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();
-    }
 }