diff --git a/.claude/settings.local.json b/.claude/settings.local.json index ae255b5..1d24ebb 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -2,7 +2,9 @@ "permissions": { "allow": [ "Bash(dotnet test:*)", - "Bash(dotnet build:*)" + "Bash(dotnet build:*)", + "Bash(ls:*)", + "Bash(dotnet publish:*)" ] } } diff --git a/src/MinimalWorker.Generators/InvocationModel.cs b/src/MinimalWorker.Generators/InvocationModel.cs index 5c64b20..bdb43bf 100644 --- a/src/MinimalWorker.Generators/InvocationModel.cs +++ b/src/MinimalWorker.Generators/InvocationModel.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using Microsoft.CodeAnalysis; namespace MinimalWorker.Generators; @@ -15,6 +16,26 @@ internal sealed class WorkerInvocationModel public string ReturnType { get; set; } = "Task"; public WorkerType Type { get; set; } public string? ScheduleArgument { get; set; } // TimeSpan or cron expression + + /// + /// Diagnostics collected during analysis. Reported during code generation. + /// + public List Diagnostics { get; set; } = new(); + + /// + /// Indicates whether the model has errors that prevent code generation. + /// + public bool HasErrors { get; set; } +} + +/// +/// Represents a diagnostic to be reported during code generation. +/// +internal sealed class DiagnosticInfo +{ + public DiagnosticDescriptor Descriptor { get; set; } = null!; + public Location Location { get; set; } = Location.None; + public object?[]? MessageArgs { get; set; } } internal sealed class ParameterModel diff --git a/src/MinimalWorker.Generators/WorkerEmitter.cs b/src/MinimalWorker.Generators/WorkerEmitter.cs index 84ef1c6..9bd4ce6 100644 --- a/src/MinimalWorker.Generators/WorkerEmitter.cs +++ b/src/MinimalWorker.Generators/WorkerEmitter.cs @@ -396,6 +396,18 @@ private static void EmitContinuousWorkerInit(StringBuilder sb, WorkerInvocationM sb.AppendLine(); sb.AppendLine(" // Start worker immediately - host has already started"); sb.AppendLine(" var token = lifetime.ApplicationStopping;"); + sb.AppendLine(" var workerId = registration.Id.ToString();"); + sb.AppendLine(" var workerName = registration.DisplayName;"); + sb.AppendLine(); + sb.AppendLine(" // Cache service lookups at initialization (outside Task.Run)"); + sb.AppendLine(" var workerLogger = host.Services.GetService()?.CreateLogger($\"MinimalWorker.{workerName}\");"); + sb.AppendLine(); + sb.AppendLine(" // Register worker for status tracking"); + sb.AppendLine(" MinimalWorkerObservability.RegisterWorker(workerId, workerName, \"continuous\");"); + sb.AppendLine(); + sb.AppendLine(" // Log worker started"); + sb.AppendLine(" if (workerLogger != null) WorkerLogMessages.WorkerStarted(workerLogger, workerName, \"continuous\", workerId, null);"); + sb.AppendLine(); sb.AppendLine(" _ = Task.Run(async () =>"); sb.AppendLine(" {"); sb.AppendLine(" using var scope = host.Services.CreateScope();"); @@ -413,18 +425,6 @@ private static void EmitContinuousWorkerInit(StringBuilder sb, WorkerInvocationM } } - sb.AppendLine(); - sb.AppendLine(" var workerId = registration.Id.ToString();"); - sb.AppendLine(" var workerName = registration.DisplayName;"); - sb.AppendLine(); - sb.AppendLine(" // Get logger for structured logging"); - sb.AppendLine(" var workerLogger = scope.ServiceProvider.GetService()?.CreateLogger($\"MinimalWorker.{workerName}\");"); - sb.AppendLine(); - sb.AppendLine(" // Register worker for status tracking"); - sb.AppendLine(" MinimalWorkerObservability.RegisterWorker(workerId, workerName, \"continuous\");"); - sb.AppendLine(); - sb.AppendLine(" // Log worker started"); - sb.AppendLine(" if (workerLogger != null) WorkerLogMessages.WorkerStarted(workerLogger, workerName, \"continuous\", workerId, null);"); sb.AppendLine(); sb.AppendLine(" var tags = new TagList"); sb.AppendLine(" {"); @@ -472,13 +472,9 @@ private static void EmitContinuousWorkerInit(StringBuilder sb, WorkerInvocationM sb.AppendLine(" activity?.SetStatus(ActivityStatusCode.Error, ex.Message);"); sb.AppendLine(" activity?.RecordException(ex);"); sb.AppendLine(); - sb.AppendLine(" var errorTags = new TagList"); - sb.AppendLine(" {"); - sb.AppendLine(" { \"worker.id\", workerId },"); - sb.AppendLine(" { \"worker.name\", workerName },"); - sb.AppendLine(" { \"worker.type\", \"continuous\" },"); - sb.AppendLine(" { \"exception.type\", ex.GetType().FullName }"); - sb.AppendLine(" };"); + sb.AppendLine(" // Copy base tags struct and add exception type (avoids full allocation in catch)"); + sb.AppendLine(" var errorTags = tags;"); + sb.AppendLine(" errorTags.Add(\"exception.type\", ex.GetType().FullName);"); sb.AppendLine(" MinimalWorkerObservability.ErrorCounter.Add(1, errorTags);"); sb.AppendLine(" MinimalWorkerObservability.RecordFailure(workerId);"); sb.AppendLine(); @@ -533,8 +529,9 @@ private static void EmitPeriodicWorkerInit(StringBuilder sb, WorkerInvocationMod sb.AppendLine(" var workerName = registration.DisplayName;"); sb.AppendLine(" var token = lifetime.ApplicationStopping;"); sb.AppendLine(); - sb.AppendLine(" // Get logger for structured logging"); + sb.AppendLine(" // Cache service lookups at initialization (outside Task.Run)"); sb.AppendLine(" var workerLogger = host.Services.GetService()?.CreateLogger($\"MinimalWorker.{workerName}\");"); + sb.AppendLine(" var timeProvider = host.Services.GetService() ?? TimeProvider.System;"); sb.AppendLine(); sb.AppendLine(" // Register worker for status tracking"); sb.AppendLine(" MinimalWorkerObservability.RegisterWorker(workerId, workerName, \"periodic\");"); @@ -553,9 +550,6 @@ private static void EmitPeriodicWorkerInit(StringBuilder sb, WorkerInvocationMod sb.AppendLine(" };"); sb.AppendLine(" var scheduleString = schedule.ToString();"); sb.AppendLine(); - sb.AppendLine(" // Resolve TimeProvider from DI, fallback to system time"); - sb.AppendLine(" var timeProvider = host.Services.GetService() ?? TimeProvider.System;"); - sb.AppendLine(); sb.AppendLine(" try"); sb.AppendLine(" {"); sb.AppendLine(" using var timer = new PeriodicTimer(schedule, timeProvider);"); @@ -618,13 +612,9 @@ private static void EmitPeriodicWorkerInit(StringBuilder sb, WorkerInvocationMod sb.AppendLine(" activity?.SetStatus(ActivityStatusCode.Error, ex.Message);"); sb.AppendLine(" activity?.RecordException(ex);"); sb.AppendLine(); - sb.AppendLine(" var errorTags = new TagList"); - sb.AppendLine(" {"); - sb.AppendLine(" { \"worker.id\", workerId },"); - sb.AppendLine(" { \"worker.name\", workerName },"); - sb.AppendLine(" { \"worker.type\", \"periodic\" },"); - sb.AppendLine(" { \"exception.type\", ex.GetType().FullName }"); - sb.AppendLine(" };"); + sb.AppendLine(" // Copy base tags struct and add exception type (avoids full allocation in catch)"); + sb.AppendLine(" var errorTags = tags;"); + sb.AppendLine(" errorTags.Add(\"exception.type\", ex.GetType().FullName);"); sb.AppendLine(" MinimalWorkerObservability.ErrorCounter.Add(1, errorTags);"); sb.AppendLine(" MinimalWorkerObservability.RecordFailure(workerId);"); sb.AppendLine(); @@ -689,8 +679,9 @@ private static void EmitCronWorkerInit(StringBuilder sb, WorkerInvocationModel w sb.AppendLine(" var workerName = registration.DisplayName;"); sb.AppendLine(" var token = lifetime.ApplicationStopping;"); sb.AppendLine(); - sb.AppendLine(" // Get logger for structured logging"); + sb.AppendLine(" // Cache service lookups at initialization (outside Task.Run)"); sb.AppendLine(" var workerLogger = host.Services.GetService()?.CreateLogger($\"MinimalWorker.{workerName}\");"); + sb.AppendLine(" var timeProvider = host.Services.GetService() ?? TimeProvider.System;"); sb.AppendLine(); sb.AppendLine(" // Register worker for status tracking"); sb.AppendLine(" MinimalWorkerObservability.RegisterWorker(workerId, workerName, \"cron\");"); @@ -708,9 +699,6 @@ private static void EmitCronWorkerInit(StringBuilder sb, WorkerInvocationModel w sb.AppendLine(" { \"worker.type\", \"cron\" }"); sb.AppendLine(" };"); sb.AppendLine(); - sb.AppendLine(" // Resolve TimeProvider from DI, fallback to system time"); - sb.AppendLine(" var timeProvider = host.Services.GetService() ?? TimeProvider.System;"); - sb.AppendLine(); sb.AppendLine(" try"); sb.AppendLine(" {"); sb.AppendLine(" while (!token.IsCancellationRequested)"); @@ -785,13 +773,9 @@ private static void EmitCronWorkerInit(StringBuilder sb, WorkerInvocationModel w sb.AppendLine(" activity?.SetStatus(ActivityStatusCode.Error, ex.Message);"); sb.AppendLine(" activity?.RecordException(ex);"); sb.AppendLine(); - sb.AppendLine(" var errorTags = new TagList"); - sb.AppendLine(" {"); - sb.AppendLine(" { \"worker.id\", workerId },"); - sb.AppendLine(" { \"worker.name\", workerName },"); - sb.AppendLine(" { \"worker.type\", \"cron\" },"); - sb.AppendLine(" { \"exception.type\", ex.GetType().FullName }"); - sb.AppendLine(" };"); + sb.AppendLine(" // Copy base tags struct and add exception type (avoids full allocation in catch)"); + sb.AppendLine(" var errorTags = tags;"); + sb.AppendLine(" errorTags.Add(\"exception.type\", ex.GetType().FullName);"); sb.AppendLine(" MinimalWorkerObservability.ErrorCounter.Add(1, errorTags);"); sb.AppendLine(" MinimalWorkerObservability.RecordFailure(workerId);"); sb.AppendLine(); diff --git a/src/MinimalWorker.Generators/WorkerGenerator.cs b/src/MinimalWorker.Generators/WorkerGenerator.cs index 6802e59..b1a5aa5 100644 --- a/src/MinimalWorker.Generators/WorkerGenerator.cs +++ b/src/MinimalWorker.Generators/WorkerGenerator.cs @@ -139,10 +139,11 @@ private static bool IsWorkerInvocation(SyntaxNode node) private static bool AnalyzeDelegate(GeneratorSyntaxContext context, ExpressionSyntax delegateExpression, WorkerInvocationModel model) { var typeInfo = context.SemanticModel.GetTypeInfo(delegateExpression); - + var location = delegateExpression.GetLocation(); + // Try to get the actual delegate type, not just the converted type INamedTypeSymbol? delegateType = null; - + if (typeInfo.Type is INamedTypeSymbol namedType && namedType.DelegateInvokeMethod != null) { delegateType = namedType; @@ -151,7 +152,7 @@ private static bool AnalyzeDelegate(GeneratorSyntaxContext context, ExpressionSy { delegateType = convertedNamedType; } - + if (delegateType == null) return false; @@ -167,7 +168,7 @@ private static bool AnalyzeDelegate(GeneratorSyntaxContext context, ExpressionSy { Name = param.Name, Type = param.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat), - IsCancellationToken = param.Type.Name == "CancellationToken" && + IsCancellationToken = param.Type.Name == "CancellationToken" && param.Type.ContainingNamespace?.ToDisplayString() == "System.Threading" }; @@ -177,20 +178,34 @@ private static bool AnalyzeDelegate(GeneratorSyntaxContext context, ExpressionSy model.Parameters.Add(paramModel); } - // Validate only one CancellationToken + // Validate only one CancellationToken - report MW0001 if (cancellationTokenCount > 1) + { + model.Diagnostics.Add(new DiagnosticInfo + { + Descriptor = Diagnostics.MultipleCancellationTokens, + Location = location + }); + model.HasErrors = true; return false; + } // Determine return type var returnType = invokeMethod.ReturnType; model.IsAsync = returnType.Name == "Task" || returnType.Name == "ValueTask"; model.ReturnType = returnType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); - // Ensure Task or void for now (simplification) + // Ensure Task or void for now (simplification) - report MW0002 if (model.ReturnType != "void" && !model.ReturnType.Contains("Task") && !model.ReturnType.Contains("ValueTask")) { + model.Diagnostics.Add(new DiagnosticInfo + { + Descriptor = Diagnostics.InvalidReturnType, + Location = location + }); + model.HasErrors = true; return false; } @@ -207,6 +222,21 @@ private static bool AnalyzeDelegate(GeneratorSyntaxContext context, ExpressionSy private static void Execute(SourceProductionContext context, ImmutableArray workers) { + // Report all collected diagnostics first + foreach (var worker in workers) + { + if (worker?.Diagnostics != null) + { + foreach (var diag in worker.Diagnostics) + { + context.ReportDiagnostic(Diagnostic.Create( + diag.Descriptor, + diag.Location, + diag.MessageArgs ?? System.Array.Empty())); + } + } + } + // Always generate a marker file to confirm generator is running var markerSource = $@"// // MinimalWorker Source Generator @@ -231,7 +261,8 @@ internal static class GeneratorMarker return; } - var validWorkers = workers.Where(w => w != null).ToList(); + // Filter out workers with errors - they should not have code generated + var validWorkers = workers.Where(w => w != null && !w.HasErrors).ToList(); if (validWorkers.Count == 0) return; diff --git a/src/MinimalWorker/BackgroundWorkerExtensions.cs b/src/MinimalWorker/BackgroundWorkerExtensions.cs index 5e04614..aefaf8b 100644 --- a/src/MinimalWorker/BackgroundWorkerExtensions.cs +++ b/src/MinimalWorker/BackgroundWorkerExtensions.cs @@ -290,6 +290,9 @@ public static IWorkerBuilder RunBackgroundWorker(this IHost host, Delegate actio /// public static IWorkerBuilder RunPeriodicBackgroundWorker(this IHost host, TimeSpan timespan, Delegate action) { + if (timespan <= TimeSpan.Zero) + throw new ArgumentOutOfRangeException(nameof(timespan), timespan, "TimeSpan must be greater than zero."); + var id = System.Threading.Interlocked.Increment(ref _registrationCounter); var parameters = action.Method.GetParameters(); var signature = string.Join(",", parameters.Select(p => FormatTypeName(p.ParameterType))); @@ -343,6 +346,9 @@ public static IWorkerBuilder RunPeriodicBackgroundWorker(this IHost host, TimeSp /// public static IWorkerBuilder RunCronBackgroundWorker(this IHost host, string cronExpression, Delegate action) { + if (string.IsNullOrWhiteSpace(cronExpression)) + throw new ArgumentException("Cron expression cannot be null or empty.", nameof(cronExpression)); + var id = System.Threading.Interlocked.Increment(ref _registrationCounter); var parameters = action.Method.GetParameters(); var signature = string.Join(",", parameters.Select(p => FormatTypeName(p.ParameterType))); diff --git a/test/MinimalWorker.Test/CronWorkerTests.cs b/test/MinimalWorker.Test/CronWorkerTests.cs index 6d2da66..b747999 100644 --- a/test/MinimalWorker.Test/CronWorkerTests.cs +++ b/test/MinimalWorker.Test/CronWorkerTests.cs @@ -272,4 +272,27 @@ public async Task CronBackgroundWorker_With_Invalid_Expression_Should_Fail_Fast( $"Expected either error about invalid cron expression or worker should not execute. " + $"Worker executed: {workerExecuted}. Logs:\n{errorOutput}"); } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void CronBackgroundWorker_With_Empty_Expression_Should_Throw_ArgumentException(string? cronExpression) + { + // Arrange + BackgroundWorkerExtensions.ClearRegistrations(); + + using var host = Host.CreateDefaultBuilder().Build(); + + // Act & Assert - Empty cron expression should throw ArgumentException + var exception = Assert.Throws(() => + { + host.RunCronBackgroundWorker(cronExpression!, (CancellationToken token) => + { + return Task.CompletedTask; + }); + }); + + Assert.Equal("cronExpression", exception.ParamName); + } } diff --git a/test/MinimalWorker.Test/PeriodicWorkerTests.cs b/test/MinimalWorker.Test/PeriodicWorkerTests.cs index 2deeef1..ab8adec 100644 --- a/test/MinimalWorker.Test/PeriodicWorkerTests.cs +++ b/test/MinimalWorker.Test/PeriodicWorkerTests.cs @@ -177,33 +177,42 @@ public async Task PeriodicWorker_Should_Not_Overlap_Executions() } [Fact] - public async Task PeriodicWorker_With_Zero_Interval_Should_Handle_Gracefully() + public void PeriodicWorker_With_Zero_Interval_Should_Throw_ArgumentOutOfRangeException() { - // TODO: Perhaps we should fire an exception here :) // Arrange BackgroundWorkerExtensions.ClearRegistrations(); using var host = Host.CreateDefaultBuilder().Build(); - // Act - TimeSpan.Zero is handled gracefully by the library - // The worker starts but the PeriodicTimer never fires with zero interval - var exception = await Record.ExceptionAsync(async () => + // Act & Assert - TimeSpan.Zero should throw ArgumentOutOfRangeException + var exception = Assert.Throws(() => { host.RunPeriodicBackgroundWorker(TimeSpan.Zero, (CancellationToken token) => { return Task.CompletedTask; }); + }); + + Assert.Equal("timespan", exception.ParamName); + } + + [Fact] + public void PeriodicWorker_With_Negative_Interval_Should_Throw_ArgumentOutOfRangeException() + { + // Arrange + BackgroundWorkerExtensions.ClearRegistrations(); + + using var host = Host.CreateDefaultBuilder().Build(); - await host.StartAsync(); - await Task.Delay(50); - await host.StopAsync(); + // Act & Assert - Negative TimeSpan should throw ArgumentOutOfRangeException + var exception = Assert.Throws(() => + { + host.RunPeriodicBackgroundWorker(TimeSpan.FromSeconds(-5), (CancellationToken token) => + { + return Task.CompletedTask; + }); }); - // Assert - Library handles zero interval gracefully without crashing - // The worker registers and starts but the periodic timer doesn't fire - // This documents the actual behavior: no exception thrown, worker doesn't execute - Assert.Null(exception); - // Note: With TimeSpan.Zero, PeriodicTimer never fires, so execution count is 0 - // This is the expected graceful handling behavior + Assert.Equal("timespan", exception.ParamName); } }