[
https://issues.apache.org/jira/browse/AMQNET-848?focusedWorklogId=974251&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-974251
]
ASF GitHub Bot logged work on AMQNET-848:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 11/Jul/25 06:53
Start Date: 11/Jul/25 06:53
Worklog Time Spent: 10m
Work Description: Copilot commented on code in PR #42:
URL:
https://github.com/apache/activemq-nms-openwire/pull/42#discussion_r2199797839
##########
src/Transport/InactivityMonitor.cs:
##########
@@ -230,32 +231,44 @@ protected override async System.Threading.Tasks.Task
OnCommand(ITransport sender
inRead.Value = true;
try
{
- if(command.IsKeepAliveInfo)
+ if (command is ExceptionResponse)
+ {
+ ExceptionResponse error = command as ExceptionResponse;
+ NMSException exception =
ExceptionFromBrokerError.CreateExceptionFromBrokerError(error.Exception);
+ if (exception is NMSSecurityException)
+ {
+ OnException(this, exception);
+ }
+ else
+ {
+ Tracer.WarnFormat("ExceptionResponse received from the
broker:", command.GetType());
Review Comment:
The format string has no placeholder for the command type argument, so
command.GetType() will be ignored. Add a placeholder, e.g., "ExceptionResponse
received from the broker: {0}".
```suggestion
Tracer.WarnFormat("ExceptionResponse received from
the broker: {0}", command.GetType());
```
##########
src/Util/ExceptionFromBrokerError.cs:
##########
@@ -0,0 +1,78 @@
+using Apache.NMS.ActiveMQ.Commands;
+using System;
+using System.Reflection;
+
+
+namespace Apache.NMS.ActiveMQ.Util
+{
+ internal class ExceptionFromBrokerError
+ {
+ public static NMSException CreateExceptionFromBrokerError(BrokerError
brokerError)
+ {
+ String exceptionClassName = brokerError.ExceptionClass;
+
+ if (String.IsNullOrEmpty(exceptionClassName))
+ {
+ return new BrokerException(brokerError);
+ }
+
+ NMSException exception = null;
+ String message = brokerError.Message;
+
+ // We only create instances of exceptions from the NMS API
+ Assembly nmsAssembly = Assembly.GetAssembly(typeof(NMSException));
+
+ // First try and see if it's one we populated ourselves in which
case
+ // it will have the correct namespace and exception name.
+ Type exceptionType = nmsAssembly.GetType(exceptionClassName,
false, true);
Review Comment:
[nitpick] ExceptionFromBrokerError performs reflection lookups on every
call, which can be expensive. Consider caching the resolved exception types or
constructors to improve performance under high load.
##########
test/Transport/Inactivity/InactivityMonitorTest.cs:
##########
@@ -133,6 +133,51 @@ public void TestWriteMessageFail()
{
}
}
+ public class TestableInactivityMonitor : InactivityMonitor
+ {
+ public TestableInactivityMonitor(ITransport next) : base(next) { }
+
+ // Expose protected method for testing
+ public async Task TestOnCommand(ITransport sender, Command command)
+ {
+ await OnCommand(sender, command);
+ }
+ }
+ [Test]
+ public void OnCommand_WithNMSSecurityException_ShouldCallOnException()
+ {
+ // Arrange
+ var brokerError = new BrokerError
+ {
+ ExceptionClass = "javax.jms.JMSSecurityException",
+ Message = "Authentication failed"
+ };
+
+ var exceptionResponse = new ExceptionResponse
+ {
+ Exception = brokerError
+ };
+
+ // Mock the static method call - this would require making
ExceptionFromBrokerError testable
+ // For this test, we'll assume it returns an NMSSecurityException
+ var securityException = new NMSSecurityException("Authentication
failed");
+ TestableInactivityMonitor monitor = new
TestableInactivityMonitor(this.transport);
+ monitor.Exception += new ExceptionHandler(OnException);
+ bool exceptionHandlerCalled = false;
+ Exception caughtException = null;
+ monitor.Exception += (sender, args) =>
+ {
+ exceptionHandlerCalled = true;
+ caughtException = args;
+ };
+ // Act
+ _ = monitor.TestOnCommand(transport, exceptionResponse);
Review Comment:
The async TestOnCommand call is not awaited, so the test may complete before
the command handling finishes. Consider awaiting the Task to ensure the event
is raised before assertions.
Issue Time Tracking
-------------------
Worklog Id: (was: 974251)
Time Spent: 20m (was: 10m)
> Failover Transport Protocol Excessive Reconnection Attempts on Credential
> Failure
> ---------------------------------------------------------------------------------
>
> Key: AMQNET-848
> URL: https://issues.apache.org/jira/browse/AMQNET-848
> Project: ActiveMQ .Net
> Issue Type: Bug
> Components: NMS
> Affects Versions: OpenWire-2.1.0
> Reporter: Murali Mogalayapalli
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> *Failover Transport Protocol behavior:*
> When authentication credentials are invalid (e.g., expired or incorrect), the
> AMQNet client’s *failover mechanism* enters a rapid reconnect loop. This
> results in a high-frequency reconnection pattern that behaves like a
> *denial-of-service (DoS)* against the broker.
> *Problem*
> The InactivityMonitor::OnCommand method doesn't handle the ExceptionResponse
> resulting in this behavior.
> *Fix*
> * Handle the ExceptionResponse the way it is handled in
> Conection::SyncRequestAsync method. Propagate the exception
> * Bail out the FailoverTransport from reconnecting when a
> NMSSecurityException is passed to FailoverTransport::HandleTransportFailure
> method.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
