Skip to content

Commit f984674

Browse files
authored
AMQNET-848 Failover Transport Protocol Excessive Reconnection Attempts
* AMQNET-848-Failover-Transport-Protocol-Excessive-Reconnection-Attempts-on-Credential-Failure * Fixed the co pilot review comments
1 parent ea15293 commit f984674

4 files changed

Lines changed: 151 additions & 9 deletions

File tree

src/Transport/Failover/FailoverTransport.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -468,9 +468,13 @@ public void HandleTransportFailure(Exception e)
468468
{
469469
if (CanReconnect())
470470
{
471-
Tracer.WarnFormat("Transport failed to {0}, attempting to automatically reconnect due to: {1}",
472-
ConnectedTransportURI, e.Message);
473-
reconnectOk = true;
471+
//Check to see if the exception is a security exception
472+
if (!(e is NMSSecurityException))
473+
{
474+
Tracer.WarnFormat("Transport failed to {0}, attempting to automatically reconnect due to: {1}",
475+
ConnectedTransportURI, e.Message);
476+
reconnectOk = true;
477+
}
474478
}
475479

476480
initialized = false;

src/Transport/InactivityMonitor.cs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
using System.Threading;
2020
using Apache.NMS.ActiveMQ.Commands;
2121
using Apache.NMS.ActiveMQ.Threads;
22+
using Apache.NMS.ActiveMQ.Util;
2223
using Apache.NMS.ActiveMQ.Util.Synchronization;
2324
using Apache.NMS.Util;
2425

@@ -230,32 +231,44 @@ protected override async System.Threading.Tasks.Task OnCommand(ITransport sender
230231
inRead.Value = true;
231232
try
232233
{
233-
if(command.IsKeepAliveInfo)
234+
if (command is ExceptionResponse)
235+
{
236+
ExceptionResponse error = command as ExceptionResponse;
237+
NMSException exception = ExceptionFromBrokerError.CreateExceptionFromBrokerError(error.Exception);
238+
if (exception is NMSSecurityException)
239+
{
240+
OnException(this, exception);
241+
}
242+
else
243+
{
244+
Tracer.WarnFormat("ExceptionResponse received from the broker:{0}", command.GetType());
245+
}
246+
}else if (command.IsKeepAliveInfo)
234247
{
235248
KeepAliveInfo info = command as KeepAliveInfo;
236-
if(info.ResponseRequired)
249+
if (info.ResponseRequired)
237250
{
238251
try
239252
{
240253
info.ResponseRequired = false;
241254
Oneway(info);
242255
}
243-
catch(IOException ex)
256+
catch (IOException ex)
244257
{
245258
OnException(this, ex);
246259
}
247260
}
248261
}
249-
else if(command.IsWireFormatInfo)
262+
else if (command.IsWireFormatInfo)
250263
{
251-
lock(monitor)
264+
lock (monitor)
252265
{
253266
remoteWireFormatInfo = command as WireFormatInfo;
254267
try
255268
{
256269
StartMonitorThreads();
257270
}
258-
catch(IOException ex)
271+
catch (IOException ex)
259272
{
260273
OnException(this, ex);
261274
}
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
using Apache.NMS.ActiveMQ.Commands;
2+
using System;
3+
using System.Reflection;
4+
5+
6+
namespace Apache.NMS.ActiveMQ.Util
7+
{
8+
internal class ExceptionFromBrokerError
9+
{
10+
public static NMSException CreateExceptionFromBrokerError(BrokerError brokerError)
11+
{
12+
String exceptionClassName = brokerError.ExceptionClass;
13+
14+
if (String.IsNullOrEmpty(exceptionClassName))
15+
{
16+
return new BrokerException(brokerError);
17+
}
18+
19+
NMSException exception = null;
20+
String message = brokerError.Message;
21+
22+
// We only create instances of exceptions from the NMS API
23+
Assembly nmsAssembly = Assembly.GetAssembly(typeof(NMSException));
24+
25+
// First try and see if it's one we populated ourselves in which case
26+
// it will have the correct namespace and exception name.
27+
Type exceptionType = nmsAssembly.GetType(exceptionClassName, false, true);
28+
29+
// Exceptions from the broker don't have the same namespace, so we
30+
// trim that and try using the NMS namespace to see if we can get an
31+
// NMSException based version of the same type. We have to convert
32+
// the JMS prefixed exceptions to NMS also.
33+
if (null == exceptionType)
34+
{
35+
if (exceptionClassName.StartsWith("java.lang.SecurityException"))
36+
{
37+
exceptionClassName = "Apache.NMS.NMSSecurityException";
38+
}
39+
else if (!exceptionClassName.StartsWith("Apache.NMS"))
40+
{
41+
string transformClassName;
42+
43+
if (exceptionClassName.Contains("."))
44+
{
45+
int pos = exceptionClassName.LastIndexOf(".");
46+
transformClassName = exceptionClassName.Substring(pos + 1).Replace("JMS", "NMS");
47+
}
48+
else
49+
{
50+
transformClassName = exceptionClassName;
51+
}
52+
53+
exceptionClassName = "Apache.NMS." + transformClassName;
54+
}
55+
56+
exceptionType = nmsAssembly.GetType(exceptionClassName, false, true);
57+
}
58+
59+
if (exceptionType != null)
60+
{
61+
object[] args = null;
62+
if (!String.IsNullOrEmpty(message))
63+
{
64+
args = new object[1];
65+
args[0] = message;
66+
}
67+
68+
exception = Activator.CreateInstance(exceptionType, args) as NMSException;
69+
}
70+
else
71+
{
72+
exception = new BrokerException(brokerError);
73+
}
74+
75+
return exception;
76+
}
77+
}
78+
}

test/Transport/Inactivity/InactivityMonitorTest.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,53 @@ public void TestWriteMessageFail()
133133
{
134134
}
135135
}
136+
public class TestableInactivityMonitor : InactivityMonitor
137+
{
138+
public TestableInactivityMonitor(ITransport next) : base(next) { }
139+
140+
// Expose protected method for testing
141+
public Task TestOnCommand(ITransport sender, Command command)
142+
{
143+
return OnCommand(sender, command);
144+
}
145+
}
146+
[Test]
147+
public void OnCommand_WithNMSSecurityException_ShouldCallOnException()
148+
{
149+
// Arrange
150+
var brokerError = new BrokerError
151+
{
152+
ExceptionClass = "javax.jms.JMSSecurityException",
153+
Message = "Authentication failed"
154+
};
155+
156+
var exceptionResponse = new ExceptionResponse
157+
{
158+
Exception = brokerError
159+
};
160+
161+
// Mock the static method call - this would require making ExceptionFromBrokerError testable
162+
// For this test, we'll assume it returns an NMSSecurityException
163+
var securityException = new NMSSecurityException("Authentication failed");
164+
TestableInactivityMonitor monitor = new TestableInactivityMonitor(this.transport);
165+
monitor.Exception += new ExceptionHandler(OnException);
166+
monitor.CommandAsync += new CommandHandlerAsync(OnCommand);
167+
bool exceptionHandlerCalled = false;
168+
Exception caughtException = null;
169+
monitor.Exception += (sender, args) =>
170+
{
171+
exceptionHandlerCalled = true;
172+
caughtException = args;
173+
};
174+
// Act
175+
Task task=monitor.TestOnCommand(transport, exceptionResponse);
176+
task.Wait();
177+
// Assert
178+
Assert.IsTrue(exceptionHandlerCalled, "Exception handler should have been called");
179+
Assert.IsNotNull(caughtException, "Exception should have been caught");
180+
Assert.IsInstanceOf<NMSSecurityException>(caughtException, "Should be NMSSecurityException");
181+
Assert.AreEqual("Authentication failed", caughtException.Message);
182+
}
136183

137184
[Test]
138185
public void TestNonFailureSendCase()

0 commit comments

Comments
 (0)