Add more validation on RetryOptions (#6702)

Author: pragnagopa

src/WebJobs.Script/CustomAttributeBuilderUtility.cs

@@ -20,14 +20,14 @@ internal static CustomAttributeBuilder GetRetryCustomAttributeBuilder(RetryOptio
                     ConstructorInfo fixedDelayRetryCtorInfo = fixedDelayRetryType.GetConstructor(new[] { typeof(int), typeof(string) });
                     CustomAttributeBuilder fixedDelayRetryBuilder = new CustomAttributeBuilder(
                     fixedDelayRetryCtorInfo,
-                    new object[] { functionRetry.MaxRetryCount, functionRetry.DelayInterval.ToString() });
+                    new object[] { functionRetry.MaxRetryCount.Value, functionRetry.DelayInterval.ToString() });
                     return fixedDelayRetryBuilder;
                 case RetryStrategy.ExponentialBackoff:
                     Type exponentialBackoffRetryType = typeof(ExponentialBackoffRetryAttribute);
                     ConstructorInfo exponentialBackoffDelayRetryCtorInfo = exponentialBackoffRetryType.GetConstructor(new[] { typeof(int), typeof(string), typeof(string) });
                     CustomAttributeBuilder exponentialBackoffRetryBuilder = new CustomAttributeBuilder(
                     exponentialBackoffDelayRetryCtorInfo,
-                    new object[] { functionRetry.MaxRetryCount, functionRetry.MinimumInterval.ToString(), functionRetry.MaximumInterval.ToString() });
+                    new object[] { functionRetry.MaxRetryCount.Value, functionRetry.MinimumInterval.ToString(), functionRetry.MaximumInterval.ToString() });
                     return exponentialBackoffRetryBuilder;
             }
             return null;

src/WebJobs.Script/Utility.cs

@@ -767,27 +767,31 @@ public static void ValidateRetryOptions(RetryOptions
             {
                 return;
             }
+            if (!retryOptions.MaxRetryCount.HasValue)
+            {
+                throw new ArgumentNullException(nameof(retryOptions.MaxRetryCount));
+            }
             switch (retryOptions.Strategy)
             {
                 case RetryStrategy.FixedDelay:
-                    if (retryOptions.DelayInterval.Ticks <= 0)
+                    if (!retryOptions.DelayInterval.HasValue)
                     {
-                        throw new ArgumentOutOfRangeException(nameof(retryOptions.DelayInterval));
+                        throw new ArgumentNullException(nameof(retryOptions.DelayInterval));
                     }
                     // ensure values specified to create FixedDelayRetryAttribute are valid
-                    _ = new FixedDelayRetryAttribute(retryOptions.MaxRetryCount, retryOptions.DelayInterval.ToString());
+                    _ = new FixedDelayRetryAttribute(retryOptions.MaxRetryCount.Value, retryOptions.DelayInterval.ToString());
                     break;
                 case RetryStrategy.ExponentialBackoff:
-                    if (retryOptions.MinimumInterval.Ticks <= 0)
+                    if (!retryOptions.MinimumInterval.HasValue)
                     {
-                        throw new ArgumentOutOfRangeException(nameof(retryOptions.DelayInterval));
+                        throw new ArgumentNullException(nameof(retryOptions.DelayInterval));
                     }
-                    if (retryOptions.MaximumInterval.Ticks <= 0)
+                    if (!retryOptions.MaximumInterval.HasValue)
                     {
-                        throw new ArgumentOutOfRangeException(nameof(retryOptions.DelayInterval));
+                        throw new ArgumentNullException(nameof(retryOptions.DelayInterval));
                     }
                     // ensure values specified to create ExponentialBackoffRetryAttribute are valid
-                    _ = new ExponentialBackoffRetryAttribute(retryOptions.MaxRetryCount, retryOptions.MinimumInterval.ToString(), retryOptions.MaximumInterval.ToString());
+                    _ = new ExponentialBackoffRetryAttribute(retryOptions.MaxRetryCount.Value, retryOptions.MinimumInterval.ToString(), retryOptions.MaximumInterval.ToString());
                     break;
             }
         }

src/WebJobs.Script/WebJobs.Script.csproj

@@ -50,7 +50,7 @@
     <PackageReference Include="Microsoft.Azure.WebJobs.Extensions" Version="4.0.1" />
     <PackageReference Include="Microsoft.Azure.WebJobs.Extensions.Http" Version="3.0.8" />
     <PackageReference Include="Microsoft.Azure.WebJobs.Logging.ApplicationInsights" Version="3.0.22" />
-    <PackageReference Include="Microsoft.Azure.WebJobs.Script.Abstractions" Version="1.0.1-preview" />
+    <PackageReference Include="Microsoft.Azure.WebJobs.Script.Abstractions" Version="1.0.2-preview" />
     <PackageReference Include="Microsoft.Build" Version="15.8.166" />
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Scripting" Version="3.3.1" />
     <PackageReference Include="Microsoft.DotNet.PlatformAbstractions" Version="2.1.0" />

test/WebJobs.Script.Tests/Configuration/ScriptHostOptionsSetupTests.cs

@@ -148,6 +148,9 @@ public void Configure_AppliesTimeout()
         [InlineData("fixedDelay", "3", null, true)]
         [InlineData("fixedDelay", "3", "-10000000000", true)]
         [InlineData("fixedDelay", "3", "10000000000:00000000:40000", true)]
+        [InlineData("fixedDelay", null, "00:00:05", true)]
+        [InlineData("fixedDelay", "-1", "00:00:05", false)]
+        [InlineData("fixedDelay", "-4", "00:00:05", true)]
         public void Configure_AppliesRetry_FixedDelay(string expectedStrategy, string maxRetryCount, string delayInterval, bool throwsError)
         {
             var settings = new Dictionary<string, string>
@@ -156,19 +159,24 @@ public void Configure_AppliesRetry_FixedDelay(string expectedStrategy, string ma
                 { ConfigurationPath.Combine(ConfigurationSectionNames.JobHost, "retry", "maxRetryCount"), maxRetryCount },
                 { ConfigurationPath.Combine(ConfigurationSectionNames.JobHost, "retry", "delayInterval"), delayInterval }
             };
-            if (string.IsNullOrEmpty(delayInterval))
+            if (string.IsNullOrEmpty(delayInterval) || string.IsNullOrEmpty(maxRetryCount))
             {
-                Assert.Throws<ArgumentOutOfRangeException>(() => GetConfiguredOptions(settings));
+                Assert.Throws<ArgumentNullException>(() => GetConfiguredOptions(settings));
                 return;
             }
             if (throwsError)
             {
+                if (int.Parse(maxRetryCount) <= 0)
+                {
+                    Assert.Throws<ArgumentOutOfRangeException>(() => GetConfiguredOptions(settings));
+                    return;
+                }
                 Assert.Throws<InvalidOperationException>(() => GetConfiguredOptions(settings));
                 return;
             }
             var options = GetConfiguredOptions(settings);
             Assert.Equal(RetryStrategy.FixedDelay, options.Retry.Strategy);
-            Assert.Equal(int.Parse(maxRetryCount), options.Retry.MaxRetryCount);
+            Assert.Equal(int.Parse(maxRetryCount), options.Retry.MaxRetryCount.Value);
             Assert.Equal(TimeSpan.Parse(delayInterval), options.Retry.DelayInterval);
         }
 
@@ -189,7 +197,7 @@ public void Configure_AppliesRetry_ExponentialBackOffDelay(string expectedStrate
             };
             if (string.IsNullOrEmpty(minimumInterval) || string.IsNullOrEmpty(maximumInterval))
             {
-                Assert.Throws<ArgumentOutOfRangeException>(() => GetConfiguredOptions(settings));
+                Assert.Throws<ArgumentNullException>(() => GetConfiguredOptions(settings));
                 return;
             }
             var minIntervalTimeSpan = TimeSpan.Parse(minimumInterval);