From e458e14294d8c39f9e6f72e2c42418bfd298eeb2 Mon Sep 17 00:00:00 2001
From: Varun Puranik <vpuranik@microsoft.com>
Date: Fri, 24 May 2019 11:04:51 -0700
Subject: [PATCH] Dispose client on getting an Object Disposed exception
 (#1211) (#1251)

---
 .../CloudProxy.cs                                 | 15 +++++++++++----
 .../CloudProxyTest.cs                             | 10 ++++++----
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.CloudProxy/CloudProxy.cs b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.CloudProxy/CloudProxy.cs
index 12a4a55efe5..a2570cb61fd 100644
--- a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.CloudProxy/CloudProxy.cs
+++ b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.CloudProxy/CloudProxy.cs
@@ -12,6 +12,7 @@ namespace Microsoft.Azure.Devices.Edge.Hub.CloudProxy
     using Microsoft.Azure.Devices.Edge.Hub.Core;
     using Microsoft.Azure.Devices.Edge.Hub.Core.Cloud;
     using Microsoft.Azure.Devices.Edge.Util;
+    using Microsoft.Azure.Devices.Routing.Core.Query;
     using Microsoft.Azure.Devices.Shared;
     using Microsoft.Extensions.Logging;
     using Newtonsoft.Json;
@@ -19,6 +20,12 @@ namespace Microsoft.Azure.Devices.Edge.Hub.CloudProxy
 
     class CloudProxy : ICloudProxy
     {
+        static readonly Type[] NonRecoverableExceptions =
+        {
+            typeof(NullReferenceException),
+            typeof(ObjectDisposedException)
+        };
+
         readonly IClient client;
         readonly IMessageConverterProvider messageConverterProvider;
         readonly Action<string, CloudConnectionStatus> connectionStatusChangedHandler;
@@ -259,9 +266,9 @@ Task HandleException(Exception ex)
                 {
                     this.connectionStatusChangedHandler(this.clientId, CloudConnectionStatus.DisconnectedTokenExpired);
                 }
-                else if (ex is NullReferenceException)
+                else if (NonRecoverableExceptions.Any(nre => nre.IsInstanceOfType(ex)))
                 {
-                    Events.HandleNre(this);
+                    Events.HandleNre(ex, this);
                     return this.CloseAsync();
                 }
             }
@@ -556,9 +563,9 @@ public static void ErrorGettingTwin(CloudProxy cloudProxy, Exception ex)
                 Log.LogDebug((int)EventIds.ErrorGettingTwin, ex, Invariant($"Error getting twin for {cloudProxy.clientId} in cloud proxy {cloudProxy.id}"));
             }
 
-            public static void HandleNre(CloudProxy cloudProxy)
+            public static void HandleNre(Exception ex, CloudProxy cloudProxy)
             {
-                Log.LogDebug((int)EventIds.HandleNre, Invariant($"Got a NullReferenceException from client for {cloudProxy.clientId}. Closing the cloud proxy since it may be in a bad state."));
+                Log.LogDebug((int)EventIds.HandleNre, ex, Invariant($"Got a non-recoverable error from client for {cloudProxy.clientId}. Closing the cloud proxy since it may be in a bad state."));
             }
 
             internal static void ExceptionInHandleException(CloudProxy cloudProxy, Exception handlingException, Exception caughtException)
diff --git a/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.CloudProxy.Test/CloudProxyTest.cs b/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.CloudProxy.Test/CloudProxyTest.cs
index 665a2ada416..24a5c26e680 100644
--- a/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.CloudProxy.Test/CloudProxyTest.cs
+++ b/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.CloudProxy.Test/CloudProxyTest.cs
@@ -191,8 +191,10 @@ public async Task TestCloseThrows()
             client.VerifyAll();
         }
 
-        [Fact]
-        public async Task TestHandlNre()
+        [Theory]
+        [InlineData(typeof(NullReferenceException))]
+        [InlineData(typeof(ObjectDisposedException))]
+        public async Task TestHandleNonRecoverableExceptions(Type exceptionType)
         {
             // Arrange
             var messageConverter = Mock.Of<IMessageConverter<Message>>(m => m.FromMessage(It.IsAny<IMessage>()) == new Message());
@@ -202,13 +204,13 @@ public async Task TestHandlNre()
             TimeSpan idleTimeout = TimeSpan.FromSeconds(60);
             Action<string, CloudConnectionStatus> connectionStatusChangedHandler = (s, status) => { };
             var client = new Mock<IClient>(MockBehavior.Strict);
-            client.Setup(c => c.SendEventAsync(It.IsAny<Message>())).ThrowsAsync(new NullReferenceException());
+            client.Setup(c => c.SendEventAsync(It.IsAny<Message>())).ThrowsAsync((Exception)Activator.CreateInstance(exceptionType, "dummy message"));
             client.Setup(c => c.CloseAsync()).Returns(Task.CompletedTask);
             var cloudProxy = new CloudProxy(client.Object, messageConverterProvider, clientId, connectionStatusChangedHandler, cloudListener, idleTimeout, false);
             IMessage message = new EdgeMessage.Builder(new byte[0]).Build();
 
             // Act
-            await Assert.ThrowsAsync<NullReferenceException>(() => cloudProxy.SendMessageAsync(message));
+            await Assert.ThrowsAsync(exceptionType, () => cloudProxy.SendMessageAsync(message));
 
             // Assert.
             client.VerifyAll();