From 0f5adec5674a980e7601fd9a464dbe8b6e3b9312 Mon Sep 17 00:00:00 2001 From: Joshua Ryder Date: Thu, 12 Feb 2026 20:11:59 +0100 Subject: [PATCH] fix bug --- .claude/settings.local.json | 4 +- src/MinimalWorker.Generators/WorkerEmitter.cs | 29 ++- .../BackgroundWorkerExtensions.cs | 16 +- test/MinimalWorker.Test/RetryTests.cs | 235 +++++++++++++++++- 4 files changed, 263 insertions(+), 21 deletions(-) diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 1d24ebb..1cf4386 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -4,7 +4,9 @@ "Bash(dotnet test:*)", "Bash(dotnet build:*)", "Bash(ls:*)", - "Bash(dotnet publish:*)" + "Bash(dotnet publish:*)", + "Bash(dotnet clean:*)", + "Bash(dotnet restore:*)" ] } } diff --git a/src/MinimalWorker.Generators/WorkerEmitter.cs b/src/MinimalWorker.Generators/WorkerEmitter.cs index e2114e2..69fd679 100644 --- a/src/MinimalWorker.Generators/WorkerEmitter.cs +++ b/src/MinimalWorker.Generators/WorkerEmitter.cs @@ -399,10 +399,11 @@ private static void EmitWorkerExtension(StringBuilder sb, List p.Type.Replace("global::", "").Replace(", ", ","))); - var signature = $"{worker.Type}:{paramTypes}"; + var returnType = worker.ReturnType.Replace("global::", "").Replace(", ", ","); + var signature = $"{worker.Type}:{paramTypes}:{returnType}"; if (!workerMap.ContainsKey(signature)) { @@ -567,12 +568,18 @@ private static void EmitContinuousWorkerInit(StringBuilder sb, WorkerInvocationM sb.AppendLine(" succeeded = true; // Mark as success to avoid error handling"); sb.AppendLine(" break;"); sb.AppendLine(" }"); - sb.AppendLine(" catch (OperationCanceledException) when (timeout.HasValue)"); + sb.AppendLine(" catch (OperationCanceledException) when (timeout.HasValue && timeoutCts?.Token.IsCancellationRequested == true && !token.IsCancellationRequested)"); sb.AppendLine(" {"); sb.AppendLine(" // Timeout - don't retry timeouts"); sb.AppendLine(" lastException = new TimeoutException($\"Worker '{workerName}' execution timed out after {timeout.Value}.\");"); sb.AppendLine(" break;"); sb.AppendLine(" }"); + sb.AppendLine(" catch (TimeoutException tex) when (timeout.HasValue)"); + sb.AppendLine(" {"); + sb.AppendLine(" // User-thrown TimeoutException - don't retry when timeout is configured"); + sb.AppendLine(" lastException = tex;"); + sb.AppendLine(" break;"); + sb.AppendLine(" }"); sb.AppendLine(" catch (Exception ex)"); sb.AppendLine(" {"); sb.AppendLine(" lastException = ex;"); @@ -703,12 +710,18 @@ private static void EmitPeriodicWorkerInit(StringBuilder sb, WorkerInvocationMod sb.AppendLine(" succeeded = true; // Mark as success to avoid error handling"); sb.AppendLine(" break;"); sb.AppendLine(" }"); - sb.AppendLine(" catch (OperationCanceledException) when (timeout.HasValue)"); + sb.AppendLine(" catch (OperationCanceledException) when (timeout.HasValue && timeoutCts?.Token.IsCancellationRequested == true && !token.IsCancellationRequested)"); sb.AppendLine(" {"); sb.AppendLine(" // Timeout - don't retry timeouts"); sb.AppendLine(" lastException = new TimeoutException($\"Worker '{workerName}' execution timed out after {timeout.Value}.\");"); sb.AppendLine(" break;"); sb.AppendLine(" }"); + sb.AppendLine(" catch (TimeoutException tex) when (timeout.HasValue)"); + sb.AppendLine(" {"); + sb.AppendLine(" // User-thrown TimeoutException - don't retry when timeout is configured"); + sb.AppendLine(" lastException = tex;"); + sb.AppendLine(" break;"); + sb.AppendLine(" }"); sb.AppendLine(" catch (Exception ex)"); sb.AppendLine(" {"); sb.AppendLine(" lastException = ex;"); @@ -865,12 +878,18 @@ private static void EmitCronWorkerInit(StringBuilder sb, WorkerInvocationModel w sb.AppendLine(" succeeded = true; // Mark as success to avoid error handling"); sb.AppendLine(" break;"); sb.AppendLine(" }"); - sb.AppendLine(" catch (OperationCanceledException) when (timeout.HasValue)"); + sb.AppendLine(" catch (OperationCanceledException) when (timeout.HasValue && timeoutCts?.Token.IsCancellationRequested == true && !token.IsCancellationRequested)"); sb.AppendLine(" {"); sb.AppendLine(" // Timeout - don't retry timeouts"); sb.AppendLine(" lastException = new TimeoutException($\"Worker '{workerName}' execution timed out after {timeout.Value}.\");"); sb.AppendLine(" break;"); sb.AppendLine(" }"); + sb.AppendLine(" catch (TimeoutException tex) when (timeout.HasValue)"); + sb.AppendLine(" {"); + sb.AppendLine(" // User-thrown TimeoutException - don't retry when timeout is configured"); + sb.AppendLine(" lastException = tex;"); + sb.AppendLine(" break;"); + sb.AppendLine(" }"); sb.AppendLine(" catch (Exception ex)"); sb.AppendLine(" {"); sb.AppendLine(" lastException = ex;"); diff --git a/src/MinimalWorker/BackgroundWorkerExtensions.cs b/src/MinimalWorker/BackgroundWorkerExtensions.cs index 92725bb..3958a7b 100644 --- a/src/MinimalWorker/BackgroundWorkerExtensions.cs +++ b/src/MinimalWorker/BackgroundWorkerExtensions.cs @@ -275,6 +275,7 @@ internal static void ClearRegistrations() private static string FormatTypeName(Type type) { // Map common types to their C# keyword equivalents + if (type == typeof(void)) return "void"; if (type == typeof(string)) return "string"; if (type == typeof(int)) return "int"; if (type == typeof(long)) return "long"; @@ -415,7 +416,8 @@ public static IWorkerBuilder RunBackgroundWorker(this IHost host, Delegate actio { var id = System.Threading.Interlocked.Increment(ref _registrationCounter); var parameters = action.Method.GetParameters(); - var signature = string.Join(",", parameters.Select(p => FormatTypeName(p.ParameterType))); + var paramSignature = string.Join(",", parameters.Select(p => FormatTypeName(p.ParameterType))); + var returnType = FormatTypeName(action.Method.ReturnType); var registration = new WorkerRegistration { @@ -425,7 +427,7 @@ public static IWorkerBuilder RunBackgroundWorker(this IHost host, Delegate actio Type = WorkerType.Continuous, Host = host, ParameterCount = parameters.Length, - Signature = $"{WorkerType.Continuous}:{signature}", + Signature = $"{WorkerType.Continuous}:{paramSignature}:{returnType}", OnError = null }; @@ -478,7 +480,8 @@ public static IWorkerBuilder RunPeriodicBackgroundWorker(this IHost host, TimeSp var id = System.Threading.Interlocked.Increment(ref _registrationCounter); var parameters = action.Method.GetParameters(); - var signature = string.Join(",", parameters.Select(p => FormatTypeName(p.ParameterType))); + var paramSignature = string.Join(",", parameters.Select(p => FormatTypeName(p.ParameterType))); + var returnType = FormatTypeName(action.Method.ReturnType); var registration = new WorkerRegistration { @@ -489,7 +492,7 @@ public static IWorkerBuilder RunPeriodicBackgroundWorker(this IHost host, TimeSp Schedule = timespan, Host = host, ParameterCount = parameters.Length, - Signature = $"{WorkerType.Periodic}:{signature}", + Signature = $"{WorkerType.Periodic}:{paramSignature}:{returnType}", OnError = null }; @@ -556,7 +559,8 @@ public static IWorkerBuilder RunCronBackgroundWorker(this IHost host, string cro var id = System.Threading.Interlocked.Increment(ref _registrationCounter); var parameters = action.Method.GetParameters(); - var signature = string.Join(",", parameters.Select(p => FormatTypeName(p.ParameterType))); + var paramSignature = string.Join(",", parameters.Select(p => FormatTypeName(p.ParameterType))); + var returnType = FormatTypeName(action.Method.ReturnType); var registration = new WorkerRegistration { @@ -567,7 +571,7 @@ public static IWorkerBuilder RunCronBackgroundWorker(this IHost host, string cro Schedule = cronExpression, Host = host, ParameterCount = parameters.Length, - Signature = $"{WorkerType.Cron}:{signature}", + Signature = $"{WorkerType.Cron}:{paramSignature}:{returnType}", OnError = null }; diff --git a/test/MinimalWorker.Test/RetryTests.cs b/test/MinimalWorker.Test/RetryTests.cs index 839f509..14418bd 100644 --- a/test/MinimalWorker.Test/RetryTests.cs +++ b/test/MinimalWorker.Test/RetryTests.cs @@ -72,9 +72,6 @@ public async Task PeriodicWorker_WithRetry_Should_Call_ErrorHandler_After_All_Re { Interlocked.Increment(ref attemptCount); throw new InvalidOperationException("Always fails"); -#pragma warning disable CS0162 // Unreachable code detected - return Task.CompletedTask; -#pragma warning restore CS0162 }) .WithRetry(maxAttempts: 3, delay: TimeSpan.FromSeconds(10)) .WithErrorHandler(ex => @@ -155,9 +152,6 @@ public async Task PeriodicWorker_WithRetry_Should_Use_Configured_Attempts() { Interlocked.Increment(ref attemptCount); throw new InvalidOperationException("Always fails"); -#pragma warning disable CS0162 // Unreachable code detected - return Task.CompletedTask; -#pragma warning restore CS0162 }) .WithRetry(maxAttempts: 3, delay: TimeSpan.FromSeconds(10)) .WithErrorHandler(ex => @@ -283,9 +277,6 @@ public async Task ContinuousWorker_WithRetry_Should_Call_ErrorHandler_After_All_ { Interlocked.Increment(ref attemptCount); throw new InvalidOperationException("Always fails"); -#pragma warning disable CS0162 // Unreachable code detected - return Task.CompletedTask; -#pragma warning restore CS0162 }) .WithRetry(maxAttempts: 2, delay: TimeSpan.FromSeconds(10)) .WithErrorHandler(ex => @@ -344,4 +335,230 @@ public async Task PeriodicWorker_WithTimeoutAndRetry_Should_Not_Retry_Timeouts() // Assert - Timeout should not be retried (only 1 attempt per timeout) Assert.True(caughtException is TimeoutException, $"Should have caught TimeoutException, got {caughtException?.GetType().Name}"); } + + [Fact] + public async Task PeriodicWorker_UserThrownOperationCanceledException_Should_Be_Retried_When_Not_Timeout() + { + // Arrange + BackgroundWorkerExtensions.ClearRegistrations(); + var attemptCount = 0; + var errorHandlerCalled = false; + var timeProvider = WorkerTestHelper.CreateTimeProvider(); + + using var host = Host.CreateDefaultBuilder() + .ConfigureServices(services => + { + services.AddSingleton(timeProvider); + }) + .Build(); + + host.RunPeriodicBackgroundWorker( + TimeSpan.FromMinutes(1), + async (CancellationToken ct) => + { + Interlocked.Increment(ref attemptCount); + if (attemptCount < 3) + { + // User code throws OperationCanceledException for its own reasons (not due to timeout) + throw new OperationCanceledException("User-initiated cancellation"); + } + await Task.CompletedTask; + }) + .WithTimeout(TimeSpan.FromMinutes(10)) // Long timeout that won't trigger + .WithRetry(maxAttempts: 3, delay: TimeSpan.FromSeconds(5)) + .WithErrorHandler(ex => + { + errorHandlerCalled = true; + }); + + // Act + await host.StartAsync(); + // Advance time: 1 min for periodic tick, then enough for retries + await WorkerTestHelper.AdvanceTimeAsync(timeProvider, TimeSpan.FromMinutes(2), steps: 24); + await host.StopAsync(); + + // Assert - User OperationCanceledException should be retried (not treated as timeout) + Assert.True(attemptCount >= 3, $"Expected at least 3 attempts (OCE should be retried), got {attemptCount}"); + Assert.False(errorHandlerCalled, "Error handler should not be called when retry succeeds"); + } + + [Fact] + public async Task PeriodicWorker_UserThrownTimeoutException_Should_Not_Be_Retried_When_Timeout_Configured() + { + // Arrange + BackgroundWorkerExtensions.ClearRegistrations(); + var attemptCount = 0; + Exception? caughtException = null; + var timeProvider = WorkerTestHelper.CreateTimeProvider(); + + using var host = Host.CreateDefaultBuilder() + .ConfigureServices(services => + { + services.AddSingleton(timeProvider); + }) + .Build(); + + host.RunPeriodicBackgroundWorker( + TimeSpan.FromMinutes(1), + (CancellationToken ct) => + { + Interlocked.Increment(ref attemptCount); + // User code throws TimeoutException directly (not from framework timeout) + throw new TimeoutException("User code detected timeout condition"); + }) + .WithTimeout(TimeSpan.FromMinutes(10)) // Long timeout that won't trigger + .WithRetry(maxAttempts: 5, delay: TimeSpan.FromSeconds(5)) + .WithErrorHandler(ex => + { + caughtException = ex; + }); + + // Act + await host.StartAsync(); + await WorkerTestHelper.AdvanceTimeAsync(timeProvider, TimeSpan.FromMinutes(2), steps: 24); + await host.StopAsync(); + + // Assert - User TimeoutException should NOT be retried when timeout is configured + Assert.Equal(1, attemptCount); // Only 1 attempt, no retries + Assert.NotNull(caughtException); + Assert.IsType(caughtException); + Assert.Equal("User code detected timeout condition", caughtException.Message); + } + + [Fact] + public async Task CronWorker_UserThrownOperationCanceledException_Should_Be_Retried_When_Not_Timeout() + { + // Arrange + BackgroundWorkerExtensions.ClearRegistrations(); + var attemptCount = 0; + var errorHandlerCalled = false; + var timeProvider = WorkerTestHelper.CreateTimeProvider(); + + using var host = Host.CreateDefaultBuilder() + .ConfigureServices(services => + { + services.AddSingleton(timeProvider); + }) + .Build(); + + // Every minute + host.RunCronBackgroundWorker( + "* * * * *", + async (CancellationToken ct) => + { + Interlocked.Increment(ref attemptCount); + if (attemptCount < 2) + { + // User code throws OperationCanceledException for its own reasons + throw new OperationCanceledException("User-initiated cancellation"); + } + await Task.CompletedTask; + }) + .WithTimeout(TimeSpan.FromMinutes(10)) + .WithRetry(maxAttempts: 3, delay: TimeSpan.FromSeconds(5)) + .WithErrorHandler(ex => + { + errorHandlerCalled = true; + }); + + // Act + await host.StartAsync(); + await WorkerTestHelper.AdvanceTimeAsync(timeProvider, TimeSpan.FromMinutes(2), steps: 24); + await host.StopAsync(); + + // Assert - User OperationCanceledException should be retried + Assert.True(attemptCount >= 2, $"Expected at least 2 attempts, got {attemptCount}"); + Assert.False(errorHandlerCalled, "Error handler should not be called when retry succeeds"); + } + + [Fact] + public async Task ContinuousWorker_UserThrownOperationCanceledException_Should_Be_Retried_When_Not_Timeout() + { + // Arrange + BackgroundWorkerExtensions.ClearRegistrations(); + var attemptCount = 0; + var errorHandlerCalled = false; + var timeProvider = WorkerTestHelper.CreateTimeProvider(); + + using var host = Host.CreateDefaultBuilder() + .ConfigureServices(services => + { + services.AddSingleton(timeProvider); + }) + .Build(); + + host.RunBackgroundWorker(async (CancellationToken ct) => + { + attemptCount++; + if (attemptCount < 3) + { + // User code throws OperationCanceledException for its own reasons + throw new OperationCanceledException("User-initiated cancellation"); + } + await Task.CompletedTask; + }) + .WithTimeout(TimeSpan.FromMinutes(10)) + .WithRetry(maxAttempts: 3, delay: TimeSpan.FromSeconds(5)) + .WithErrorHandler(ex => + { + errorHandlerCalled = true; + }); + + // Act + await host.StartAsync(); + await WorkerTestHelper.AdvanceTimeAsync(timeProvider, TimeSpan.FromMinutes(1), steps: 12); + await host.StopAsync(); + + // Assert - User OperationCanceledException should be retried + Assert.True(attemptCount >= 3, $"Expected at least 3 attempts, got {attemptCount}"); + Assert.False(errorHandlerCalled, "Error handler should not be called when retry succeeds"); + } + + [Fact] + public async Task PeriodicWorker_WithRetry_Should_Respect_Delay_Between_Attempts() + { + // Arrange + BackgroundWorkerExtensions.ClearRegistrations(); + var attemptTimestamps = new List(); + var timeProvider = WorkerTestHelper.CreateTimeProvider(); + + using var host = Host.CreateDefaultBuilder() + .ConfigureServices(services => + { + services.AddSingleton(timeProvider); + }) + .Build(); + + host.RunPeriodicBackgroundWorker( + TimeSpan.FromMinutes(1), + async (CancellationToken ct) => + { + attemptTimestamps.Add(timeProvider.GetUtcNow()); + if (attemptTimestamps.Count < 3) + { + throw new InvalidOperationException("Simulated failure"); + } + await Task.CompletedTask; + }) + .WithRetry(maxAttempts: 3, delay: TimeSpan.FromSeconds(30)) + .WithErrorHandler(ex => { }); + + // Act + await host.StartAsync(); + // Advance time: 1 min for periodic tick, then enough for retries with delays + await WorkerTestHelper.AdvanceTimeAsync(timeProvider, TimeSpan.FromMinutes(3), steps: 36); + await host.StopAsync(); + + // Assert - Verify delay between retry attempts + Assert.True(attemptTimestamps.Count >= 3, $"Expected at least 3 attempts, got {attemptTimestamps.Count}"); + + // Check that there was a delay between attempts + for (int i = 1; i < Math.Min(attemptTimestamps.Count, 3); i++) + { + var gap = attemptTimestamps[i] - attemptTimestamps[i - 1]; + // The gap should be at least the retry delay (30 seconds) + Assert.True(gap >= TimeSpan.FromSeconds(25), + $"Expected at least 25s between attempts {i-1} and {i}, got {gap}"); + } + } }