Improve logging of failed connection attempts

also improve XMPPTCPConnection.connectUsingConfiguration(): Try further
hostAddresses if getAllByName failes.

Fixes SMACK-711
This commit is contained in:
Florian Schmaus 2016-01-07 19:22:03 +01:00
parent 658a671cbe
commit 7a8d66e9e6
3 changed files with 71 additions and 36 deletions

View File

@ -16,13 +16,20 @@
*/ */
package org.jivesoftware.smack.util.dns; 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.SmackException.ConnectionException;
import org.jivesoftware.smack.util.Objects; import org.jivesoftware.smack.util.Objects;
public class HostAddress { public class HostAddress {
private final String fqdn; private final String fqdn;
private final int port; private final int port;
private Exception exception; private final Map<InetAddress, Exception> exceptions = new LinkedHashMap<>();
/** /**
* Creates a new HostAddress with the given FQDN. The port will be set to the default XMPP client port: 5222 * 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; return port;
} }
public void setException(Exception e) { public void setException(Exception exception) {
this.exception = e; 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 * @return the Exception causing this HostAddress to fail
*/ */
public Exception getException() { public Map<InetAddress, Exception> getExceptions() {
return this.exception; return Collections.unmodifiableMap(exceptions);
} }
@Override @Override
@ -109,9 +121,24 @@ public class HostAddress {
} }
public String getErrorMessage() { public String getErrorMessage() {
if (exception == null) { if (exceptions.isEmpty()) {
return "No error logged"; return "No error logged";
} }
return "'" + toString() + "' failed because " + exception.toString(); StringBuilder sb = new StringBuilder();
sb.append('\'').append(toString()).append("' failed because: ");
Iterator<Entry<InetAddress, Exception>> iterator = exceptions.entrySet().iterator();
while (iterator.hasNext()) {
Entry<InetAddress, Exception> 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();
} }
} }

View File

@ -41,7 +41,7 @@ public class SmackExceptionTest {
ConnectionException connectionException = ConnectionException.from(failedAddresses); ConnectionException connectionException = ConnectionException.from(failedAddresses);
String message = connectionException.getMessage(); 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); message);
} }

View File

@ -119,6 +119,7 @@ import java.security.cert.CertificateException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collection; import java.util.Collection;
import java.util.Collections;
import java.util.Iterator; import java.util.Iterator;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.LinkedList; 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<HostAddress> failedAddresses = populateHostAddresses(); List<HostAddress> failedAddresses = populateHostAddresses();
SocketFactory socketFactory = config.getSocketFactory(); SocketFactory socketFactory = config.getSocketFactory();
ProxyInfo proxyInfo = config.getProxyInfo(); ProxyInfo proxyInfo = config.getProxyInfo();
@ -545,45 +546,52 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
socketFactory = SocketFactory.getDefault(); socketFactory = SocketFactory.getDefault();
} }
for (HostAddress hostAddress : hostAddresses) { for (HostAddress hostAddress : hostAddresses) {
Iterator<InetAddress> inetAddresses = null;
String host = hostAddress.getFQDN(); String host = hostAddress.getFQDN();
int port = hostAddress.getPort(); int port = hostAddress.getPort();
socket = socketFactory.createSocket(); socket = socketFactory.createSocket();
try { try {
Iterator<InetAddress> inetAddresses = Arrays.asList(InetAddress.getAllByName(host)).iterator(); inetAddresses = Arrays.asList(InetAddress.getAllByName(host)).iterator();
if (!inetAddresses.hasNext()) { if (!inetAddresses.hasNext()) {
// This should not happen // This should not happen
LOGGER.warning("InetAddress.getAllByName() returned empty result array."); LOGGER.warning("InetAddress.getAllByName() returned empty result array.");
throw new UnknownHostException(host); 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); hostAddress.setException(e);
failedAddresses.add(hostAddress); // TODO: Change to emptyIterator() once Smack's minimum Android SDK level is >= 19.
List<InetAddress> 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 // There are no more host addresses to try
// throw an exception and report all tried // throw an exception and report all tried