From 9a00e09c0a23140beb7fd015548fe518c85b12b3 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 2 Jun 2015 17:21:33 +0200 Subject: [PATCH] Improve NoResponseException messages --- .../smack/AbstractXMPPConnection.java | 4 +-- .../smack/SASLAuthentication.java | 2 +- .../jivesoftware/smack/SmackException.java | 27 +++++++++++++------ .../smack/SynchronizationPoint.java | 7 +++-- .../smackx/filetransfer/StreamNegotiator.java | 2 +- .../smack/tcp/XMPPTCPConnection.java | 16 ++++++----- 6 files changed, 37 insertions(+), 21 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java index 93c78d74a..835db34fd 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -194,13 +194,13 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { * parsed. */ protected final SynchronizationPoint lastFeaturesReceived = new SynchronizationPoint( - AbstractXMPPConnection.this); + AbstractXMPPConnection.this, "last stream features received from server"); /** * Set to success if the sasl feature has been received. */ protected final SynchronizationPoint saslFeatureReceived = new SynchronizationPoint( - AbstractXMPPConnection.this); + AbstractXMPPConnection.this, "SASL mechanisms stream feature from server"); /** * The SASLAuthentication manager that is responsible for authenticating with the server. diff --git a/smack-core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java b/smack-core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java index b15ce272b..24e203528 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java @@ -207,7 +207,7 @@ public final class SASLAuthentication { } if (!authenticationSuccessful) { - throw NoResponseException.newWith(connection); + throw NoResponseException.newWith(connection, "successful SASL authentication"); } } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/SmackException.java b/smack-core/src/main/java/org/jivesoftware/smack/SmackException.java index c2fbca05e..be7412d41 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/SmackException.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/SmackException.java @@ -1,6 +1,6 @@ /** * - * Copyright 2014 Florian Schmaus + * Copyright 2014-2015 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -68,6 +68,10 @@ public class SmackException extends Exception { private final StanzaFilter filter; + private NoResponseException(String message) { + this(message, null); + } + private NoResponseException(String message, StanzaFilter filter) { super(message); this.filter = filter; @@ -82,8 +86,10 @@ public class SmackException extends Exception { return filter; } - public static NoResponseException newWith(XMPPConnection connection) { - return newWith(connection, (StanzaFilter) null); + public static NoResponseException newWith(XMPPConnection connection, String waitingFor) { + final StringBuilder sb = getWaitingFor(connection); + sb.append(" While waiting for ").append(waitingFor); + return new NoResponseException(sb.toString()); } public static NoResponseException newWith(XMPPConnection connection, @@ -92,11 +98,8 @@ public class SmackException extends Exception { } public static NoResponseException newWith(XMPPConnection connection, StanzaFilter filter) { - final long replyTimeout = connection.getPacketReplyTimeout(); - final StringBuilder sb = new StringBuilder(256); - sb.append("No response received within reply timeout. Timeout was " - + replyTimeout + "ms (~" - + replyTimeout / 1000 + "s). Used filter: "); + final StringBuilder sb = getWaitingFor(connection); + sb.append(" Waited for response using: "); if (filter != null) { sb.append(filter.toString()); } @@ -107,6 +110,14 @@ public class SmackException extends Exception { return new NoResponseException(sb.toString(), filter); } + private static StringBuilder getWaitingFor(XMPPConnection connection) { + final long replyTimeout = connection.getPacketReplyTimeout(); + final StringBuilder sb = new StringBuilder(256); + sb.append("No response received within reply timeout. Timeout was " + + replyTimeout + "ms (~" + + replyTimeout / 1000 + "s)."); + return sb; + } } public static class NotLoggedInException extends SmackException { diff --git a/smack-core/src/main/java/org/jivesoftware/smack/SynchronizationPoint.java b/smack-core/src/main/java/org/jivesoftware/smack/SynchronizationPoint.java index e7deef172..4c754cd92 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/SynchronizationPoint.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/SynchronizationPoint.java @@ -31,6 +31,7 @@ public class SynchronizationPoint { private final AbstractXMPPConnection connection; private final Lock connectionLock; private final Condition condition; + private final String waitFor; // Note that there is no need to make 'state' and 'failureException' volatile. Since 'lock' and 'unlock' have the // same memory synchronization effects as synchronization block enter and leave. @@ -41,11 +42,13 @@ public class SynchronizationPoint { * Construct a new synchronization point for the given connection. * * @param connection the connection of this synchronization point. + * @param waitFor a description of the event this synchronization point handles. */ - public SynchronizationPoint(AbstractXMPPConnection connection) { + public SynchronizationPoint(AbstractXMPPConnection connection, String waitFor) { this.connection = connection; this.connectionLock = connection.getConnectionLock(); this.condition = connection.getConnectionLock().newCondition(); + this.waitFor = waitFor; init(); } @@ -253,7 +256,7 @@ public class SynchronizationPoint { case Initial: case NoResponse: case RequestSent: - throw NoResponseException.newWith(connection); + throw NoResponseException.newWith(connection, waitFor); case Success: return true; case Failure: diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/StreamNegotiator.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/StreamNegotiator.java index 6883afe8f..dcc2c0ed6 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/StreamNegotiator.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/StreamNegotiator.java @@ -114,7 +114,7 @@ public abstract class StreamNegotiator { } if (streamMethodInitiation == null) { - throw NoResponseException.newWith(connection); + throw NoResponseException.newWith(connection, "stream initiation"); } XMPPErrorException.ifHasErrorThenThrow(streamMethodInitiation); return streamMethodInitiation; 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 393be87dd..f2dca387e 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 @@ -174,25 +174,27 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { */ protected PacketReader packetReader; - private final SynchronizationPoint initalOpenStreamSend = new SynchronizationPoint(this); + private final SynchronizationPoint initalOpenStreamSend = new SynchronizationPoint<>( + this, "initial open stream element send to server"); /** * */ private final SynchronizationPoint maybeCompressFeaturesReceived = new SynchronizationPoint( - this); + this, "stream compression feature"); /** * */ private final SynchronizationPoint compressSyncPoint = new SynchronizationPoint( - this); + this, "stream compression"); /** * A synchronization point which is successful if this connection has received the closing * stream element from the remote end-point, i.e. the server. */ - private final SynchronizationPoint closingStreamReceived = new SynchronizationPoint(this); + private final SynchronizationPoint closingStreamReceived = new SynchronizationPoint<>( + this, "stream closing element received"); /** * The default bundle and defer callback, used for new connections. @@ -221,10 +223,10 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { private String smSessionId; private final SynchronizationPoint smResumedSyncPoint = new SynchronizationPoint( - this); + this, "stream resumed element"); private final SynchronizationPoint smEnabledSyncPoint = new SynchronizationPoint( - this); + this, "stream enabled element"); /** * The client's preferred maximum resumption time in seconds. @@ -1169,7 +1171,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { * Needs to be protected for unit testing purposes. */ protected SynchronizationPoint shutdownDone = new SynchronizationPoint( - XMPPTCPConnection.this); + XMPPTCPConnection.this, "shutdown completed"); /** * If set, the stanza(/packet) writer is shut down