From 7a8d66e9e6c608ae0dacd094821a43001f302767 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 7 Jan 2016 19:22:03 +0100 Subject: [PATCH] Improve logging of failed connection attempts also improve XMPPTCPConnection.connectUsingConfiguration(): Try further hostAddresses if getAllByName failes. Fixes SMACK-711 --- .../smack/util/dns/HostAddress.java | 41 ++++++++++-- .../smack/SmackExceptionTest.java | 2 +- .../smack/tcp/XMPPTCPConnection.java | 64 +++++++++++-------- 3 files changed, 71 insertions(+), 36 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/dns/HostAddress.java b/smack-core/src/main/java/org/jivesoftware/smack/util/dns/HostAddress.java index 57c60c07a..3e93872f8 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/dns/HostAddress.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/dns/HostAddress.java @@ -16,13 +16,20 @@ */ package org.jivesoftware.smack.util.dns; +import java.net.InetAddress; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Map.Entry; + import org.jivesoftware.smack.SmackException.ConnectionException; import org.jivesoftware.smack.util.Objects; public class HostAddress { private final String fqdn; private final int port; - private Exception exception; + private final Map exceptions = new LinkedHashMap<>(); /** * Creates a new HostAddress with the given FQDN. The port will be set to the default XMPP client port: 5222 @@ -64,8 +71,13 @@ public class HostAddress { return port; } - public void setException(Exception e) { - this.exception = e; + public void setException(Exception exception) { + setException(null, exception); + } + + public void setException(InetAddress inetAddress, Exception exception) { + Exception old = exceptions.put(inetAddress, exception); + assert(old == null); } /** @@ -75,8 +87,8 @@ public class HostAddress { * * @return the Exception causing this HostAddress to fail */ - public Exception getException() { - return this.exception; + public Map getExceptions() { + return Collections.unmodifiableMap(exceptions); } @Override @@ -109,9 +121,24 @@ public class HostAddress { } public String getErrorMessage() { - if (exception == null) { + if (exceptions.isEmpty()) { return "No error logged"; } - return "'" + toString() + "' failed because " + exception.toString(); + StringBuilder sb = new StringBuilder(); + sb.append('\'').append(toString()).append("' failed because: "); + Iterator> iterator = exceptions.entrySet().iterator(); + while (iterator.hasNext()) { + Entry entry = iterator.next(); + InetAddress inetAddress = entry.getKey(); + if (inetAddress != null) { + sb.append(entry.getKey()).append(" exception: "); + } + sb.append(entry.getValue()); + if (iterator.hasNext()) { + sb.append(", "); + } + } + + return sb.toString(); } } diff --git a/smack-core/src/test/java/org/jivesoftware/smack/SmackExceptionTest.java b/smack-core/src/test/java/org/jivesoftware/smack/SmackExceptionTest.java index 498a17cb9..8f231049e 100644 --- a/smack-core/src/test/java/org/jivesoftware/smack/SmackExceptionTest.java +++ b/smack-core/src/test/java/org/jivesoftware/smack/SmackExceptionTest.java @@ -41,7 +41,7 @@ public class SmackExceptionTest { ConnectionException connectionException = ConnectionException.from(failedAddresses); String message = connectionException.getMessage(); - assertEquals("The following addresses failed: 'foo.bar.example:1234' failed because java.lang.Exception: Failed for some reason, 'barz.example:5678' failed because java.lang.Exception: Failed for some other reason", + assertEquals("The following addresses failed: 'foo.bar.example:1234' failed because: java.lang.Exception: Failed for some reason, 'barz.example:5678' failed because: java.lang.Exception: Failed for some other reason", message); } diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 9bef30bf9..72f9cd4d0 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -119,6 +119,7 @@ import java.security.cert.CertificateException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.LinkedList; @@ -536,7 +537,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { } } - private void connectUsingConfiguration() throws IOException, ConnectionException { + private void connectUsingConfiguration() throws ConnectionException, IOException { List failedAddresses = populateHostAddresses(); SocketFactory socketFactory = config.getSocketFactory(); ProxyInfo proxyInfo = config.getProxyInfo(); @@ -545,45 +546,52 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { socketFactory = SocketFactory.getDefault(); } for (HostAddress hostAddress : hostAddresses) { + Iterator inetAddresses = null; String host = hostAddress.getFQDN(); int port = hostAddress.getPort(); socket = socketFactory.createSocket(); try { - Iterator inetAddresses = Arrays.asList(InetAddress.getAllByName(host)).iterator(); + inetAddresses = Arrays.asList(InetAddress.getAllByName(host)).iterator(); if (!inetAddresses.hasNext()) { // This should not happen LOGGER.warning("InetAddress.getAllByName() returned empty result array."); throw new UnknownHostException(host); } - innerloop: while (inetAddresses.hasNext()) { - final InetAddress inetAddress = inetAddresses.next(); - final String inetAddressAndPort = inetAddress + " at port " + port; - LOGGER.finer("Trying to establish TCP connection to " + inetAddressAndPort); - try { - if (proxyInfo == null) { - socket.connect(new InetSocketAddress(inetAddress, port), timeout); - } - else { - proxyInfo.getProxySocketConnection().connect(socket, inetAddress, port, timeout); - } - } catch (Exception e) { - if (inetAddresses.hasNext()) { - continue innerloop; - } else { - throw e; - } - } - LOGGER.finer("Established TCP connection to " + inetAddressAndPort); - // We found a host to connect to, return here - this.host = host; - this.port = port; - return; - } } - catch (Exception e) { + catch (UnknownHostException e) { hostAddress.setException(e); - failedAddresses.add(hostAddress); + // TODO: Change to emptyIterator() once Smack's minimum Android SDK level is >= 19. + List emptyInetAddresses = Collections.emptyList(); + inetAddresses = emptyInetAddresses.iterator(); + continue; } + innerloop: while (inetAddresses.hasNext()) { + final InetAddress inetAddress = inetAddresses.next(); + final String inetAddressAndPort = inetAddress + " at port " + port; + LOGGER.finer("Trying to establish TCP connection to " + inetAddressAndPort); + try { + if (proxyInfo == null) { + socket.connect(new InetSocketAddress(inetAddress, port), timeout); + } + else { + proxyInfo.getProxySocketConnection().connect(socket, inetAddress, port, timeout); + } + } catch (Exception e) { + hostAddress.setException(inetAddress, e); + if (inetAddresses.hasNext()) { + continue innerloop; + } + else { + break innerloop; + } + } + LOGGER.finer("Established TCP connection to " + inetAddressAndPort); + // We found a host to connect to, return here + this.host = host; + this.port = port; + return; + } + failedAddresses.add(hostAddress); } // There are no more host addresses to try // throw an exception and report all tried