From 3b812ffb39913d0237049df45a73f963d67346c8 Mon Sep 17 00:00:00 2001 From: "David R. Williamson" Date: Mon, 12 Jul 2021 10:07:53 -0700 Subject: [PATCH 1/5] More codes doc'd, code clean up --- common/src/service/ExceptionHandlingHelper.cs | 169 ++++++++---------- common/src/service/HttpClientHelper.cs | 5 +- .../Common/Exceptions/ServerBusyException.cs | 3 +- .../Common/Exceptions/ServerErrorException.cs | 5 +- iothub/device/src/Edge/TrustBundleProvider.cs | 2 +- .../HttpHsmSignatureProvider.cs | 20 ++- .../src/RetryPolicies/ExponentialBackoff.cs | 5 +- ....cs => ExponentialBackoffRetryStrategy.cs} | 43 +++-- .../src/TransientFaultHandling/RetryPolicy.cs | 2 +- .../TransientFaultHandling/RetryStrategy.cs | 93 +++++----- .../device/tests/ExponentialBackoffTests.cs | 6 +- .../src/Common/Exceptions/ErrorCode.cs | 12 +- .../Common/Exceptions/ServerErrorException.cs | 4 +- iothub/service/src/JobClient/HttpJobClient.cs | 3 +- 14 files changed, 190 insertions(+), 182 deletions(-) rename iothub/device/src/TransientFaultHandling/{ExponentialBackoff.cs => ExponentialBackoffRetryStrategy.cs} (69%) diff --git a/common/src/service/ExceptionHandlingHelper.cs b/common/src/service/ExceptionHandlingHelper.cs index 947659eaad..3ce3c7fb4d 100644 --- a/common/src/service/ExceptionHandlingHelper.cs +++ b/common/src/service/ExceptionHandlingHelper.cs @@ -17,86 +17,77 @@ namespace Microsoft.Azure.Devices { internal class ExceptionHandlingHelper { - public static IDictionary>> GetDefaultErrorMapping() + private static readonly IReadOnlyDictionary>> s_mappings = + new Dictionary>> { - var mappings = new Dictionary>> { - { - HttpStatusCode.NoContent, - async (response) => new DeviceNotFoundException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - }, - - { - HttpStatusCode.NotFound, - async (response) => new DeviceNotFoundException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - }, - - { - HttpStatusCode.Conflict, - async (response) => new DeviceAlreadyExistsException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - }, - - { HttpStatusCode.BadRequest, async (response) => new ArgumentException( - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) }, - - { - HttpStatusCode.Unauthorized, - async (response) => new UnauthorizedException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - }, - - { - HttpStatusCode.Forbidden, - async (response) => new QuotaExceededException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - }, - - { - HttpStatusCode.PreconditionFailed, - async (response) => new DeviceMessageLockLostException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - }, - - { - HttpStatusCode.RequestEntityTooLarge, - async (response) => new MessageTooLargeException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - }, - - { - HttpStatusCode.InternalServerError, - async (response) => new ServerErrorException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - }, - - { - HttpStatusCode.ServiceUnavailable, - async (response) => new ServerBusyException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - }, - - { - (HttpStatusCode)429, - async (response) => new ThrottlingException( - code: await GetExceptionCodeAsync(response).ConfigureAwait(false), - message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) - } - }; + HttpStatusCode.NoContent, + async (response) => new DeviceNotFoundException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + HttpStatusCode.NotFound, + async (response) => new DeviceNotFoundException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + HttpStatusCode.Conflict, + async (response) => new DeviceAlreadyExistsException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + HttpStatusCode.BadRequest, async (response) => new ArgumentException( + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + HttpStatusCode.Unauthorized, + async (response) => new UnauthorizedException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + HttpStatusCode.Forbidden, + async (response) => new QuotaExceededException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + HttpStatusCode.PreconditionFailed, + async (response) => new DeviceMessageLockLostException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + HttpStatusCode.RequestEntityTooLarge, + async (response) => new MessageTooLargeException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + HttpStatusCode.InternalServerError, + async (response) => new ServerErrorException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + HttpStatusCode.ServiceUnavailable, + async (response) => new ServerBusyException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + }, + { + (HttpStatusCode)429, + async (response) => new ThrottlingException( + code: await GetExceptionCodeAsync(response).ConfigureAwait(false), + message: await GetExceptionMessageAsync(response).ConfigureAwait(false)) + } + }; - return mappings; - } + public static IReadOnlyDictionary>> GetDefaultErrorMapping() => + s_mappings; public static Task GetExceptionMessageAsync(HttpResponseMessage response) { @@ -104,10 +95,10 @@ public static Task GetExceptionMessageAsync(HttpResponseMessage response } /// - /// Get the fully qualified error code from the http response message, if exists + /// Get the fully-qualified error code from the HTTP response message, if exists. /// - /// The http response message - /// The fully qualified error code, or the response status code if no error code was provided. + /// The HTTP response message + /// The fully-qualified error code, or the response status code, if no error code was provided. public static async Task GetExceptionCodeAsync(HttpResponseMessage response) { // First we will attempt to retrieve the error code from the response content. @@ -121,22 +112,16 @@ public static async Task GetExceptionCodeAsync(HttpResponseMessage re // to 'error code' enum mapping, the SDK will check if both values are a match. If so, the SDK will populate the exception with the proper Code. In the case where // there is a mismatch between the error code and the description, the SDK returns ErrorCode.InvalidErrorCode and log a warning. - int errorCode; + int errorCodeValue = (int)ErrorCode.InvalidErrorCode; try { - IoTHubExceptionResult responseContent = JsonConvert - .DeserializeObject(responseContentStr); - Dictionary messageFields = JsonConvert - .DeserializeObject>(responseContent.Message); + IoTHubExceptionResult responseContent = JsonConvert.DeserializeObject(responseContentStr); + Dictionary messageFields = JsonConvert.DeserializeObject>(responseContent.Message); if (messageFields != null && messageFields.TryGetValue(CommonConstants.ErrorCode, out string errorCodeObj)) { - errorCode = Convert.ToInt32(errorCodeObj, CultureInfo.InvariantCulture); - } - else - { - return ErrorCode.InvalidErrorCode; + errorCodeValue = Convert.ToInt32(errorCodeObj, CultureInfo.InvariantCulture); } } catch (JsonReaderException ex) @@ -152,7 +137,7 @@ public static async Task GetExceptionCodeAsync(HttpResponseMessage re if (headerErrorCodeString != null && Enum.TryParse(headerErrorCodeString, out ErrorCode headerErrorCode)) { - if ((int)headerErrorCode == errorCode) + if ((int)headerErrorCode == errorCodeValue) { // We have a match. Therefore, return the proper error code. return headerErrorCode; @@ -160,7 +145,7 @@ public static async Task GetExceptionCodeAsync(HttpResponseMessage re if (Logging.IsEnabled) Logging.Error(null, $"There is a mismatch between the error code retrieved from the response content and the response header." + - $"Content error code: {errorCode}. Header error code description: {(int)headerErrorCode}."); + $"Content error code: {errorCodeValue}. Header error code description: {(int)headerErrorCode}."); } return ErrorCode.InvalidErrorCode; diff --git a/common/src/service/HttpClientHelper.cs b/common/src/service/HttpClientHelper.cs index fb17cebcce..a3ac7fccb3 100644 --- a/common/src/service/HttpClientHelper.cs +++ b/common/src/service/HttpClientHelper.cs @@ -45,14 +45,14 @@ internal sealed class HttpClientHelper : IHttpClientHelper public HttpClientHelper( Uri baseAddress, IAuthorizationHeaderProvider authenticationHeaderProvider, - IDictionary>> defaultErrorMapping, + IReadOnlyDictionary>> defaultErrorMapping, TimeSpan timeout, IWebProxy customHttpProxy, int connectionLeaseTimeoutMilliseconds) { _baseAddress = baseAddress; _authenticationHeaderProvider = authenticationHeaderProvider; - _defaultErrorMapping = new ReadOnlyDictionary>>(defaultErrorMapping); + _defaultErrorMapping = defaultErrorMapping; _defaultOperationTimeout = timeout; // We need two types of HttpClients, one with our default operation timeout, and one without. The one without will rely on @@ -924,7 +924,6 @@ internal static HttpMessageHandler CreateDefaultHttpMessageHandler(IWebProxy web #endif #pragma warning restore CA2000 // Dispose objects before losing scope - if (webProxy != DefaultWebProxySettings.Instance) { httpMessageHandler.UseProxy = webProxy != null; diff --git a/iothub/device/src/Common/Exceptions/ServerBusyException.cs b/iothub/device/src/Common/Exceptions/ServerBusyException.cs index cf2324359c..a4b2a2aee5 100644 --- a/iothub/device/src/Common/Exceptions/ServerBusyException.cs +++ b/iothub/device/src/Common/Exceptions/ServerBusyException.cs @@ -10,7 +10,8 @@ namespace Microsoft.Azure.Devices.Client.Exceptions /// The exception that is thrown when the IoT Hub is busy. /// /// - /// This exception typically means the service is unavailable due to high load or an unexpected error and is usually transient. The best course of action is to retry your operation after some time. By default, the SDK will utilize the retry strategy. + /// This exception typically means the service is unavailable due to high load or an unexpected error and is usually transient. + /// The best course of action is to retry your operation after some time. By default, the SDK will utilize the retry strategy. /// [Serializable] public sealed class ServerBusyException : IotHubException diff --git a/iothub/device/src/Common/Exceptions/ServerErrorException.cs b/iothub/device/src/Common/Exceptions/ServerErrorException.cs index 16db31cdc0..02ca731e78 100644 --- a/iothub/device/src/Common/Exceptions/ServerErrorException.cs +++ b/iothub/device/src/Common/Exceptions/ServerErrorException.cs @@ -9,7 +9,10 @@ namespace Microsoft.Azure.Devices.Client.Exceptions /// The exception that is thrown when the IoT Hub returned an internal service error. /// /// - /// This exception typically means the IoT Hub service has encountered an unexpected error and is usually transient. Please review the 500xxx Internal errors guide for more information. The best course of action is to retry your operation after some time. By default, the SDK will utilize the retry strategy. + /// This exception typically means the IoT Hub service has encountered an unexpected error and is usually transient. + /// Please review the 500xxx Internal errors + /// guide for more information. The best course of action is to retry your operation after some time. By default, + /// the SDK will utilize the retry strategy. /// [Serializable] public sealed class ServerErrorException : IotHubException diff --git a/iothub/device/src/Edge/TrustBundleProvider.cs b/iothub/device/src/Edge/TrustBundleProvider.cs index 949b214559..d7bf525a03 100644 --- a/iothub/device/src/Edge/TrustBundleProvider.cs +++ b/iothub/device/src/Edge/TrustBundleProvider.cs @@ -17,7 +17,7 @@ internal class TrustBundleProvider : ITrustBundleProvider private static readonly ITransientErrorDetectionStrategy s_transientErrorDetectionStrategy = new ErrorDetectionStrategy(); private static readonly RetryStrategy s_transientRetryStrategy = - new TransientFaultHandling.ExponentialBackoff( + new ExponentialBackoffRetryStrategy( retryCount: 3, minBackoff: TimeSpan.FromSeconds(2), maxBackoff: TimeSpan.FromSeconds(30), diff --git a/iothub/device/src/HsmAuthentication/HttpHsmSignatureProvider.cs b/iothub/device/src/HsmAuthentication/HttpHsmSignatureProvider.cs index 98d1c2d27b..64ad119b95 100644 --- a/iothub/device/src/HsmAuthentication/HttpHsmSignatureProvider.cs +++ b/iothub/device/src/HsmAuthentication/HttpHsmSignatureProvider.cs @@ -25,8 +25,11 @@ internal class HttpHsmSignatureProvider : ISignatureProvider private static readonly ITransientErrorDetectionStrategy s_transientErrorDetectionStrategy = new ErrorDetectionStrategy(); - private static readonly RetryStrategy s_transientRetryStrategy = - new TransientFaultHandling.ExponentialBackoff(retryCount: 3, minBackoff: TimeSpan.FromSeconds(2), maxBackoff: TimeSpan.FromSeconds(30), deltaBackoff: TimeSpan.FromSeconds(3)); + private static readonly RetryStrategy s_transientRetryStrategy = new ExponentialBackoffRetryStrategy( + retryCount: 3, + minBackoff: TimeSpan.FromSeconds(2), + maxBackoff: TimeSpan.FromSeconds(30), + deltaBackoff: TimeSpan.FromSeconds(3)); public HttpHsmSignatureProvider(string providerUri, string apiVersion) { @@ -69,7 +72,8 @@ public async Task SignAsync(string moduleId, string generationId, string BaseUrl = HttpClientHelper.GetBaseUrl(_providerUri) }; - SignResponse response = await SignAsyncWithRetryAsync(hsmHttpClient, moduleId, generationId, signRequest).ConfigureAwait(false); + SignResponse response = await SignAsyncWithRetryAsync(hsmHttpClient, moduleId, generationId, signRequest) + .ConfigureAwait(false); return Convert.ToBase64String(response.Digest); } @@ -91,10 +95,16 @@ public async Task SignAsync(string moduleId, string generationId, string } } - private async Task SignAsyncWithRetryAsync(HttpHsmClient hsmHttpClient, string moduleId, string generationId, SignRequest signRequest) + private async Task SignAsyncWithRetryAsync( + HttpHsmClient hsmHttpClient, + string moduleId, + string generationId, + SignRequest signRequest) { var transientRetryPolicy = new RetryPolicy(s_transientErrorDetectionStrategy, s_transientRetryStrategy); - SignResponse response = await transientRetryPolicy.ExecuteAsync(() => hsmHttpClient.SignAsync(_apiVersion, moduleId, generationId, signRequest)).ConfigureAwait(false); + SignResponse response = await transientRetryPolicy + .ExecuteAsync(() => hsmHttpClient.SignAsync(_apiVersion, moduleId, generationId, signRequest)) + .ConfigureAwait(false); return response; } diff --git a/iothub/device/src/RetryPolicies/ExponentialBackoff.cs b/iothub/device/src/RetryPolicies/ExponentialBackoff.cs index dd1d7fc333..e46d149962 100644 --- a/iothub/device/src/RetryPolicies/ExponentialBackoff.cs +++ b/iothub/device/src/RetryPolicies/ExponentialBackoff.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using Microsoft.Azure.Devices.Client.TransientFaultHandling; namespace Microsoft.Azure.Devices.Client { @@ -10,7 +11,7 @@ namespace Microsoft.Azure.Devices.Client /// public class ExponentialBackoff : IRetryPolicy { - private readonly TransientFaultHandling.ExponentialBackoff _exponentialBackoffRetryStrategy; + private readonly ExponentialBackoffRetryStrategy _exponentialBackoffRetryStrategy; /// /// Creates an instance of ExponentialBackoff. @@ -22,7 +23,7 @@ public class ExponentialBackoff : IRetryPolicy public ExponentialBackoff(int retryCount, TimeSpan minBackoff, TimeSpan maxBackoff, TimeSpan deltaBackoff) { - _exponentialBackoffRetryStrategy = new TransientFaultHandling.ExponentialBackoff(retryCount, minBackoff, maxBackoff, deltaBackoff); + _exponentialBackoffRetryStrategy = new ExponentialBackoffRetryStrategy(retryCount, minBackoff, maxBackoff, deltaBackoff); } /// diff --git a/iothub/device/src/TransientFaultHandling/ExponentialBackoff.cs b/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs similarity index 69% rename from iothub/device/src/TransientFaultHandling/ExponentialBackoff.cs rename to iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs index 0813e4c20c..5b589183e7 100644 --- a/iothub/device/src/TransientFaultHandling/ExponentialBackoff.cs +++ b/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs @@ -1,31 +1,34 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. //Copyright(c) Microsoft.All rights reserved. //Microsoft would like to thank its contributors, a list //of whom are at http://aka.ms/entlib-contributors using System; -//Licensed under the Apache License, Version 2.0 (the "License"); you -//may not use this file except in compliance with the License. You may -//obtain a copy of the License at +// Licensed under the Apache License, Version 2.0 (the "License"); you +// may not use this file except in compliance with the License. You may +// obtain a copy of the License at //http://www.apache.org/licenses/LICENSE-2.0 -//Unless required by applicable law or agreed to in writing, software -//distributed under the License is distributed on an "AS IS" BASIS, -//WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or -//implied. See the License for the specific language governing permissions -//and limitations under the License. +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing permissions +// and limitations under the License. // THIS FILE HAS BEEN MODIFIED FROM ITS ORIGINAL FORM. // Change Log: // 9/1/2017 jasminel Renamed namespace to Microsoft.Azure.Devices.Client.TransientFaultHandling and modified access modifier to internal. +// 7/12/2021 drwill Renamed class from ExponentialBackoff to ExponentialBackoffRetryStrategy to avoid naming internal conflict. namespace Microsoft.Azure.Devices.Client.TransientFaultHandling { /// /// A retry strategy with back-off parameters for calculating the exponential delay between retries. /// - internal class ExponentialBackoff : RetryStrategy + internal class ExponentialBackoffRetryStrategy : RetryStrategy { private readonly int _retryCount; @@ -36,37 +39,40 @@ internal class ExponentialBackoff : RetryStrategy private readonly TimeSpan _deltaBackoff; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// - public ExponentialBackoff() : this(DefaultClientRetryCount, DefaultMinBackoff, DefaultMaxBackoff, DefaultClientBackoff) + public ExponentialBackoffRetryStrategy() + : this(DefaultClientRetryCount, DefaultMinBackoff, DefaultMaxBackoff, DefaultClientBackoff) { } /// - /// Initializes a new instance of the class with the specified retry settings. + /// Initializes a new instance of the class with the specified retry settings. /// /// The maximum number of retry attempts. /// The minimum back-off time /// The maximum back-off time. /// The value that will be used to calculate a random delta in the exponential delay between retries. - public ExponentialBackoff(int retryCount, TimeSpan minBackoff, TimeSpan maxBackoff, TimeSpan deltaBackoff) : this(null, retryCount, minBackoff, maxBackoff, deltaBackoff, RetryStrategy.DefaultFirstFastRetry) + public ExponentialBackoffRetryStrategy(int retryCount, TimeSpan minBackoff, TimeSpan maxBackoff, TimeSpan deltaBackoff) + : this(null, retryCount, minBackoff, maxBackoff, deltaBackoff, RetryStrategy.DefaultFirstFastRetry) { } /// - /// Initializes a new instance of the class with the specified name and retry settings. + /// Initializes a new instance of the class with the specified name and retry settings. /// /// The name of the retry strategy. /// The maximum number of retry attempts. /// The minimum back-off time /// The maximum back-off time. /// The value that will be used to calculate a random delta in the exponential delay between retries. - public ExponentialBackoff(string name, int retryCount, TimeSpan minBackoff, TimeSpan maxBackoff, TimeSpan deltaBackoff) : this(name, retryCount, minBackoff, maxBackoff, deltaBackoff, RetryStrategy.DefaultFirstFastRetry) + public ExponentialBackoffRetryStrategy(string name, int retryCount, TimeSpan minBackoff, TimeSpan maxBackoff, TimeSpan deltaBackoff) + : this(name, retryCount, minBackoff, maxBackoff, deltaBackoff, RetryStrategy.DefaultFirstFastRetry) { } /// - /// Initializes a new instance of the class with the specified name, retry settings, and fast retry option. + /// Initializes a new instance of the class with the specified name, retry settings, and fast retry option. /// /// The name of the retry strategy. /// The maximum number of retry attempts. @@ -74,7 +80,8 @@ public ExponentialBackoff(string name, int retryCount, TimeSpan minBackoff, Time /// The maximum back-off time. /// The value that will be used to calculate a random delta in the exponential delay between retries. /// true to immediately retry in the first attempt; otherwise, false. The subsequent retries will remain subject to the configured retry interval. - public ExponentialBackoff(string name, int retryCount, TimeSpan minBackoff, TimeSpan maxBackoff, TimeSpan deltaBackoff, bool firstFastRetry) : base(name, firstFastRetry) + public ExponentialBackoffRetryStrategy(string name, int retryCount, TimeSpan minBackoff, TimeSpan maxBackoff, TimeSpan deltaBackoff, bool firstFastRetry) + : base(name, firstFastRetry) { Guard.ArgumentNotNegativeValue(retryCount, "retryCount"); Guard.ArgumentNotNegativeValue(minBackoff.Ticks, "minBackoff"); @@ -97,7 +104,7 @@ public override ShouldRetry GetShouldRetry() { if (currentRetryCount < _retryCount) { - Random random = new Random(); + var random = new Random(); double exponentialInterval = (Math.Pow(2.0, currentRetryCount) - 1.0) diff --git a/iothub/device/src/TransientFaultHandling/RetryPolicy.cs b/iothub/device/src/TransientFaultHandling/RetryPolicy.cs index 5e46d23f65..2131a7cbec 100644 --- a/iothub/device/src/TransientFaultHandling/RetryPolicy.cs +++ b/iothub/device/src/TransientFaultHandling/RetryPolicy.cs @@ -151,7 +151,7 @@ public RetryPolicy( TimeSpan minBackoff, TimeSpan maxBackoff, TimeSpan deltaBackoff) - : this(errorDetectionStrategy, new ExponentialBackoff(retryCount, minBackoff, maxBackoff, deltaBackoff)) + : this(errorDetectionStrategy, new ExponentialBackoffRetryStrategy(retryCount, minBackoff, maxBackoff, deltaBackoff)) { } diff --git a/iothub/device/src/TransientFaultHandling/RetryStrategy.cs b/iothub/device/src/TransientFaultHandling/RetryStrategy.cs index c18c5c4eef..88c9a39300 100644 --- a/iothub/device/src/TransientFaultHandling/RetryStrategy.cs +++ b/iothub/device/src/TransientFaultHandling/RetryStrategy.cs @@ -1,24 +1,27 @@ -//Copyright(c) Microsoft.All rights reserved. -//Microsoft would like to thank its contributors, a list -//of whom are at http://aka.ms/entlib-contributors +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// Copyright(c) Microsoft.All rights reserved. +// Microsoft would like to thank its contributors, a list +// of whom are at http://aka.ms/entlib-contributors using System; -//Licensed under the Apache License, Version 2.0 (the "License"); you -//may not use this file except in compliance with the License. You may -//obtain a copy of the License at +// Licensed under the Apache License, Version 2.0 (the "License"); you +// may not use this file except in compliance with the License. You may +// obtain a copy of the License at //http://www.apache.org/licenses/LICENSE-2.0 -//Unless required by applicable law or agreed to in writing, software -//distributed under the License is distributed on an "AS IS" BASIS, -//WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or -//implied. See the License for the specific language governing permissions -//and limitations under the License. +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing permissions +// and limitations under the License. // THIS FILE HAS BEEN MODIFIED FROM ITS ORIGINAL FORM. // Change Log: // 9/1/2017 jasminel Renamed namespace to Microsoft.Azure.Devices.Client.TransientFaultHandling and modified access modifier to internal. +// 7/12/2021 drwill Changed property+backing field to auto-property. namespace Microsoft.Azure.Devices.Client.TransientFaultHandling { @@ -63,73 +66,57 @@ internal abstract class RetryStrategy /// public const bool DefaultFirstFastRetry = true; - private static readonly RetryStrategy s_noRetry = new FixedInterval(0, DefaultRetryInterval); - - private static readonly RetryStrategy s_defaultFixed = new FixedInterval(DefaultClientRetryCount, DefaultRetryInterval); - - private static readonly RetryStrategy s_defaultProgressive = new Incremental(DefaultClientRetryCount, DefaultRetryInterval, DefaultRetryIncrement); - - private static readonly RetryStrategy s_defaultExponential = new ExponentialBackoff(DefaultClientRetryCount, DefaultMinBackoff, DefaultMaxBackoff, DefaultClientBackoff); + /// + /// Initializes a new instance of the class. + /// + /// The name of the retry strategy. + /// + /// True to immediately retry in the first attempt; otherwise, false. + /// The subsequent retries will remain subject to the configured retry interval. + /// + protected RetryStrategy(string name, bool firstFastRetry) + { + Name = name; + FastFirstRetry = firstFastRetry; + } /// /// Returns a default policy that performs no retries, but invokes the action only once. /// - public static RetryStrategy NoRetry => s_noRetry; + public static RetryStrategy NoRetry { get; } = new FixedInterval(0, DefaultRetryInterval); /// - /// Returns a default policy that implements a fixed retry interval configured with the and parameters. - /// The default retry policy treats all caught exceptions as transient errors. + /// Returns a default policy that implements a fixed retry interval configured with the + /// and parameters. The default retry policy treats all caught exceptions as transient errors. /// - public static RetryStrategy DefaultFixed => s_defaultFixed; + public static RetryStrategy DefaultFixed { get; } = new FixedInterval(DefaultClientRetryCount, DefaultRetryInterval); /// - /// Returns a default policy that implements a progressive retry interval configured with the - /// , - /// , + /// Returns a default policy that implements a progressive retry interval configured with the + /// , + /// , /// and parameters. /// The default retry policy treats all caught exceptions as transient errors. /// - public static RetryStrategy DefaultProgressive => s_defaultProgressive; + public static RetryStrategy DefaultProgressive { get; } = new Incremental(DefaultClientRetryCount, DefaultRetryInterval, DefaultRetryIncrement); /// - /// Returns a default policy that implements a random exponential retry interval configured with the - /// , - /// , - /// , - /// and parameters. + /// Returns a default policy that implements a random exponential retry interval configured with the , + /// , , and parameters. /// The default retry policy treats all caught exceptions as transient errors. /// - public static RetryStrategy DefaultExponential => s_defaultExponential; + public static RetryStrategy DefaultExponential { get; } = new ExponentialBackoffRetryStrategy(DefaultClientRetryCount, DefaultMinBackoff, DefaultMaxBackoff, DefaultClientBackoff); /// /// Gets or sets a value indicating whether the first retry attempt will be made immediately, /// whereas subsequent retries will remain subject to the retry interval. /// - public bool FastFirstRetry - { - get; - set; - } + public bool FastFirstRetry { get; set; } /// /// Gets the name of the retry strategy. /// - public string Name - { - get; - private set; - } - - /// - /// Initializes a new instance of the class. - /// - /// The name of the retry strategy. - /// true to immediately retry in the first attempt; otherwise, false. The subsequent retries will remain subject to the configured retry interval. - protected RetryStrategy(string name, bool firstFastRetry) - { - Name = name; - FastFirstRetry = firstFastRetry; - } + public string Name { get; private set; } /// /// Returns the corresponding ShouldRetry delegate. diff --git a/iothub/device/tests/ExponentialBackoffTests.cs b/iothub/device/tests/ExponentialBackoffTests.cs index 8abb193ef5..9d4d756cea 100644 --- a/iothub/device/tests/ExponentialBackoffTests.cs +++ b/iothub/device/tests/ExponentialBackoffTests.cs @@ -16,7 +16,11 @@ public class ExponentialBackoffTests [TestCategory("Unit")] public void ExponentialBackoffDoesNotUnderflow() { - var exponentialBackoff = new TransientFaultHandling.ExponentialBackoff(MAX_RETRY_ATTEMPTS, RetryStrategy.DefaultMinBackoff, RetryStrategy.DefaultMaxBackoff, RetryStrategy.DefaultClientBackoff); + var exponentialBackoff = new ExponentialBackoffRetryStrategy( + MAX_RETRY_ATTEMPTS, + RetryStrategy.DefaultMinBackoff, + RetryStrategy.DefaultMaxBackoff, + RetryStrategy.DefaultClientBackoff); ShouldRetry shouldRetry = exponentialBackoff.GetShouldRetry(); for (int i = 1; i < MAX_RETRY_ATTEMPTS; i++) { diff --git a/iothub/service/src/Common/Exceptions/ErrorCode.cs b/iothub/service/src/Common/Exceptions/ErrorCode.cs index 2d64bd11e9..507896cd8d 100644 --- a/iothub/service/src/Common/Exceptions/ErrorCode.cs +++ b/iothub/service/src/Common/Exceptions/ErrorCode.cs @@ -7,14 +7,24 @@ namespace Microsoft.Azure.Devices.Common.Exceptions { /// - /// Error Codes for common IoT hub exceptions. + /// Error codes for common IoT hub exceptions. /// public enum ErrorCode { #pragma warning disable CS1591 // Missing XML comment for publicly visible type or member + + /// + /// Used when the error code returned by the hub is unrecognized. If encountered, please report the issue so it can be added here. + /// InvalidErrorCode = 0, // BadRequest - 400 + + /// + /// Unused error code. Service does not return it and neither does the SDK. + /// + [Obsolete("This error does not appear to be returned by the service.")] + [EditorBrowsable(EditorBrowsableState.Never)] InvalidProtocolVersion = 400001, /// diff --git a/iothub/service/src/Common/Exceptions/ServerErrorException.cs b/iothub/service/src/Common/Exceptions/ServerErrorException.cs index 91d678f696..4cf3c31e1f 100644 --- a/iothub/service/src/Common/Exceptions/ServerErrorException.cs +++ b/iothub/service/src/Common/Exceptions/ServerErrorException.cs @@ -10,7 +10,9 @@ namespace Microsoft.Azure.Devices.Common.Exceptions /// The exception that is thrown when the IoT Hub returned an internal service error. /// /// - /// This exception typically means the IoT Hub service has encountered an unexpected error and is usually transient. Please review the 500xxx Internal errors guide for more information. The best course of action is to retry your operation after some time. + /// This exception typically means the IoT Hub service has encountered an unexpected error and is usually transient. + /// Please review the 500xxx Internal errors + /// guide for more information. The best course of action is to retry your operation after some time. /// [Serializable] public sealed class ServerErrorException : IotHubException diff --git a/iothub/service/src/JobClient/HttpJobClient.cs b/iothub/service/src/JobClient/HttpJobClient.cs index 63fe9f3683..88a1d2e0ab 100644 --- a/iothub/service/src/JobClient/HttpJobClient.cs +++ b/iothub/service/src/JobClient/HttpJobClient.cs @@ -8,10 +8,9 @@ using System.Text; using System.Threading; using System.Threading.Tasks; -using Microsoft.Azure.Devices.Shared; using Microsoft.Azure.Devices.Common; using Microsoft.Azure.Devices.Common.Exceptions; -using System.Diagnostics.CodeAnalysis; +using Microsoft.Azure.Devices.Shared; namespace Microsoft.Azure.Devices { From 77a124f9ac7a5c7584ea81f63f00982dce6a1a86 Mon Sep 17 00:00:00 2001 From: "David R. Williamson" Date: Mon, 12 Jul 2021 10:41:08 -0700 Subject: [PATCH 2/5] Update license --- .../ExponentialBackoffRetryStrategy.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs b/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs index 5b589183e7..cbe81da345 100644 --- a/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs +++ b/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs @@ -1,12 +1,11 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -//Copyright(c) Microsoft.All rights reserved. -//Microsoft would like to thank its contributors, a list -//of whom are at http://aka.ms/entlib-contributors + +// Microsoft would like to thank its contributors, a list of whom are at http://aka.ms/entlib-contributors using System; -// Licensed under the Apache License, Version 2.0 (the "License"); you +// Source licensed under the Apache License, Version 2.0 (the "License"); you // may not use this file except in compliance with the License. You may // obtain a copy of the License at From 935471cfd59a4c602862a28fc9a991f57fd49570 Mon Sep 17 00:00:00 2001 From: "David R. Williamson" Date: Mon, 12 Jul 2021 10:43:16 -0700 Subject: [PATCH 3/5] Random is static --- .../ExponentialBackoffRetryStrategy.cs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs b/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs index cbe81da345..cc8d9a14be 100644 --- a/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs +++ b/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs @@ -29,12 +29,11 @@ namespace Microsoft.Azure.Devices.Client.TransientFaultHandling /// internal class ExponentialBackoffRetryStrategy : RetryStrategy { - private readonly int _retryCount; + private static readonly Random s_random = new Random(); + private readonly int _retryCount; private readonly TimeSpan _minBackoff; - - private readonly TimeSpan _maxBackoff; - +| private readonly TimeSpan _maxBackoff; private readonly TimeSpan _deltaBackoff; /// @@ -103,11 +102,8 @@ public override ShouldRetry GetShouldRetry() { if (currentRetryCount < _retryCount) { - var random = new Random(); - - double exponentialInterval = - (Math.Pow(2.0, currentRetryCount) - 1.0) - * random.Next( + double exponentialInterval = (Math.Pow(2.0, currentRetryCount) - 1.0) + * s_random.Next( (int)_deltaBackoff.TotalMilliseconds * 8 / 10, (int)_deltaBackoff.TotalMilliseconds * 12 / 10) + _minBackoff.TotalMilliseconds; From f3c1add5157b4b859b883b9d0506ff34fed5d6fb Mon Sep 17 00:00:00 2001 From: "David R. Williamson" Date: Mon, 12 Jul 2021 10:44:10 -0700 Subject: [PATCH 4/5] License header --- iothub/device/src/TransientFaultHandling/RetryStrategy.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/iothub/device/src/TransientFaultHandling/RetryStrategy.cs b/iothub/device/src/TransientFaultHandling/RetryStrategy.cs index 88c9a39300..a73ef0779e 100644 --- a/iothub/device/src/TransientFaultHandling/RetryStrategy.cs +++ b/iothub/device/src/TransientFaultHandling/RetryStrategy.cs @@ -1,12 +1,11 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -// Copyright(c) Microsoft.All rights reserved. -// Microsoft would like to thank its contributors, a list -// of whom are at http://aka.ms/entlib-contributors + +// Microsoft would like to thank its contributors, a list of whom are at http://aka.ms/entlib-contributors using System; -// Licensed under the Apache License, Version 2.0 (the "License"); you +// Source licensed under the Apache License, Version 2.0 (the "License"); you // may not use this file except in compliance with the License. You may // obtain a copy of the License at From a80c914e7033db6213c708ecb34b9f66681ae924 Mon Sep 17 00:00:00 2001 From: "David R. Williamson" Date: Mon, 12 Jul 2021 10:07:53 -0700 Subject: [PATCH 5/5] More codes doc'd, code clean up --- .../ExponentialBackoffRetryStrategy.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs b/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs index cc8d9a14be..2736744263 100644 --- a/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs +++ b/iothub/device/src/TransientFaultHandling/ExponentialBackoffRetryStrategy.cs @@ -102,8 +102,11 @@ public override ShouldRetry GetShouldRetry() { if (currentRetryCount < _retryCount) { - double exponentialInterval = (Math.Pow(2.0, currentRetryCount) - 1.0) - * s_random.Next( + Random random = new Random(); + + double exponentialInterval = + (Math.Pow(2.0, currentRetryCount) - 1.0) + * random.Next( (int)_deltaBackoff.TotalMilliseconds * 8 / 10, (int)_deltaBackoff.TotalMilliseconds * 12 / 10) + _minBackoff.TotalMilliseconds;