From d976434bb3a0f1a7107e04a34b863a198a731590 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 29 Nov 2016 16:40:08 +0100 Subject: [PATCH] Improve message of XMPPErrorException by including the XMPP entity which send the XMPP error reply to us. Also cleanup the no longer used constructors. --- .../org/jivesoftware/smack/XMPPException.java | 90 +++++++++++-------- .../socks5/Socks5BytestreamRequest.java | 2 +- .../smackx/commands/AdHocCommandManager.java | 1 + .../filetransfer/FileTransferNegotiator.java | 2 +- .../jivesoftware/util/ConnectionUtils.java | 4 +- ...SmackIntegrationTestFrameworkUnitTest.java | 6 +- .../smack/tcp/XMPPTCPConnection.java | 17 ++-- 7 files changed, 65 insertions(+), 57 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/XMPPException.java b/smack-core/src/main/java/org/jivesoftware/smack/XMPPException.java index 6ee7d1658..99f3fb4b5 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/XMPPException.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/XMPPException.java @@ -17,9 +17,11 @@ package org.jivesoftware.smack; +import org.jivesoftware.smack.packet.Nonza; import org.jivesoftware.smack.packet.Stanza; import org.jivesoftware.smack.packet.StreamError; import org.jivesoftware.smack.packet.XMPPError; +import org.jxmpp.jid.Jid; /** * A generic exception that is thrown when an error occurs performing an @@ -72,48 +74,28 @@ public abstract class XMPPException extends Exception { */ private static final long serialVersionUID = 212790389529249604L; private final XMPPError error; + private final Stanza stanza; + /** + * Creates a new XMPPErrorException with the given builder. + * + * @param xmppErrorBuilder + * @deprecated Use {@link #XMPPErrorException(Stanza, XMPPError)} instead. + */ + @Deprecated public XMPPErrorException(XMPPError.Builder xmppErrorBuilder) { - this(xmppErrorBuilder.build()); + this(null, xmppErrorBuilder.build()); } /** - * Creates a new XMPPException with the XMPPError that was the root case of the exception. + * Creates a new XMPPErrorException with the XMPPError that was the root case of the exception. * * @param error the root cause of the exception. */ - public XMPPErrorException(XMPPError error) { + public XMPPErrorException(Stanza stanza, XMPPError error) { super(); this.error = error; - } - - /** - * Creates a new XMPPException with a description of the exception, an XMPPError, and the - * Throwable that was the root cause of the exception. - * - * @param message a description of the exception. - * @param error the root cause of the exception. - * @param wrappedThrowable the root cause of the exception. - * @deprecated use {@link #XMPPException.XMPPErrorException(XMPPError)} instead. - */ - @Deprecated - public XMPPErrorException(String message, XMPPError error, Throwable wrappedThrowable) { - super(message, wrappedThrowable); - this.error = error; - } - - /** - * Creates a new XMPPException with a description of the exception and the XMPPException - * that was the root cause of the exception. - * - * @param message a description of the exception. - * @param error the root cause of the exception. - * @deprecated use {@link #XMPPException.XMPPErrorException(XMPPError)} instead. - */ - @Deprecated - public XMPPErrorException(String message, XMPPError error) { - super(message); - this.error = error; + this.stanza = stanza; } /** @@ -128,23 +110,53 @@ public abstract class XMPPException extends Exception { @Override public String getMessage() { - String superMessage = super.getMessage(); - if (superMessage != null) { - return superMessage; - } - else { - return error.toString(); + StringBuilder sb = new StringBuilder(); + + if (stanza != null) { + Jid from = stanza.getFrom(); + if (from != null) { + sb.append("XMPP error reply received from " + from + ": "); + } } + + sb.append(error); + + return sb.toString(); } public static void ifHasErrorThenThrow(Stanza packet) throws XMPPErrorException { XMPPError xmppError = packet.getError(); if (xmppError != null) { - throw new XMPPErrorException(xmppError); + throw new XMPPErrorException(packet, xmppError); } } } + public static class FailedNonzaException extends XMPPException { + + /** + * + */ + private static final long serialVersionUID = 1L; + + private final XMPPError.Condition condition; + + private final Nonza nonza; + + public FailedNonzaException(Nonza nonza, XMPPError.Condition condition) { + this.condition = condition; + this.nonza = nonza; + } + + public XMPPError.Condition getCondition() { + return condition; + } + + public Nonza getNonza() { + return nonza; + } + } + public static class StreamErrorException extends XMPPException { /** * diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamRequest.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamRequest.java index 0da45e27b..18f1bbac7 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamRequest.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamRequest.java @@ -280,7 +280,7 @@ public class Socks5BytestreamRequest implements BytestreamRequest { XMPPError.Builder error = XMPPError.from(XMPPError.Condition.item_not_found, errorMessage); IQ errorIQ = IQ.createErrorResponse(this.bytestreamRequest, error); this.manager.getConnection().sendStanza(errorIQ); - throw new XMPPErrorException(error); + throw new XMPPErrorException(errorIQ, error.build()); } /** diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/commands/AdHocCommandManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/commands/AdHocCommandManager.java index f5c1c8249..b0e9902a1 100755 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/commands/AdHocCommandManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/commands/AdHocCommandManager.java @@ -615,6 +615,7 @@ public final class AdHocCommandManager extends Manager { * @return the command instance to execute. * @throws XMPPErrorException if there is problem creating the new instance. */ + @SuppressWarnings("deprecation") private LocalCommand newInstanceOfCmd(String commandNode, String sessionID) throws XMPPErrorException { AdHocCommandInfo commandInfo = commands.get(commandNode); diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java index 77db7ed49..60292e69c 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java @@ -327,7 +327,7 @@ public final class FileTransferNegotiator extends Manager { } else { - throw new XMPPErrorException(iqResponse.getError()); + throw new XMPPErrorException(iqResponse, iqResponse.getError()); } } else { diff --git a/smack-extensions/src/test/java/org/jivesoftware/util/ConnectionUtils.java b/smack-extensions/src/test/java/org/jivesoftware/util/ConnectionUtils.java index 1f120e03d..2010272ce 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/util/ConnectionUtils.java +++ b/smack-extensions/src/test/java/org/jivesoftware/util/ConnectionUtils.java @@ -30,7 +30,6 @@ import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.filter.StanzaFilter; import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.Stanza; -import org.jivesoftware.smack.packet.XMPPError; import org.jivesoftware.smackx.disco.ServiceDiscoveryManager; import org.jxmpp.jid.DomainBareJid; import org.jxmpp.jid.EntityFullJid; @@ -113,8 +112,7 @@ public class ConnectionUtils { public Stanza answer(InvocationOnMock invocation) throws Throwable { Stanza packet = protocol.getResponses().poll(); if (packet == null) return packet; - XMPPError xmppError = packet.getError(); - if (xmppError != null) throw new XMPPErrorException(xmppError); + XMPPErrorException.ifHasErrorThenThrow(packet); return packet; } }; diff --git a/smack-integration-test/src/test/java/org/igniterealtime/smack/inttest/unittest/SmackIntegrationTestFrameworkUnitTest.java b/smack-integration-test/src/test/java/org/igniterealtime/smack/inttest/unittest/SmackIntegrationTestFrameworkUnitTest.java index e44395c74..3bb1c357c 100644 --- a/smack-integration-test/src/test/java/org/igniterealtime/smack/inttest/unittest/SmackIntegrationTestFrameworkUnitTest.java +++ b/smack-integration-test/src/test/java/org/igniterealtime/smack/inttest/unittest/SmackIntegrationTestFrameworkUnitTest.java @@ -35,6 +35,7 @@ import org.igniterealtime.smack.inttest.SmackIntegrationTestFramework.TestRunRes import org.jivesoftware.smack.SmackException; import org.jivesoftware.smack.XMPPException; import org.jivesoftware.smack.XMPPException.XMPPErrorException; +import org.jivesoftware.smack.packet.Message; import org.jivesoftware.smack.packet.XMPPError; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -97,8 +98,9 @@ public class SmackIntegrationTestFrameworkUnitTest { @SmackIntegrationTest public void throwRuntimeExceptionTest() throws XMPPErrorException { - throw new XMPPException.XMPPErrorException( - XMPPError.from(XMPPError.Condition.bad_request, DESCRIPTIVE_TEXT)); + Message message = new Message(); + throw new XMPPException.XMPPErrorException(message, + XMPPError.from(XMPPError.Condition.bad_request, DESCRIPTIVE_TEXT).build()); } } 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 b9444a8ed..3619525bc 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 @@ -31,10 +31,10 @@ import org.jivesoftware.smack.SmackException.NotConnectedException; import org.jivesoftware.smack.SmackException.ConnectionException; import org.jivesoftware.smack.SmackException.SecurityRequiredByServerException; import org.jivesoftware.smack.SynchronizationPoint; +import org.jivesoftware.smack.XMPPException.FailedNonzaException; import org.jivesoftware.smack.XMPPException.StreamErrorException; import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.XMPPException; -import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.compress.packet.Compressed; import org.jivesoftware.smack.compression.XMPPInputOutputStream; import org.jivesoftware.smack.filter.StanzaFilter; @@ -67,7 +67,6 @@ import org.jivesoftware.smack.sm.packet.StreamManagement.StreamManagementFeature import org.jivesoftware.smack.sm.predicates.Predicate; import org.jivesoftware.smack.sm.provider.ParseStreamManagement; import org.jivesoftware.smack.packet.Nonza; -import org.jivesoftware.smack.packet.XMPPError; import org.jivesoftware.smack.proxy.ProxyInfo; import org.jivesoftware.smack.util.ArrayBlockingQueueWithShutdown; import org.jivesoftware.smack.util.Async; @@ -220,10 +219,10 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { */ private String smSessionId; - private final SynchronizationPoint smResumedSyncPoint = new SynchronizationPoint( + private final SynchronizationPoint smResumedSyncPoint = new SynchronizationPoint<>( this, "stream resumed element"); - private final SynchronizationPoint smEnabledSyncPoint = new SynchronizationPoint( + private final SynchronizationPoint smEnabledSyncPoint = new SynchronizationPoint<>( this, "stream enabled element"); /** @@ -1094,10 +1093,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { if (enabled.isResumeSet()) { smSessionId = enabled.getId(); if (StringUtils.isNullOrEmpty(smSessionId)) { - XMPPError.Builder builder = XMPPError.getBuilder(XMPPError.Condition.bad_request); - builder.setDescriptiveEnText("Stream Management 'enabled' element with resume attribute but without session id received"); - XMPPErrorException xmppException = new XMPPErrorException( - builder); + SmackException xmppException = new SmackException("Stream Management 'enabled' element with resume attribute but without session id received"); smEnabledSyncPoint.reportFailure(xmppException); throw xmppException; } @@ -1113,8 +1109,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { break; case Failed.ELEMENT: Failed failed = ParseStreamManagement.failed(parser); - XMPPError.Builder xmppError = XMPPError.getBuilder(failed.getXMPPErrorCondition()); - XMPPException xmppException = new XMPPErrorException(xmppError); + FailedNonzaException xmppException = new FailedNonzaException(failed, failed.getXMPPErrorCondition()); // If only XEP-198 would specify different failure elements for the SM // enable and SM resume failure case. But this is not the case, so we // need to determine if this is a 'Failed' response for either 'Enable' @@ -1126,7 +1121,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { if (!smEnabledSyncPoint.requestSent()) { throw new IllegalStateException("Failed element received but SM was not previously enabled"); } - smEnabledSyncPoint.reportFailure(xmppException); + smEnabledSyncPoint.reportFailure(new SmackException(xmppException)); // Report success for last lastFeaturesReceived so that in case a // failed resumption, we can continue with normal resource binding. // See text of XEP-198 5. below Example 11.