Update twin property value size limit (#171)

* Update twin property value size * Add exception handling around twin * Update property length test
Azure · Aug 17, 2018 · 2590d7e · 2590d7e
1 parent dfffc98
commit 2590d7e
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 13 deletions.
diff --git a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/AmqpTwinMessageConverter.cs b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/AmqpTwinMessageConverter.cs
@@ -20,6 +20,12 @@ public AmqpMessage FromMessage(IMessage message)
             {
                 amqpMessage.Properties.CorrelationId = correlationId;
             }
+
+            if (message.SystemProperties.TryGetNonEmptyValue(SystemProperties.StatusCode, out string statusCode)
+                && int.TryParse(statusCode, out int status))
+            {
+                amqpMessage.MessageAnnotations.Map["status"] = status;
+            }
             return amqpMessage;
         }
 

diff --git a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/TwinManager.cs b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/TwinManager.cs
@@ -21,7 +21,7 @@ namespace Microsoft.Azure.Devices.Edge.Hub.Core
     public class TwinManager : ITwinManager
     {
         const int TwinPropertyMaxDepth = 5; // taken from IoTHub
-        const int TwinPropertyNameMaxLength = 512; // bytes. taken from IoTHub
+        const int TwinPropertyValueMaxLength = 4096; // bytes. taken from IoTHub
         const long TwinPropertyMaxSafeValue = 4503599627370495; // (2^52) - 1. taken from IoTHub
         const long TwinPropertyMinSafeValue = -4503599627370496; // -2^52. taken from IoTHub
         const int TwinPropertyDocMaxLength = 8 * 1024; // 8K bytes. taken from IoTHub
@@ -501,10 +501,10 @@ Task SendReportedPropertiesToCloudProxy(string id, IMessage reported)
 
         static void ValidatePropertyNameAndLength(string name)
         {
-            if (name != null && Encoding.UTF8.GetByteCount(name) > TwinPropertyNameMaxLength)
+            if (name != null && Encoding.UTF8.GetByteCount(name) > TwinPropertyValueMaxLength)
             {
                 string truncated = name.Substring(0, 10);
-                throw new InvalidOperationException($"Length of property name {truncated}.. exceeds maximum length of {TwinPropertyNameMaxLength}");
+                throw new InvalidOperationException($"Length of property name {truncated}.. exceeds maximum length of {TwinPropertyValueMaxLength}");
             }
 
             // Disabling Possible Null Referece, since name is being tested above.
@@ -524,9 +524,9 @@ static void ValidatePropertyNameAndLength(string name)
 
         static void ValidatePropertyValueLength(string name, string value)
         {
-            if (value != null && Encoding.UTF8.GetByteCount(value) > TwinPropertyNameMaxLength)
+            if (value != null && Encoding.UTF8.GetByteCount(value) > TwinPropertyValueMaxLength)
             {
-                throw new InvalidOperationException($"Value associated with property name {name} exceeds maximum length of {TwinPropertyNameMaxLength}");
+                throw new InvalidOperationException($"Value associated with property name {name} exceeds maximum length of {TwinPropertyValueMaxLength}");
             }
         }
 

diff --git a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/device/DeviceMessageHandler.cs b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/device/DeviceMessageHandler.cs
@@ -122,10 +122,18 @@ public async Task ProcessMessageFeedbackAsync(string messageId, FeedbackStatus f
 
         public async Task SendGetTwinRequest(string correlationId)
         {
-            IMessage twin = await this.edgeHub.GetTwinAsync(this.Identity.Id);
-            twin.SystemProperties[SystemProperties.CorrelationId] = correlationId;
-            twin.SystemProperties[SystemProperties.StatusCode] = ((int)HttpStatusCode.OK).ToString();
-            await this.SendTwinUpdate(twin);
+            try
+            {
+                IMessage twin = await this.edgeHub.GetTwinAsync(this.Identity.Id);
+                twin.SystemProperties[SystemProperties.CorrelationId] = correlationId;
+                twin.SystemProperties[SystemProperties.StatusCode] = ((int)HttpStatusCode.OK).ToString();
+                await this.SendTwinUpdate(twin);
+            }
+            catch (Exception e)
+            {
+                Events.ErrorGettingTwin(this.Identity, e);
+                await this.HandleTwinOperationException(correlationId, e);
+            }
         }
 
         public Task ProcessDeviceMessageAsync(IMessage message) => this.edgeHub.ProcessDeviceMessage(this.Identity, message);
@@ -148,16 +156,45 @@ public async Task UpdateReportedPropertiesAsync(IMessage reportedPropertiesMessa
                     reportedPropertiesMessage.SystemProperties[SystemProperties.ConnectionDeviceId] = deviceIdentity.DeviceId;
                     break;
             }
-            await this.edgeHub.UpdateReportedPropertiesAsync(this.Identity, reportedPropertiesMessage);
 
+            try
+            {
+                await this.edgeHub.UpdateReportedPropertiesAsync(this.Identity, reportedPropertiesMessage);
+
+                if (!string.IsNullOrWhiteSpace(correlationId))
+                {
+                    IMessage responseMessage = new EdgeMessage.Builder(new byte[0])
+                        .SetSystemProperties(new Dictionary<string, string>
+                        {
+                            [SystemProperties.CorrelationId] = correlationId,
+                            [SystemProperties.EnqueuedTime] = DateTime.UtcNow.ToString("o"),
+                            [SystemProperties.StatusCode] = ((int)HttpStatusCode.NoContent).ToString()
+                        })
+                        .Build();
+                    await this.SendTwinUpdate(responseMessage);
+                }
+            }
+            catch (Exception e)
+            {
+                Events.ErrorUpdatingReportedPropertiesTwin(this.Identity, e);
+                await this.HandleTwinOperationException(correlationId, e);
+            }
+        }
+
+        async Task HandleTwinOperationException(string correlationId, Exception e)
+        {
             if (!string.IsNullOrWhiteSpace(correlationId))
             {
+                int statusCode = e is InvalidOperationException || e is ArgumentException
+                    ? (int)HttpStatusCode.BadRequest
+                    : (int)HttpStatusCode.InternalServerError;
+
                 IMessage responseMessage = new EdgeMessage.Builder(new byte[0])
                     .SetSystemProperties(new Dictionary<string, string>
                     {
                         [SystemProperties.CorrelationId] = correlationId,
                         [SystemProperties.EnqueuedTime] = DateTime.UtcNow.ToString("o"),
-                        [SystemProperties.StatusCode] = ((int)HttpStatusCode.NoContent).ToString()
+                        [SystemProperties.StatusCode] = statusCode.ToString()
                     })
                     .Build();
                 await this.SendTwinUpdate(responseMessage);
@@ -255,7 +292,9 @@ enum EventIds
                 MessageFeedbackTimedout,
                 MessageFeedbackReceived,
                 MessageFeedbackWithNoMessageId,
-                MessageSentToClient
+                MessageSentToClient,
+                ErrorGettingTwin,
+                ErrorUpdatingReportedProperties
             }
 
             public static void BindDeviceProxy(IIdentity identity)
@@ -312,6 +351,16 @@ public static void SendingMessage(IIdentity identity, string lockToken)
             {
                 Log.LogDebug((int)EventIds.MessageSentToClient, Invariant($"Sent message with correlation ID {lockToken} to {identity.Id}"));
             }
+
+            public static void ErrorGettingTwin(IIdentity identity, Exception ex)
+            {
+                Log.LogWarning((int)EventIds.ErrorGettingTwin, ex, Invariant($"Error getting twin for {identity.Id}"));
+            }
+
+            public static void ErrorUpdatingReportedPropertiesTwin(IIdentity identity, Exception ex)
+            {
+                Log.LogWarning((int)EventIds.ErrorUpdatingReportedProperties, ex, Invariant($"Error updating reported properties for {identity.Id}"));
+            }
         }
     }
 }
diff --git a/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Core.Test/TwinManagerTest.cs b/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Core.Test/TwinManagerTest.cs
@@ -1221,7 +1221,7 @@ public async void ConnectionReestablishedDoesNotSyncReportedPropertiesWhenEmptyS
         [Fact]
         public void ValidateTwinPropertiesSuccess()
         {
-            string tooLong = Enumerable.Repeat("A", 520).Aggregate((sum, next) => sum + next);
+            string tooLong = Enumerable.Repeat("A", 5000).Aggregate((sum, next) => sum + next);
             var reported = new Dictionary<string, string>
             {
                 [tooLong] = "wrong"