fix(iot-dev): Fix bug where routine MQTT SAS token expired disconnects didn't provide the correct reason

device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransport.java

@@ -1045,12 +1045,7 @@ private IotHubConnectionStatusChangeReason exceptionToStatusChangeReason(Throwab
         if (e instanceof TransportException)
         {
             TransportException transportException = (TransportException) e;
-            if (transportException.isRetryable())
-            {
-                log.debug("Mapping throwable to NO_NETWORK because it was a retryable exception", e);
-                return IotHubConnectionStatusChangeReason.NO_NETWORK;
-            }
-            else if (isSasTokenExpired())
+            if (isSasTokenExpired())
             {
                 log.debug("Mapping throwable to EXPIRED_SAS_TOKEN because it was a non-retryable exception and the saved sas token has expired", e);
                 return IotHubConnectionStatusChangeReason.EXPIRED_SAS_TOKEN;
@@ -1060,6 +1055,11 @@ else if (e instanceof UnauthorizedException || e instanceof MqttUnauthorizedExce
                 log.debug("Mapping throwable to BAD_CREDENTIAL because it was a non-retryable exception authorization exception but the saved sas token has not expired yet", e);
                 return IotHubConnectionStatusChangeReason.BAD_CREDENTIAL;
             }
+            else if (transportException.isRetryable())
+            {
+                log.debug("Mapping throwable to NO_NETWORK because it was a retryable exception", e);
+                return IotHubConnectionStatusChangeReason.NO_NETWORK;
+            }
         }
 
         log.debug("Mapping exception throwable to COMMUNICATION_ERROR because the sdk was unable to classify the thrown exception to anything other category", e);

device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportTest.java

@@ -786,14 +786,6 @@ public void exceptionToStatusChangeReasonSasTokenExpired()
         };
         final IotHubTransport transport = new IotHubTransport(mockedConfig, mockedIotHubConnectionStatusChangeCallback, false);
 
-        new Expectations()
-        {
-            {
-                mockedTransportException.isRetryable();
-                result = false;
-            }
-        };
-
         //act
         IotHubConnectionStatusChangeReason reason = Deencapsulation.invoke(transport, "exceptionToStatusChangeReason", new Class[] {Throwable.class}, mockedTransportException);
 

iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/TokenRenewalTests.java

@@ -150,6 +150,7 @@ public void tokenRenewalWorks() throws Exception
         Success[] amqpDisconnectDidNotHappenSuccesses = new Success[clients.size()];
         Success[] mqttDisconnectDidHappenSuccesses = new Success[clients.size()];
         Success[] shutdownWasGracefulSuccesses = new Success[clients.size()];
+        Success[] mqttDisconnectHadTokenExpiredReasonSuccesses = new Success[clients.size()];
         for (int clientIndex = 0; clientIndex < clients.size(); clientIndex++)
         {
             amqpDisconnectDidNotHappenSuccesses[clientIndex] = new Success();
@@ -159,8 +160,16 @@ public void tokenRenewalWorks() throws Exception
             amqpDisconnectDidNotHappenSuccesses[clientIndex].setResult(true); //assume success until unexpected DISCONNECTED_RETRYING
             mqttDisconnectDidHappenSuccesses[clientIndex].setResult(false); //assume failure until DISCONNECTED_RETRYING is triggered by token expiring
             shutdownWasGracefulSuccesses[clientIndex].setResult(true); //assume success until DISCONNECTED callback without CLIENT_CLOSE
-
-            clients.get(clientIndex).registerConnectionStatusChangeCallback(new IotHubConnectionStatusChangeTokenRenewalCallbackVerifier(clients.get(clientIndex).getConfig().getProtocol(), amqpDisconnectDidNotHappenSuccesses[clientIndex], mqttDisconnectDidHappenSuccesses[clientIndex], shutdownWasGracefulSuccesses[clientIndex]), clients.get(clientIndex));
+            mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex].setResult(false); //assume failure until first disconnected_retrying executes with reason EXPIRED_SAS_TOKEN
+
+            clients.get(clientIndex).registerConnectionStatusChangeCallback(
+                new IotHubConnectionStatusChangeTokenRenewalCallbackVerifier(
+                    clients.get(clientIndex).getConfig().getProtocol(),
+                    amqpDisconnectDidNotHappenSuccesses[clientIndex],
+                    mqttDisconnectDidHappenSuccesses[clientIndex],
+                    shutdownWasGracefulSuccesses[clientIndex],
+                    mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex]),
+                clients.get(clientIndex));
         }
 
         openEachClient(clients);
@@ -182,7 +191,7 @@ public void tokenRenewalWorks() throws Exception
 
         testIdentities.clear();
 
-        verifyClientsConnectivityBehavedCorrectly(clients, amqpDisconnectDidNotHappenSuccesses, mqttDisconnectDidHappenSuccesses, shutdownWasGracefulSuccesses);
+        verifyClientsConnectivityBehavedCorrectly(clients, amqpDisconnectDidNotHappenSuccesses, mqttDisconnectDidHappenSuccesses, shutdownWasGracefulSuccesses, mqttDisconnectHadTokenExpiredReasonSuccesses);
     }
 
     private void closeClients(List<InternalClient> clients) throws IOException
@@ -201,7 +210,12 @@ private void closeClients(List<InternalClient> clients) throws IOException
         }
     }
 
-    private void verifyClientsConnectivityBehavedCorrectly(List<InternalClient> clients, Success[] amqpDisconnectDidNotHappenSuccesses, Success[] mqttDisconnectDidHappenSuccesses, Success[] shutdownWasGracefulSuccesses)
+    private void verifyClientsConnectivityBehavedCorrectly(
+        List<InternalClient> clients,
+        Success[] amqpDisconnectDidNotHappenSuccesses,
+        Success[] mqttDisconnectDidHappenSuccesses,
+        Success[] shutdownWasGracefulSuccesses,
+        Success[] mqttDisconnectHadTokenExpiredReasonSuccesses)
     {
         for (int clientIndex = 0; clientIndex < clients.size(); clientIndex++)
         {
@@ -212,6 +226,7 @@ private void verifyClientsConnectivityBehavedCorrectly(List<InternalClient> clie
             {
                 assertTrue(CorrelationDetailsLoggingAssert.buildExceptionMessage("MQTT connection was never lost, token renewal was not tested", client), mqttDisconnectDidHappenSuccesses[clientIndex].result);
                 assertTrue(CorrelationDetailsLoggingAssert.buildExceptionMessage("Connection was lost during test, or shutdown was not graceful", client), shutdownWasGracefulSuccesses[clientIndex].result);
+                assertTrue(CorrelationDetailsLoggingAssert.buildExceptionMessage("MQTT connection was lost, but for an unexpected reason", client), mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex].result);
             }
             else if (protocol == AMQPS || protocol == AMQPS_WS)
             {
@@ -304,13 +319,15 @@ private static class IotHubConnectionStatusChangeTokenRenewalCallbackVerifier im
         Success amqpDisconnectDidNotHappen;
         Success mqttDisconnectDidHappen;
         Success shutdownWasGraceful;
+        Success mqttDisconnectHadTokenExpiredReason;
 
-        public IotHubConnectionStatusChangeTokenRenewalCallbackVerifier(IotHubClientProtocol protocol, Success amqpDisconnectDidNotHappen, Success mqttDisconnectDidHappen, Success shutdownWasGraceful)
+        public IotHubConnectionStatusChangeTokenRenewalCallbackVerifier(IotHubClientProtocol protocol, Success amqpDisconnectDidNotHappen, Success mqttDisconnectDidHappen, Success shutdownWasGraceful, Success mqttDisconnectHadTokenExpiredReason)
         {
             this.protocol = protocol;
             this.mqttDisconnectDidHappen = mqttDisconnectDidHappen;
             this.amqpDisconnectDidNotHappen = amqpDisconnectDidNotHappen;
             this.shutdownWasGraceful = shutdownWasGraceful;
+            this.mqttDisconnectHadTokenExpiredReason = mqttDisconnectHadTokenExpiredReason;
         }
 
         @Override
@@ -345,6 +362,12 @@ else if (status == IotHubConnectionStatus.DISCONNECTED_RETRYING)
                 {
                     amqpDisconnectDidNotHappen.setResult(false);
                 }
+
+                if (protocol == MQTT || protocol == MQTT_WS)
+                {
+                    boolean reasonIsSasTokenExpired = statusChangeReason == IotHubConnectionStatusChangeReason.EXPIRED_SAS_TOKEN;
+                    mqttDisconnectHadTokenExpiredReason.setResult(reasonIsSasTokenExpired);
+                }
             }
         }
     }