From 9e5ac5a39ae5f2ec570be543debe424a31dd1209 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 17 Oct 2024 17:34:05 +0200 Subject: [PATCH] [core] Wrap current connection exception when re-throwing Instead of directly throwing the current connection exception, wrap it, so we do not lose the stack trace of the thread invoking waitForConditionorThrowConnectionException(). --- .../smack/AbstractXMPPConnection.java | 36 ++++++------------- .../ModularXmppClientToServerConnection.java | 9 +++-- ...rXmppClientToServerConnectionInternal.java | 4 ++- .../smack/tcp/XMPPTCPConnection.java | 7 ++-- .../smack/tcp/XmppTcpTransportModule.java | 7 ++-- 5 files changed, 24 insertions(+), 39 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 a8e0909c6..2967a44f1 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -281,8 +281,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { */ protected Writer writer; - protected SmackException currentSmackException; - protected XMPPException currentXmppException; + private Exception currentConnectionException; protected boolean tlsHandled; @@ -511,8 +510,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { public abstract boolean isUsingCompression(); protected void initState() { - currentSmackException = null; - currentXmppException = null; + currentConnectionException = null; saslFeatureReceived = lastFeaturesReceived = tlsHandled = false; // TODO: We do not init closingStreamReceived here, as the integration tests use it to check if we waited for // it. @@ -686,28 +684,12 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { return streamId; } - protected final void throwCurrentConnectionException() throws SmackException, XMPPException { - if (currentSmackException != null) { - throw currentSmackException; - } else if (currentXmppException != null) { - throw currentXmppException; - } - - throw new AssertionError("No current connection exception set, although throwCurrentException() was called"); - } - protected final boolean hasCurrentConnectionException() { - return currentSmackException != null || currentXmppException != null; + return currentConnectionException != null; } protected final void setCurrentConnectionExceptionAndNotify(Exception exception) { - if (exception instanceof SmackException) { - currentSmackException = (SmackException) exception; - } else if (exception instanceof XMPPException) { - currentXmppException = (XMPPException) exception; - } else { - currentSmackException = new SmackException.SmackWrappedException(exception); - } + currentConnectionException = exception; notifyWaitingThreads(); } @@ -741,10 +723,12 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { return true; } - protected final void waitForConditionOrThrowConnectionException(Supplier condition, String waitFor) throws InterruptedException, SmackException, XMPPException { + protected final void waitForConditionOrThrowConnectionException(Supplier condition, String waitFor) + throws InterruptedException, SmackException.SmackWrappedException, NoResponseException { boolean success = waitFor(() -> condition.get().booleanValue() || hasCurrentConnectionException()); - if (hasCurrentConnectionException()) { - throwCurrentConnectionException(); + final Exception currentConnectionException = this.currentConnectionException; + if (currentConnectionException != null) { + throw new SmackException.SmackWrappedException(currentConnectionException); } // If there was no connection exception and we still did not successfully wait for the condition to hold, then @@ -1048,7 +1032,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { protected final boolean waitForClosingStreamTagFromServer() { try { waitForConditionOrThrowConnectionException(() -> closingStreamReceived, "closing stream tag from the server"); - } catch (InterruptedException | SmackException | XMPPException e) { + } catch (InterruptedException | SmackException.SmackWrappedException | NoResponseException e) { LOGGER.log(Level.INFO, "Exception while waiting for closing stream element from the server " + this, e); return false; } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java index ca3de8190..8cdac17b9 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java @@ -39,6 +39,7 @@ import org.jivesoftware.smack.SmackException; import org.jivesoftware.smack.SmackException.NoResponseException; import org.jivesoftware.smack.SmackException.NotConnectedException; import org.jivesoftware.smack.SmackException.OutgoingQueueFullException; +import org.jivesoftware.smack.SmackException.SmackWrappedException; import org.jivesoftware.smack.SmackFuture; import org.jivesoftware.smack.XMPPException; import org.jivesoftware.smack.XMPPException.FailedNonzaException; @@ -259,7 +260,7 @@ public final class ModularXmppClientToServerConnection extends AbstractXMPPConne @Override public void waitForConditionOrThrowConnectionException(Supplier condition, String waitFor) - throws InterruptedException, SmackException, XMPPException { + throws InterruptedException, SmackWrappedException, NoResponseException { ModularXmppClientToServerConnection.this.waitForConditionOrThrowConnectionException(condition, waitFor); } @@ -596,8 +597,7 @@ public final class ModularXmppClientToServerConnection extends AbstractXMPPConne case "error": StreamError streamError = PacketParserUtils.parseStreamError(parser, null); StreamErrorException streamErrorException = new StreamErrorException(streamError); - currentXmppException = streamErrorException; - notifyWaitingThreads(); + setCurrentConnectionExceptionAndNotify(streamErrorException); throw streamErrorException; case "features": parseFeatures(parser); @@ -1048,8 +1048,7 @@ public final class ModularXmppClientToServerConnection extends AbstractXMPPConne XmppInputOutputFilter filter = it.next(); try { filter.waitUntilInputOutputClosed(); - } catch (IOException | CertificateException | InterruptedException | SmackException - | XMPPException e) { + } catch (IOException | CertificateException | InterruptedException | SmackException | XMPPException e) { LOGGER.log(Level.WARNING, "waitUntilInputOutputClosed() threw", e); } } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/c2s/internal/ModularXmppClientToServerConnectionInternal.java b/smack-core/src/main/java/org/jivesoftware/smack/c2s/internal/ModularXmppClientToServerConnectionInternal.java index bc8b9d441..42cf9c650 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/c2s/internal/ModularXmppClientToServerConnectionInternal.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/c2s/internal/ModularXmppClientToServerConnectionInternal.java @@ -26,6 +26,7 @@ import java.util.Queue; import org.jivesoftware.smack.SmackException; import org.jivesoftware.smack.SmackException.NoResponseException; import org.jivesoftware.smack.SmackException.NotConnectedException; +import org.jivesoftware.smack.SmackException.SmackWrappedException; import org.jivesoftware.smack.SmackReactor; import org.jivesoftware.smack.SmackReactor.ChannelSelectedCallback; import org.jivesoftware.smack.XMPPException; @@ -127,7 +128,8 @@ public abstract class ModularXmppClientToServerConnectionInternal { public abstract void asyncGo(Runnable runnable); - public abstract void waitForConditionOrThrowConnectionException(Supplier condition, String waitFor) throws InterruptedException, SmackException, XMPPException; + public abstract void waitForConditionOrThrowConnectionException(Supplier condition, String waitFor) + throws InterruptedException, SmackWrappedException, NoResponseException; public abstract void notifyWaitingThreads(); 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 1e622bedc..8f9b8a9cb 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 @@ -889,8 +889,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { if (startTlsFeature != null) { if (startTlsFeature.required() && config.getSecurityMode() == SecurityMode.disabled) { SecurityRequiredByServerException smackException = new SecurityRequiredByServerException(); - currentSmackException = smackException; - notifyWaitingThreads(); + setCurrentConnectionExceptionAndNotify(smackException); throw smackException; } @@ -1020,8 +1019,8 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { // situation. It is still possible to authenticate and // use the connection but using an uncompressed connection // TODO Parse failure stanza - currentSmackException = new SmackException.SmackMessageException("Could not establish compression"); - notifyWaitingThreads(); + SmackException.SmackMessageException exception = new SmackException.SmackMessageException("Could not establish compression"); + setCurrentConnectionExceptionAndNotify(exception); break; default: parseAndProcessNonza(parser); diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XmppTcpTransportModule.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XmppTcpTransportModule.java index fabfa4d6f..b7f7e6a98 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XmppTcpTransportModule.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XmppTcpTransportModule.java @@ -46,9 +46,11 @@ import javax.net.ssl.SSLSession; import org.jivesoftware.smack.ConnectionConfiguration.SecurityMode; import org.jivesoftware.smack.SmackException; +import org.jivesoftware.smack.SmackException.NoResponseException; import org.jivesoftware.smack.SmackException.SecurityRequiredByClientException; import org.jivesoftware.smack.SmackException.SecurityRequiredByServerException; import org.jivesoftware.smack.SmackException.SmackCertificateException; +import org.jivesoftware.smack.SmackException.SmackWrappedException; import org.jivesoftware.smack.SmackFuture; import org.jivesoftware.smack.SmackFuture.InternalSmackFuture; import org.jivesoftware.smack.SmackReactor.SelectionKeyAttachment; @@ -1201,7 +1203,7 @@ public class XmppTcpTransportModule extends ModularXmppClientToServerConnectionM return handshakeStatus == TlsHandshakeStatus.successful || handshakeStatus == TlsHandshakeStatus.failed; } - private void waitForHandshakeFinished() throws InterruptedException, CertificateException, SSLException, SmackException, XMPPException { + private void waitForHandshakeFinished() throws InterruptedException, CertificateException, SSLException, SmackWrappedException, NoResponseException { connectionInternal.waitForConditionOrThrowConnectionException(() -> isHandshakeFinished(), "TLS handshake to finish"); if (handshakeStatus == TlsHandshakeStatus.failed) { @@ -1234,8 +1236,7 @@ public class XmppTcpTransportModule extends ModularXmppClientToServerConnectionM } @Override - public void waitUntilInputOutputClosed() throws IOException, CertificateException, InterruptedException, - SmackException, XMPPException { + public void waitUntilInputOutputClosed() throws IOException, CertificateException, InterruptedException, SmackWrappedException, NoResponseException { waitForHandshakeFinished(); }