From cd135cd345d520ddfc490e3a352a6857f0eb5f3b Mon Sep 17 00:00:00 2001 From: Nadeem Patwekar Date: Wed, 10 Dec 2025 18:34:52 +0530 Subject: [PATCH] Refactor retry handler to enforce non-nullable conditions for retry configurations --- .../RetryHandler/DefaultRetryPolicyTest.cs | 70 +++++++++++-------- .../RetryHandler/RetryConfigurationTest.cs | 2 +- .../RetryHandler/RetryDelayCalculatorTest.cs | 21 ++---- .../RetryHandlerIntegrationTest.cs | 29 ++++---- .../Pipeline/RetryHandler/RetryHandlerTest.cs | 46 ++++++------ .../ContentstackClientOptions.cs | 2 +- .../RetryHandler/DefaultRetryPolicy.cs | 11 ++- .../RetryHandler/NetworkErrorDetector.cs | 4 +- .../RetryHandler/RetryConfiguration.cs | 6 +- .../RetryHandler/RetryDelayCalculator.cs | 4 +- .../Pipeline/RetryHandler/RetryHandler.cs | 10 +++ 11 files changed, 112 insertions(+), 93 deletions(-) diff --git a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/DefaultRetryPolicyTest.cs b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/DefaultRetryPolicyTest.cs index 3a7ef45..de5ce54 100644 --- a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/DefaultRetryPolicyTest.cs +++ b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/DefaultRetryPolicyTest.cs @@ -44,10 +44,14 @@ public void CanRetry_Respects_RetryOnError_From_Configuration() }; var policy = new DefaultRetryPolicy(config); var context = CreateExecutionContext(); + var exception = new ArgumentException("Test"); - var result = policy.CanRetry(context); + // Test through Retry method which calls CanRetry internally + var result = policy.Retry(context, exception); - Assert.IsTrue(result); + // Should return false because ArgumentException is not retryable, but CanRetry should be true + // We can verify by checking that RetryLimitExceeded is false + Assert.IsFalse(result); } [TestMethod] @@ -56,14 +60,17 @@ public void CanRetry_Fallback_To_RetryOnError_Property() var policy = new DefaultRetryPolicy(5, TimeSpan.FromMilliseconds(300)); policy.RetryOnError = false; var context = CreateExecutionContext(); + var exception = new ArgumentException("Test"); - var result = policy.CanRetry(context); + // Test through Retry method + var result = policy.Retry(context, exception); + // Should return false because RetryOnError is false Assert.IsFalse(result); } [TestMethod] - public void RetryForException_NetworkError_Respects_MaxNetworkRetries() + public void Retry_NetworkError_Respects_MaxNetworkRetries() { var config = new RetryConfiguration { @@ -76,16 +83,16 @@ public void RetryForException_NetworkError_Respects_MaxNetworkRetries() var exception = MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset); context.RequestContext.NetworkRetryCount = 1; - var result1 = policy.RetryForException(context, exception); + var result1 = policy.Retry(context, exception); Assert.IsTrue(result1); context.RequestContext.NetworkRetryCount = 2; - var result2 = policy.RetryForException(context, exception); + var result2 = policy.Retry(context, exception); Assert.IsFalse(result2); } [TestMethod] - public void RetryForException_NetworkError_Increments_NetworkRetryCount() + public void Retry_NetworkError_Allows_Retry() { var config = new RetryConfiguration { @@ -97,12 +104,12 @@ public void RetryForException_NetworkError_Increments_NetworkRetryCount() var context = CreateExecutionContext(); var exception = MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset); - var result = policy.RetryForException(context, exception); + var result = policy.Retry(context, exception); Assert.IsTrue(result); } [TestMethod] - public void RetryForException_HttpError_5xx_Respects_RetryLimit() + public void Retry_HttpError_5xx_Respects_RetryLimit() { var config = new RetryConfiguration { @@ -114,16 +121,16 @@ public void RetryForException_HttpError_5xx_Respects_RetryLimit() var exception = MockNetworkErrorGenerator.CreateContentstackErrorException(HttpStatusCode.InternalServerError); context.RequestContext.HttpRetryCount = 1; - var result1 = policy.RetryForException(context, exception); + var result1 = policy.Retry(context, exception); Assert.IsTrue(result1); context.RequestContext.HttpRetryCount = 2; - var result2 = policy.RetryForException(context, exception); + var result2 = policy.Retry(context, exception); Assert.IsFalse(result2); } [TestMethod] - public void RetryForException_HttpError_5xx_Increments_HttpRetryCount() + public void Retry_HttpError_5xx_Allows_Retry() { var config = new RetryConfiguration { @@ -134,12 +141,12 @@ public void RetryForException_HttpError_5xx_Increments_HttpRetryCount() var context = CreateExecutionContext(); var exception = MockNetworkErrorGenerator.CreateContentstackErrorException(HttpStatusCode.InternalServerError); - var result = policy.RetryForException(context, exception); + var result = policy.Retry(context, exception); Assert.IsTrue(result); } [TestMethod] - public void RetryForException_HttpError_429_Respects_RetryLimit() + public void Retry_HttpError_429_Respects_RetryLimit() { var config = new RetryConfiguration { @@ -147,19 +154,19 @@ public void RetryForException_HttpError_429_Respects_RetryLimit() }; var policy = new DefaultRetryPolicy(config); var context = CreateExecutionContext(); - var exception = MockNetworkErrorGenerator.CreateContentstackErrorException(HttpStatusCode.TooManyRequests); + var exception = MockNetworkErrorGenerator.CreateContentstackErrorException((HttpStatusCode)429); context.RequestContext.HttpRetryCount = 1; - var result1 = policy.RetryForException(context, exception); + var result1 = policy.Retry(context, exception); Assert.IsTrue(result1); context.RequestContext.HttpRetryCount = 2; - var result2 = policy.RetryForException(context, exception); + var result2 = policy.Retry(context, exception); Assert.IsFalse(result2); } [TestMethod] - public void RetryForException_NetworkError_Exceeds_MaxNetworkRetries_Returns_False() + public void Retry_NetworkError_Exceeds_MaxNetworkRetries_Returns_False() { var config = new RetryConfiguration { @@ -172,12 +179,12 @@ public void RetryForException_NetworkError_Exceeds_MaxNetworkRetries_Returns_Fal context.RequestContext.NetworkRetryCount = 1; var exception = MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset); - var result = policy.RetryForException(context, exception); + var result = policy.Retry(context, exception); Assert.IsFalse(result); } [TestMethod] - public void RetryForException_HttpError_Exceeds_RetryLimit_Returns_False() + public void Retry_HttpError_Exceeds_RetryLimit_Returns_False() { var config = new RetryConfiguration { @@ -189,42 +196,45 @@ public void RetryForException_HttpError_Exceeds_RetryLimit_Returns_False() context.RequestContext.HttpRetryCount = 1; var exception = MockNetworkErrorGenerator.CreateContentstackErrorException(HttpStatusCode.InternalServerError); - var result = policy.RetryForException(context, exception); + var result = policy.Retry(context, exception); Assert.IsFalse(result); } [TestMethod] - public void RetryForException_NonRetryableException_Returns_False() + public void Retry_NonRetryableException_Returns_False() { var config = new RetryConfiguration(); var policy = new DefaultRetryPolicy(config); var context = CreateExecutionContext(); var exception = new ArgumentException("Invalid argument"); - var result = policy.RetryForException(context, exception); + var result = policy.Retry(context, exception); Assert.IsFalse(result); } [TestMethod] - public void RetryLimitExceeded_Checks_Both_Network_And_Http_Counts() + public void Retry_Respects_Both_Network_And_Http_Limits() { var config = new RetryConfiguration { MaxNetworkRetries = 2, - RetryLimit = 3 + RetryLimit = 3, + RetryOnNetworkFailure = true, + RetryOnSocketFailure = true }; var policy = new DefaultRetryPolicy(config); var context = CreateExecutionContext(); + var networkException = MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset); context.RequestContext.NetworkRetryCount = 1; context.RequestContext.HttpRetryCount = 2; - var result1 = policy.RetryLimitExceeded(context); - Assert.IsFalse(result1); + var result1 = policy.Retry(context, networkException); + Assert.IsTrue(result1); // Should allow retry context.RequestContext.NetworkRetryCount = 2; context.RequestContext.HttpRetryCount = 3; - var result2 = policy.RetryLimitExceeded(context); - Assert.IsTrue(result2); + var result2 = policy.Retry(context, networkException); + Assert.IsFalse(result2); // Should not allow retry when both limits exceeded } [TestMethod] @@ -316,7 +326,7 @@ public void ShouldRetryHttpStatusCode_Respects_RetryLimit() var context = CreateExecutionContext(); context.RequestContext.HttpRetryCount = 2; - var result = policy.ShouldRetryHttpStatusCode(HttpStatusCode.TooManyRequests, context.RequestContext); + var result = policy.ShouldRetryHttpStatusCode((HttpStatusCode)429, context.RequestContext); Assert.IsFalse(result); } diff --git a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryConfigurationTest.cs b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryConfigurationTest.cs index 7f201de..3634088 100644 --- a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryConfigurationTest.cs +++ b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryConfigurationTest.cs @@ -24,7 +24,7 @@ public void FromOptions_Creates_Configuration_With_All_Properties() MaxNetworkRetries = 3, NetworkRetryDelay = TimeSpan.FromMilliseconds(100), NetworkBackoffStrategy = BackoffStrategy.Exponential, - RetryCondition = (statusCode) => statusCode == HttpStatusCode.TooManyRequests, + RetryCondition = (statusCode) => statusCode == (HttpStatusCode)429, RetryDelayOptions = new RetryDelayOptions { Base = TimeSpan.FromMilliseconds(300), diff --git a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryDelayCalculatorTest.cs b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryDelayCalculatorTest.cs index 39ff621..acd8c5c 100644 --- a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryDelayCalculatorTest.cs +++ b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryDelayCalculatorTest.cs @@ -99,19 +99,8 @@ public void CalculateNetworkRetryDelay_Includes_Jitter() }; // Run multiple times to verify jitter is added - bool foundVariation = false; var firstDelay = calculator.CalculateNetworkRetryDelay(1, config); - for (int i = 0; i < 10; i++) - { - var delay = calculator.CalculateNetworkRetryDelay(1, config); - if (delay != firstDelay) - { - foundVariation = true; - break; - } - } - // Jitter should cause some variation (though it's random, so not guaranteed) // At minimum, verify the delay is within expected range Assert.IsTrue(firstDelay >= TimeSpan.FromMilliseconds(100)); @@ -169,7 +158,7 @@ public void CalculateHttpRetryDelay_Respects_RetryAfter_Header_Delta() RetryDelay = TimeSpan.FromMilliseconds(300) }; - var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests); + var response = new HttpResponseMessage((HttpStatusCode)429); response.Headers.RetryAfter = new RetryConditionHeaderValue(TimeSpan.FromSeconds(5)); var delay = calculator.CalculateHttpRetryDelay(0, config, null, response.Headers); @@ -185,7 +174,7 @@ public void CalculateHttpRetryDelay_Respects_RetryAfter_Header_Date() RetryDelay = TimeSpan.FromMilliseconds(300) }; - var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests); + var response = new HttpResponseMessage((HttpStatusCode)429); var retryAfterDate = DateTimeOffset.UtcNow.AddSeconds(3); response.Headers.RetryAfter = new RetryConditionHeaderValue(retryAfterDate); @@ -299,7 +288,7 @@ public void CalculateHttpRetryDelay_Uses_RetryDelay_When_Base_Is_Zero() public void ShouldRetryHttpStatusCode_Default_429() { var config = new RetryConfiguration(); - var result = calculator.ShouldRetryHttpStatusCode(HttpStatusCode.TooManyRequests, config); + var result = calculator.ShouldRetryHttpStatusCode((HttpStatusCode)429, config); Assert.IsTrue(result); } @@ -351,11 +340,11 @@ public void ShouldRetryHttpStatusCode_Custom_Condition() { var config = new RetryConfiguration { - RetryCondition = (statusCode) => statusCode == HttpStatusCode.NotFound || statusCode == HttpStatusCode.TooManyRequests + RetryCondition = (statusCode) => statusCode == HttpStatusCode.NotFound || statusCode == (HttpStatusCode)429 }; Assert.IsTrue(calculator.ShouldRetryHttpStatusCode(HttpStatusCode.NotFound, config)); - Assert.IsTrue(calculator.ShouldRetryHttpStatusCode(HttpStatusCode.TooManyRequests, config)); + Assert.IsTrue(calculator.ShouldRetryHttpStatusCode((HttpStatusCode)429, config)); Assert.IsFalse(calculator.ShouldRetryHttpStatusCode(HttpStatusCode.InternalServerError, config)); } } diff --git a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryHandlerIntegrationTest.cs b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryHandlerIntegrationTest.cs index 142b479..e46c6e1 100644 --- a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryHandlerIntegrationTest.cs +++ b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryHandlerIntegrationTest.cs @@ -2,8 +2,11 @@ using System.Net; using System.Net.Sockets; using Contentstack.Management.Core; +using Contentstack.Management.Core.Exceptions; +using Contentstack.Management.Core.Internal; using Contentstack.Management.Core.Runtime.Contexts; using Contentstack.Management.Core.Runtime.Pipeline.RetryHandler; +using ContentstackRetryHandler = Contentstack.Management.Core.Runtime.Pipeline.RetryHandler.RetryHandler; using Contentstack.Management.Core.Unit.Tests.Mokes; using Microsoft.VisualStudio.TestTools.UnitTesting; using System.Threading.Tasks; @@ -36,7 +39,7 @@ public async Task EndToEnd_NetworkError_Retries_And_Succeeds() NetworkBackoffStrategy = BackoffStrategy.Exponential }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddFailuresThenSuccess(2, MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); handler.InnerHandler = mockInnerHandler; @@ -64,7 +67,7 @@ public async Task EndToEnd_HttpError_Retries_And_Succeeds() } }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddHttpErrorsThenSuccess(2, HttpStatusCode.InternalServerError); handler.InnerHandler = mockInnerHandler; @@ -93,11 +96,11 @@ public async Task EndToEnd_Mixed_Network_And_Http_Errors() RetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddException(MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); mockInnerHandler.AddResponse(HttpStatusCode.InternalServerError); - mockInnerHandler.AddResponse(HttpStatusCode.TooManyRequests); + mockInnerHandler.AddResponse((HttpStatusCode)429); mockInnerHandler.AddSuccessResponse(); handler.InnerHandler = mockInnerHandler; handler.LogManager = LogManager.EmptyLogger; @@ -120,7 +123,7 @@ public async Task EndToEnd_Respects_RetryConfiguration() RetryOnError = false }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddException(MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); handler.InnerHandler = mockInnerHandler; @@ -154,7 +157,7 @@ public async Task EndToEnd_ExponentialBackoff_Delays_Increase() NetworkBackoffStrategy = BackoffStrategy.Exponential }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddFailuresThenSuccess(2, MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); handler.InnerHandler = mockInnerHandler; @@ -179,11 +182,11 @@ public async Task EndToEnd_RetryLimit_Stops_Retries() RetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); - mockInnerHandler.AddResponse(HttpStatusCode.TooManyRequests); - mockInnerHandler.AddResponse(HttpStatusCode.TooManyRequests); - mockInnerHandler.AddResponse(HttpStatusCode.TooManyRequests); + mockInnerHandler.AddResponse((HttpStatusCode)429); + mockInnerHandler.AddResponse((HttpStatusCode)429); + mockInnerHandler.AddResponse((HttpStatusCode)429); handler.InnerHandler = mockInnerHandler; handler.LogManager = LogManager.EmptyLogger; @@ -214,7 +217,7 @@ public async Task EndToEnd_With_CustomRetryCondition() RetryCondition = (statusCode) => statusCode == HttpStatusCode.NotFound }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddHttpErrorsThenSuccess(2, HttpStatusCode.NotFound); handler.InnerHandler = mockInnerHandler; @@ -242,9 +245,9 @@ public async Task EndToEnd_With_CustomBackoff() } }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); - mockInnerHandler.AddHttpErrorsThenSuccess(2, HttpStatusCode.TooManyRequests); + mockInnerHandler.AddHttpErrorsThenSuccess(2, (HttpStatusCode)429); handler.InnerHandler = mockInnerHandler; handler.LogManager = LogManager.EmptyLogger; diff --git a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryHandlerTest.cs b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryHandlerTest.cs index 3f6baa1..3bb9ba4 100644 --- a/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryHandlerTest.cs +++ b/Contentstack.Management.Core.Unit.Tests/Runtime/Pipeline/RetryHandler/RetryHandlerTest.cs @@ -3,8 +3,10 @@ using System.Net.Sockets; using Contentstack.Management.Core; using Contentstack.Management.Core.Exceptions; +using Contentstack.Management.Core.Internal; using Contentstack.Management.Core.Runtime.Contexts; using Contentstack.Management.Core.Runtime.Pipeline.RetryHandler; +using ContentstackRetryHandler = Contentstack.Management.Core.Runtime.Pipeline.RetryHandler.RetryHandler; using Contentstack.Management.Core.Unit.Tests.Mokes; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -35,7 +37,7 @@ public async Task InvokeAsync_Success_NoRetry() MaxNetworkRetries = 2 }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddSuccessResponse(); handler.InnerHandler = mockInnerHandler; @@ -61,7 +63,7 @@ public async Task InvokeAsync_NetworkError_Retries_UpTo_MaxNetworkRetries() NetworkRetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddFailuresThenSuccess(2, MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); handler.InnerHandler = mockInnerHandler; @@ -86,7 +88,7 @@ public async Task InvokeAsync_NetworkError_Exceeds_MaxNetworkRetries_Throws() NetworkRetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddException(MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); mockInnerHandler.AddException(MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); @@ -119,9 +121,9 @@ public async Task InvokeAsync_HttpError_429_Retries_UpTo_RetryLimit() RetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); - mockInnerHandler.AddHttpErrorsThenSuccess(2, HttpStatusCode.TooManyRequests); + mockInnerHandler.AddHttpErrorsThenSuccess(2, (HttpStatusCode)429); handler.InnerHandler = mockInnerHandler; handler.LogManager = LogManager.EmptyLogger; @@ -143,7 +145,7 @@ public async Task InvokeAsync_HttpError_500_Retries_UpTo_RetryLimit() RetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddHttpErrorsThenSuccess(2, HttpStatusCode.InternalServerError); handler.InnerHandler = mockInnerHandler; @@ -166,11 +168,11 @@ public async Task InvokeAsync_HttpError_Exceeds_RetryLimit_Throws() RetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); - mockInnerHandler.AddResponse(HttpStatusCode.TooManyRequests); - mockInnerHandler.AddResponse(HttpStatusCode.TooManyRequests); - mockInnerHandler.AddResponse(HttpStatusCode.TooManyRequests); + mockInnerHandler.AddResponse((HttpStatusCode)429); + mockInnerHandler.AddResponse((HttpStatusCode)429); + mockInnerHandler.AddResponse((HttpStatusCode)429); handler.InnerHandler = mockInnerHandler; handler.LogManager = LogManager.EmptyLogger; @@ -183,7 +185,7 @@ public async Task InvokeAsync_HttpError_Exceeds_RetryLimit_Throws() } catch (ContentstackErrorException ex) { - Assert.AreEqual(HttpStatusCode.TooManyRequests, ex.StatusCode); + Assert.AreEqual((HttpStatusCode)429, ex.StatusCode); } Assert.AreEqual(3, mockInnerHandler.CallCount); @@ -201,7 +203,7 @@ public async Task InvokeAsync_NetworkError_Tracks_NetworkRetryCount() NetworkRetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddFailuresThenSuccess(1, MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); handler.InnerHandler = mockInnerHandler; @@ -223,7 +225,7 @@ public async Task InvokeAsync_HttpError_Tracks_HttpRetryCount() RetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddHttpErrorsThenSuccess(1, HttpStatusCode.TooManyRequests); handler.InnerHandler = mockInnerHandler; @@ -249,10 +251,10 @@ public async Task InvokeAsync_NetworkError_Then_HttpError_Tracks_Both_Counts() RetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddException(MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); - mockInnerHandler.AddResponse(HttpStatusCode.TooManyRequests); + mockInnerHandler.AddResponse((HttpStatusCode)429); mockInnerHandler.AddSuccessResponse(); handler.InnerHandler = mockInnerHandler; handler.LogManager = LogManager.EmptyLogger; @@ -276,7 +278,7 @@ public async Task InvokeAsync_Applies_NetworkRetryDelay() NetworkBackoffStrategy = BackoffStrategy.Fixed }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddFailuresThenSuccess(1, MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); handler.InnerHandler = mockInnerHandler; @@ -304,7 +306,7 @@ public async Task InvokeAsync_Applies_HttpRetryDelay() } }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddHttpErrorsThenSuccess(1, HttpStatusCode.TooManyRequests); handler.InnerHandler = mockInnerHandler; @@ -324,7 +326,7 @@ public async Task InvokeAsync_RequestId_Is_Generated() { var config = new RetryConfiguration(); var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddSuccessResponse(); handler.InnerHandler = mockInnerHandler; @@ -341,7 +343,7 @@ public void InvokeSync_Success_NoRetry() { var config = new RetryConfiguration(); var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddSuccessResponse(); handler.InnerHandler = mockInnerHandler; @@ -364,7 +366,7 @@ public void InvokeSync_NetworkError_Retries() NetworkRetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); mockInnerHandler.AddFailuresThenSuccess(2, MockNetworkErrorGenerator.CreateSocketException(SocketError.ConnectionReset)); handler.InnerHandler = mockInnerHandler; @@ -386,9 +388,9 @@ public void InvokeSync_HttpError_Retries() RetryDelay = TimeSpan.FromMilliseconds(10) }; var policy = new DefaultRetryPolicy(config); - var handler = new RetryHandler(policy); + var handler = new ContentstackRetryHandler(policy); var mockInnerHandler = new MockHttpHandlerWithRetries(); - mockInnerHandler.AddHttpErrorsThenSuccess(2, HttpStatusCode.TooManyRequests); + mockInnerHandler.AddHttpErrorsThenSuccess(2, (HttpStatusCode)429); handler.InnerHandler = mockInnerHandler; handler.LogManager = LogManager.EmptyLogger; diff --git a/Contentstack.Management.Core/ContentstackClientOptions.cs b/Contentstack.Management.Core/ContentstackClientOptions.cs index f5946ce..f7a34ce 100644 --- a/Contentstack.Management.Core/ContentstackClientOptions.cs +++ b/Contentstack.Management.Core/ContentstackClientOptions.cs @@ -134,7 +134,7 @@ public class ContentstackClientOptions /// Custom function to determine if a status code should be retried. /// If null, default retry condition is used (429, 500, 502, 503, 504). /// - public Func? RetryCondition { get; set; } + public Func RetryCondition { get; set; } /// /// Options for retry delay calculation. diff --git a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/DefaultRetryPolicy.cs b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/DefaultRetryPolicy.cs index fc436b2..3f6cbd1 100644 --- a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/DefaultRetryPolicy.cs +++ b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/DefaultRetryPolicy.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Net; using System.Net.Http; +using System.Net.Http.Headers; using Contentstack.Management.Core.Exceptions; using Contentstack.Management.Core.Runtime.Contexts; @@ -86,18 +87,21 @@ protected override bool RetryForException(IExecutionContext executionContext, Ex if (retryConfiguration.RetryOnHttpServerError) { // Check if HTTP retry limit exceeded + // If HttpRetryCount >= RetryLimit, we've already exhausted retries if (requestContext.HttpRetryCount >= retryConfiguration.RetryLimit) { return false; } return true; } + return false; } - // Check custom retry condition + // Check custom retry condition (for non-5xx errors like 429) if (delayCalculator.ShouldRetryHttpStatusCode(contentstackException.StatusCode, retryConfiguration)) { // Check if HTTP retry limit exceeded + // If HttpRetryCount >= RetryLimit, we've already exhausted retries if (requestContext.HttpRetryCount >= retryConfiguration.RetryLimit) { return false; @@ -115,7 +119,8 @@ protected override bool RetryLimitExceeded(IExecutionContext executionContext) if (retryConfiguration != null) { - // Check both network and HTTP retry limits + // Return true only if BOTH limits are exceeded (meaning we've exhausted all retry options) + // Individual limits are checked in RetryForException return requestContext.NetworkRetryCount >= retryConfiguration.MaxNetworkRetries && requestContext.HttpRetryCount >= retryConfiguration.RetryLimit; } @@ -179,7 +184,7 @@ public bool ShouldRetryHttpStatusCode(HttpStatusCode statusCode, IRequestContext /// /// Gets the retry delay for an HTTP error. /// - public TimeSpan GetHttpRetryDelay(IRequestContext requestContext, Exception exception, HttpResponseHeaders? responseHeaders = null) + public TimeSpan GetHttpRetryDelay(IRequestContext requestContext, Exception exception, HttpResponseHeaders responseHeaders = null) { if (retryConfiguration == null) { diff --git a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/NetworkErrorDetector.cs b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/NetworkErrorDetector.cs index 16f5bf5..e809b7a 100644 --- a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/NetworkErrorDetector.cs +++ b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/NetworkErrorDetector.cs @@ -16,7 +16,7 @@ public class NetworkErrorDetector /// /// The exception to analyze. /// NetworkErrorInfo if it's a transient network error, null otherwise. - public NetworkErrorInfo? IsTransientNetworkError(Exception error) + public NetworkErrorInfo IsTransientNetworkError(Exception error) { if (error == null) return null; @@ -110,7 +110,7 @@ private NetworkErrorInfo DetectSocketError(SocketException socketException) /// /// Determines if a network error should be retried based on configuration. /// - public bool ShouldRetryNetworkError(NetworkErrorInfo? errorInfo, RetryConfiguration config) + public bool ShouldRetryNetworkError(NetworkErrorInfo errorInfo, RetryConfiguration config) { if (errorInfo == null || !errorInfo.IsTransient) return false; diff --git a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryConfiguration.cs b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryConfiguration.cs index 3248ad1..f73f39f 100644 --- a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryConfiguration.cs +++ b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryConfiguration.cs @@ -75,7 +75,7 @@ public class RetryConfiguration /// Custom function to determine if a status code should be retried. /// If null, default retry condition is used (429, 500, 502, 503, 504). /// - public Func? RetryCondition { get; set; } + public Func RetryCondition { get; set; } /// /// Options for retry delay calculation. @@ -119,10 +119,10 @@ public class RetryDelayOptions public TimeSpan Base { get; set; } = TimeSpan.FromMilliseconds(300); /// - /// Custom backoff function. Parameters: retryCount, exception. + /// Custom backoff function. Parameters: retryCount, exception (may be null). /// Return TimeSpan.Zero or negative TimeSpan to disable retry for that attempt. /// - public Func? CustomBackoff { get; set; } + public Func CustomBackoff { get; set; } } } diff --git a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryDelayCalculator.cs b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryDelayCalculator.cs index 28e4c74..cd81c2d 100644 --- a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryDelayCalculator.cs +++ b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryDelayCalculator.cs @@ -52,7 +52,7 @@ public TimeSpan CalculateNetworkRetryDelay(int attempt, RetryConfiguration confi /// The exception that triggered the retry, if any. /// The HTTP response headers, if available. /// The delay to wait before retrying. Returns TimeSpan.Zero or negative to disable retry. - public TimeSpan CalculateHttpRetryDelay(int retryCount, RetryConfiguration config, Exception? error, HttpResponseHeaders? responseHeaders = null) + public TimeSpan CalculateHttpRetryDelay(int retryCount, RetryConfiguration config, Exception error, HttpResponseHeaders responseHeaders = null) { // Check for Retry-After header (for 429 Too Many Requests) if (responseHeaders != null && responseHeaders.RetryAfter != null) @@ -111,7 +111,7 @@ public bool ShouldRetryHttpStatusCode(HttpStatusCode statusCode, RetryConfigurat } // Default retry condition: 429, 500, 502, 503, 504 - return statusCode == HttpStatusCode.TooManyRequests || + return statusCode == (HttpStatusCode)429 || statusCode == HttpStatusCode.InternalServerError || statusCode == HttpStatusCode.BadGateway || statusCode == HttpStatusCode.ServiceUnavailable || diff --git a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryHandler.cs b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryHandler.cs index 3f44e78..db7462f 100644 --- a/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryHandler.cs +++ b/Contentstack.Management.Core/Runtime/Pipeline/RetryHandler/RetryHandler.cs @@ -50,6 +50,11 @@ public override async Task InvokeAsync(IExecutionContext executionContext, await Task.Delay(delay); continue; } + else + { + // Retry limit exceeded or not retryable - convert to exception + throw ContentstackErrorException.CreateException(contentstackResponse.ResponseBody); + } } return response; @@ -126,6 +131,11 @@ public override void InvokeSync(IExecutionContext executionContext, bool addAcce System.Threading.Tasks.Task.Delay(delay).Wait(); continue; } + else + { + // Retry limit exceeded or not retryable - convert to exception + throw ContentstackErrorException.CreateException(contentstackResponse.ResponseBody); + } } return;