diff --git a/Brainarr.Plugin/Services/Core/BrainarrOrchestratorFactory.cs b/Brainarr.Plugin/Services/Core/BrainarrOrchestratorFactory.cs index 7fdb884f..95446bcb 100644 --- a/Brainarr.Plugin/Services/Core/BrainarrOrchestratorFactory.cs +++ b/Brainarr.Plugin/Services/Core/BrainarrOrchestratorFactory.cs @@ -156,7 +156,8 @@ public static void ConfigureServices(IServiceCollection services) services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(sp => new TopUpPlanner(sp.GetRequiredService())); - services.TryAddSingleton(); + // WS4.2: Use CommonBreakerRegistry which delegates to Common's AdvancedCircuitBreaker + services.TryAddSingleton(); services.TryAddSingleton(sp => new RecommendationPipeline( diff --git a/Brainarr.Plugin/Services/Resilience/BrainarrCircuitBreakerAdapter.cs b/Brainarr.Plugin/Services/Resilience/BrainarrCircuitBreakerAdapter.cs new file mode 100644 index 00000000..ef99d4b1 --- /dev/null +++ b/Brainarr.Plugin/Services/Resilience/BrainarrCircuitBreakerAdapter.cs @@ -0,0 +1,205 @@ +using System; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using NLog; +using CommonBreaker = Lidarr.Plugin.Common.Services.Resilience.AdvancedCircuitBreaker; +using CommonOptions = Lidarr.Plugin.Common.Services.Resilience.AdvancedCircuitBreakerOptions; +using CommonState = Lidarr.Plugin.Common.Services.Resilience.CircuitState; +using CommonOpenException = Lidarr.Plugin.Common.Services.Resilience.CircuitBreakerOpenException; + +namespace NzbDrone.Core.ImportLists.Brainarr.Services.Resilience +{ + /// + /// Adapts Common's AdvancedCircuitBreaker to Brainarr's ICircuitBreaker interface. + /// This adapter preserves existing Brainarr semantics while delegating to Common's implementation. + /// + /// + /// Key behavior preserved from Brainarr's original CircuitBreaker: + /// - TaskCanceledException is treated as a failure (surprising but documented) + /// - HttpRequestException with "4" AND "Bad Request" in message is excluded (brittle string matching) + /// - TimeoutException and HttpRequestException are treated as failures + /// - Other exceptions pass through without affecting breaker state + /// + internal sealed class BrainarrCircuitBreakerAdapter : ICircuitBreaker + { + private readonly CommonBreaker _inner; + private readonly Logger _logger; + private readonly TimeSpan _breakDuration; + private DateTime _lastStateChange = DateTime.UtcNow; + + public BrainarrCircuitBreakerAdapter(string resourceName, CommonOptions options, Logger logger) + { + ResourceName = resourceName ?? throw new ArgumentNullException(nameof(resourceName)); + _logger = logger ?? LogManager.GetCurrentClassLogger(); + + // Configure Common's options with Brainarr's exception classification + var configuredOptions = ConfigureOptions(options); + _breakDuration = configuredOptions.BreakDuration; + _inner = new CommonBreaker(resourceName, configuredOptions); + _inner.StateChanged += OnInnerStateChanged; + } + + public string ResourceName { get; } + + public CircuitState State => MapState(_inner.State); + + public DateTime LastStateChange => _lastStateChange; + + public int ConsecutiveFailures => _inner.ConsecutiveFailures; + + public double FailureRate => _inner.FailureRate; + + public event EventHandler CircuitOpened; + public event EventHandler CircuitClosed; + + public async Task ExecuteAsync(Func> operation, CancellationToken cancellationToken = default) + { + try + { + return await _inner.ExecuteAsync(operation).ConfigureAwait(false); + } + catch (CommonOpenException ex) + { + // Translate Common's exception to Brainarr's + throw new CircuitBreakerOpenException( + $"Circuit breaker is open for {ResourceName}. Will retry at {DateTime.UtcNow.Add(ex.RetryAfter ?? TimeSpan.Zero):HH:mm:ss}"); + } + } + + public async Task ExecuteWithFallbackAsync( + Func> operation, + T fallbackValue, + CancellationToken cancellationToken = default) + { + try + { + return await ExecuteAsync(operation, cancellationToken).ConfigureAwait(false); + } + catch (CircuitBreakerOpenException) + { + _logger.Warn($"Circuit breaker open for {ResourceName}, using fallback value"); + return fallbackValue; + } + } + + public CircuitBreakerStatistics GetStatistics() + { + var stats = _inner.Statistics; + DateTime? nextHalfOpenAttempt = null; + if (State == CircuitState.Open) + { + nextHalfOpenAttempt = stats.LastOpenedTime?.Add(_breakDuration) ?? _lastStateChange.Add(_breakDuration); + } + return new CircuitBreakerStatistics + { + ResourceName = ResourceName, + State = State, + ConsecutiveFailures = ConsecutiveFailures, + FailureRate = FailureRate, + TotalOperations = (int)(stats.TotalSuccesses + stats.TotalFailures), + LastStateChange = _lastStateChange, + NextHalfOpenAttempt = nextHalfOpenAttempt, + RecentOperations = null // Not exposed by Common, and not critical for behavior + }; + } + + public void Reset() + { + _inner.Reset(); + _lastStateChange = DateTime.UtcNow; + _logger.Info($"Circuit breaker manually RESET for {ResourceName}"); + } + + /// + /// Configures Common's AdvancedCircuitBreakerOptions with Brainarr's exception classification. + /// + private static CommonOptions ConfigureOptions(CommonOptions baseOptions) + { + // Clone the base options and override exception classification + return new CommonOptions + { + ConsecutiveFailureThreshold = baseOptions?.ConsecutiveFailureThreshold ?? 5, + FailureRateThreshold = baseOptions?.FailureRateThreshold ?? 0.5, + MinimumThroughput = baseOptions?.MinimumThroughput ?? 10, + SamplingWindowSize = baseOptions?.SamplingWindowSize ?? 20, + BreakDuration = baseOptions?.BreakDuration ?? TimeSpan.FromSeconds(30), + HalfOpenSuccessThreshold = baseOptions?.HalfOpenSuccessThreshold ?? 3, + IsFailure = IsFailure, + IsIgnored = IsIgnored + }; + } + + /// + /// Determines if an exception counts as a failure (trips the breaker). + /// Matches original Brainarr behavior: TaskCanceledException, TimeoutException, HttpRequestException + /// EXCEPT client errors with "4" AND "Bad Request" in message. + /// + private static bool IsFailure(Exception ex) + { + // First check if it's a handled type + if (!(ex is TaskCanceledException || ex is TimeoutException || ex is HttpRequestException)) + { + return false; + } + + // Exclude client errors (4xx) - brittle string matching preserved from original + if (ex.Message.Contains("4") && ex.Message.Contains("Bad Request")) + { + return false; + } + + return true; + } + + /// + /// Determines if an exception should be ignored entirely (not counted at all). + /// Currently null - all non-failure exceptions pass through without counting. + /// + private static bool IsIgnored(Exception ex) + { + // In Brainarr's original implementation, non-handled exceptions just pass through + // without being recorded. The Common breaker handles this via IsFailure returning false. + return false; + } + + private void OnInnerStateChanged(object sender, global::Lidarr.Plugin.Common.Services.Resilience.CircuitBreakerEventArgs e) + { + _lastStateChange = DateTime.UtcNow; + + var args = new CircuitBreakerEventArgs + { + ResourceName = ResourceName, + State = MapState(e.NewState), + Timestamp = DateTime.UtcNow + }; + + if (e.NewState == CommonState.Open) + { + _logger.Error($"Circuit breaker OPENED for {ResourceName}. " + + $"Failures: {ConsecutiveFailures}, Rate: {FailureRate:P}"); + CircuitOpened?.Invoke(this, args); + } + else if (e.NewState == CommonState.Closed && e.PreviousState != CommonState.Closed) + { + _logger.Info($"Circuit breaker CLOSED for {ResourceName}. Provider recovered successfully."); + CircuitClosed?.Invoke(this, args); + } + else if (e.NewState == CommonState.HalfOpen) + { + _logger.Info($"Circuit breaker transitioned to HALF-OPEN for {ResourceName}"); + } + } + + private static CircuitState MapState(CommonState state) + { + return state switch + { + CommonState.Closed => CircuitState.Closed, + CommonState.Open => CircuitState.Open, + CommonState.HalfOpen => CircuitState.HalfOpen, + _ => CircuitState.Closed + }; + } + } +} diff --git a/Brainarr.Plugin/Services/Resilience/CommonBreakerRegistry.cs b/Brainarr.Plugin/Services/Resilience/CommonBreakerRegistry.cs new file mode 100644 index 00000000..5e6439c8 --- /dev/null +++ b/Brainarr.Plugin/Services/Resilience/CommonBreakerRegistry.cs @@ -0,0 +1,64 @@ +using System; +using System.Collections.Concurrent; +using NLog; +using NzbDrone.Core.ImportLists.Brainarr.Configuration; +using NzbDrone.Core.ImportLists.Brainarr.Services.Core; +using CommonOptions = Lidarr.Plugin.Common.Services.Resilience.AdvancedCircuitBreakerOptions; + +namespace NzbDrone.Core.ImportLists.Brainarr.Services.Resilience +{ + /// + /// Registry that provides per-provider+model circuit breakers using Common's AdvancedCircuitBreaker. + /// This is the WS4.2 replacement for BreakerRegistry, delegating to Common while preserving Brainarr semantics. + /// + public sealed class CommonBreakerRegistry : IBreakerRegistry + { + private readonly ConcurrentDictionary _breakers = new(); + + public ICircuitBreaker Get(ModelKey key, Logger logger, CircuitBreakerOptions? options = null) + { + if (string.IsNullOrWhiteSpace(key.Provider)) + throw new ArgumentException("Provider is required", nameof(key)); + + logger ??= LogManager.GetCurrentClassLogger(); + + return _breakers.GetOrAdd(key, k => + { + var resource = $"ai:{k.Provider}:{k.ModelId}"; + var commonOptions = MapOptions(options); + return new BrainarrCircuitBreakerAdapter(resource, commonOptions, logger); + }); + } + + /// + /// Maps Brainarr's CircuitBreakerOptions to Common's AdvancedCircuitBreakerOptions. + /// Preserves all the configuration knobs while using Common's defaults where not specified. + /// + private static CommonOptions MapOptions(CircuitBreakerOptions? brainarrOptions) + { + if (brainarrOptions == null) + { + // Use Brainarr's default values from BrainarrConstants + return new CommonOptions + { + ConsecutiveFailureThreshold = 5, + FailureRateThreshold = BrainarrConstants.CircuitBreakerFailureThreshold, + MinimumThroughput = BrainarrConstants.CircuitBreakerMinimumThroughput, + SamplingWindowSize = BrainarrConstants.CircuitBreakerSamplingWindow, + BreakDuration = TimeSpan.FromSeconds(BrainarrConstants.CircuitBreakerDurationSeconds), + HalfOpenSuccessThreshold = 3 + }; + } + + return new CommonOptions + { + ConsecutiveFailureThreshold = brainarrOptions.FailureThreshold, + FailureRateThreshold = brainarrOptions.FailureRateThreshold, + MinimumThroughput = brainarrOptions.MinimumThroughput, + SamplingWindowSize = brainarrOptions.SamplingWindowSize, + BreakDuration = brainarrOptions.BreakDuration, + HalfOpenSuccessThreshold = brainarrOptions.HalfOpenSuccessThreshold + }; + } + } +} diff --git a/Brainarr.Tests/Services/Resilience/BreakerRegistryInjectionTests.cs b/Brainarr.Tests/Services/Resilience/BreakerRegistryInjectionTests.cs index c74bff0f..2d69b1e2 100644 --- a/Brainarr.Tests/Services/Resilience/BreakerRegistryInjectionTests.cs +++ b/Brainarr.Tests/Services/Resilience/BreakerRegistryInjectionTests.cs @@ -35,7 +35,7 @@ public void Factory_Registers_IBreakerRegistry() // Assert var registry = provider.GetService(); registry.Should().NotBeNull("IBreakerRegistry should be registered by factory"); - registry.Should().BeOfType(); + registry.Should().BeOfType(); } [Fact] @@ -74,7 +74,7 @@ public void Orchestrator_Accepts_Injected_BreakerRegistry() [Fact] public void DI_Resolves_Same_Registry_Instance_For_Multiple_Orchestrators() { - // IMPORTANT: BreakerRegistry must be singleton to preserve breaker state across + // IMPORTANT: IBreakerRegistry must be singleton to preserve breaker state across // orchestrator instances. If transient, breaker history would reset per instance. var services = new ServiceCollection(); services.AddSingleton(LogManager.GetCurrentClassLogger()); @@ -90,7 +90,7 @@ public void DI_Resolves_Same_Registry_Instance_For_Multiple_Orchestrators() var registry2 = provider.GetRequiredService(); // Assert - must be same instance (singleton) - registry1.Should().BeSameAs(registry2, "BreakerRegistry must be singleton to preserve state"); + registry1.Should().BeSameAs(registry2, "IBreakerRegistry must be singleton to preserve state"); } [Fact] diff --git a/ext-common-sha.txt b/ext-common-sha.txt index 4214b801..3e5c32f1 100644 --- a/ext-common-sha.txt +++ b/ext-common-sha.txt @@ -1 +1 @@ -e1fd02e15f7e94bdc8d83c245691a1f8ae07cb65 +deac78ca19848f20369c54c510fb66997e3eb0e8 diff --git a/ext/lidarr.plugin.common b/ext/lidarr.plugin.common index e1fd02e1..deac78ca 160000 --- a/ext/lidarr.plugin.common +++ b/ext/lidarr.plugin.common @@ -1 +1 @@ -Subproject commit e1fd02e15f7e94bdc8d83c245691a1f8ae07cb65 +Subproject commit deac78ca19848f20369c54c510fb66997e3eb0e8