Skip to content

Commit

Permalink
Refactor code to reduce nesting (#1453)
Browse files Browse the repository at this point in the history 
Reduced nesting levels through block-scoped `using`s and the inversion of an `if` block.

Co-authored-by: Lehonti
  • Loading branch information
@Lehonti
Lehonti authored Aug 4, 2023
1 parent 77e91d5 commit 0f38c51
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 187 deletions.
97 changes: 47 additions & 50 deletions src/Polly/CircuitBreaker/AdvancedCircuitController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,73 +29,70 @@ Action onHalfOpen

public override void OnCircuitReset(Context context)
{
using (TimedLock.Lock(_lock))
{
// Is only null during initialization of the current class
// as the variable is not set, before the base class calls
// current method from constructor.
_metrics?.Reset_NeedsLock();
using var _ = TimedLock.Lock(_lock);

ResetInternal_NeedsLock(context);
}
// Is only null during initialization of the current class
// as the variable is not set, before the base class calls
// current method from constructor.
_metrics?.Reset_NeedsLock();

ResetInternal_NeedsLock(context);
}

public override void OnActionSuccess(Context context)
{
using (TimedLock.Lock(_lock))
{
switch (_circuitState)
{
case CircuitState.HalfOpen:
OnCircuitReset(context);
break;
using var _ = TimedLock.Lock(_lock);

case CircuitState.Closed:
break;
switch (_circuitState)
{
case CircuitState.HalfOpen:
OnCircuitReset(context);
break;

case CircuitState.Open:
case CircuitState.Isolated:
break; // A successful call result may arrive when the circuit is open, if it was placed before the circuit broke. We take no special action; only time passing governs transitioning from Open to HalfOpen state.
case CircuitState.Closed:
break;

default:
throw new InvalidOperationException("Unhandled CircuitState.");
}
case CircuitState.Open:
case CircuitState.Isolated:
break; // A successful call result may arrive when the circuit is open, if it was placed before the circuit broke. We take no special action; only time passing governs transitioning from Open to HalfOpen state.

_metrics.IncrementSuccess_NeedsLock();
default:
throw new InvalidOperationException("Unhandled CircuitState.");
}

_metrics.IncrementSuccess_NeedsLock();
}

public override void OnActionFailure(DelegateResult<TResult> outcome, Context context)
{
using (TimedLock.Lock(_lock))
using var _ = TimedLock.Lock(_lock);

_lastOutcome = outcome;

switch (_circuitState)
{
_lastOutcome = outcome;
case CircuitState.HalfOpen:
Break_NeedsLock(context);
return;

case CircuitState.Closed:
_metrics.IncrementFailure_NeedsLock();
var healthCount = _metrics.GetHealthCount_NeedsLock();

switch (_circuitState)
{
case CircuitState.HalfOpen:
int throughput = healthCount.Total;
if (throughput >= _minimumThroughput && (double)healthCount.Failures / throughput >= _failureThreshold)
{
Break_NeedsLock(context);
return;

case CircuitState.Closed:
_metrics.IncrementFailure_NeedsLock();
var healthCount = _metrics.GetHealthCount_NeedsLock();

int throughput = healthCount.Total;
if (throughput >= _minimumThroughput && (double)healthCount.Failures / throughput >= _failureThreshold)
{
Break_NeedsLock(context);
}
break;

case CircuitState.Open:
case CircuitState.Isolated:
_metrics.IncrementFailure_NeedsLock();
break; // A failure call result may arrive when the circuit is open, if it was placed before the circuit broke. We take no action beyond tracking the metric; we do not want to duplicate-signal onBreak; we do not want to extend time for which the circuit is broken. We do not want to mask the fact that the call executed (as replacing its result with a Broken/IsolatedCircuitException would do).

default:
throw new InvalidOperationException("Unhandled CircuitState.");
}
}
break;

case CircuitState.Open:
case CircuitState.Isolated:
_metrics.IncrementFailure_NeedsLock();
break; // A failure call result may arrive when the circuit is open, if it was placed before the circuit broke. We take no action beyond tracking the metric; we do not want to duplicate-signal onBreak; we do not want to extend time for which the circuit is broken. We do not want to mask the fact that the call executed (as replacing its result with a Broken/IsolatedCircuitException would do).

default:
throw new InvalidOperationException("Unhandled CircuitState.");
}
}
}
47 changes: 20 additions & 27 deletions src/Polly/CircuitBreaker/CircuitStateController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,51 +36,44 @@ public CircuitState CircuitState
return _circuitState;
}

using (TimedLock.Lock(_lock))
using var _ = TimedLock.Lock(_lock);

if (_circuitState == CircuitState.Open && !IsInAutomatedBreak_NeedsLock)
{
if (_circuitState == CircuitState.Open && !IsInAutomatedBreak_NeedsLock)
{
_circuitState = CircuitState.HalfOpen;
_onHalfOpen();
}
return _circuitState;
_circuitState = CircuitState.HalfOpen;
_onHalfOpen();
}

return _circuitState;
}
}

public Exception LastException
{
get
{
using (TimedLock.Lock(_lock))
{
return _lastOutcome?.Exception;
}
using var _ = TimedLock.Lock(_lock);
return _lastOutcome?.Exception;
}
}

public TResult LastHandledResult
{
get
{
using (TimedLock.Lock(_lock))
{
return _lastOutcome != null
? _lastOutcome.Result : default;
}
using var _ = TimedLock.Lock(_lock);
return _lastOutcome != null ? _lastOutcome.Result : default;
}
}

protected bool IsInAutomatedBreak_NeedsLock => SystemClock.UtcNow().Ticks < _blockedTill;

public void Isolate()
{
using (TimedLock.Lock(_lock))
{
_lastOutcome = new DelegateResult<TResult>(new IsolatedCircuitException("The circuit is manually held open and is not allowing calls."));
BreakFor_NeedsLock(TimeSpan.MaxValue, Context.None());
_circuitState = CircuitState.Isolated;
}
using var _ = TimedLock.Lock(_lock);
_lastOutcome = new DelegateResult<TResult>(new IsolatedCircuitException("The circuit is manually held open and is not allowing calls."));
BreakFor_NeedsLock(TimeSpan.MaxValue, Context.None());
_circuitState = CircuitState.Isolated;
}

protected void Break_NeedsLock(Context context) =>
Expand Down Expand Up @@ -118,13 +111,13 @@ protected void ResetInternal_NeedsLock(Context context)
protected bool PermitHalfOpenCircuitTest()
{
long currentlyBlockedUntil = _blockedTill;
if (SystemClock.UtcNow().Ticks >= currentlyBlockedUntil)
if (SystemClock.UtcNow().Ticks < currentlyBlockedUntil)
{
// It's time to permit a / another trial call in the half-open state ...
// ... but to prevent race conditions/multiple calls, we have to ensure only _one_ thread wins the race to own this next call.
return Interlocked.CompareExchange(ref _blockedTill, SystemClock.UtcNow().Ticks + _durationOfBreak.Ticks, currentlyBlockedUntil) == currentlyBlockedUntil;
return false;
}
return false;
// It's time to permit a / another trial call in the half-open state ...
// ... but to prevent race conditions/multiple calls, we have to ensure only _one_ thread wins the race to own this next call.
return Interlocked.CompareExchange(ref _blockedTill, SystemClock.UtcNow().Ticks + _durationOfBreak.Ticks, currentlyBlockedUntil) == currentlyBlockedUntil;
}

private BrokenCircuitException GetBreakingException()
Expand Down
77 changes: 36 additions & 41 deletions src/Polly/CircuitBreaker/ConsecutiveCountCircuitController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,65 +16,60 @@ Action onHalfOpen

public override void OnCircuitReset(Context context)
{
using (TimedLock.Lock(_lock))
{
_consecutiveFailureCount = 0;

ResetInternal_NeedsLock(context);
}
using var _ = TimedLock.Lock(_lock);
_consecutiveFailureCount = 0;
ResetInternal_NeedsLock(context);
}

public override void OnActionSuccess(Context context)
{
using (TimedLock.Lock(_lock))
using var _ = TimedLock.Lock(_lock);

switch (_circuitState)
{
switch (_circuitState)
{
case CircuitState.HalfOpen:
OnCircuitReset(context);
break;
case CircuitState.HalfOpen:
OnCircuitReset(context);
break;

case CircuitState.Closed:
_consecutiveFailureCount = 0;
break;
case CircuitState.Closed:
_consecutiveFailureCount = 0;
break;

case CircuitState.Open:
case CircuitState.Isolated:
break; // A successful call result may arrive when the circuit is open, if it was placed before the circuit broke. We take no action; only time passing governs transitioning from Open to HalfOpen state.
case CircuitState.Open:
case CircuitState.Isolated:
break; // A successful call result may arrive when the circuit is open, if it was placed before the circuit broke. We take no action; only time passing governs transitioning from Open to HalfOpen state.

default:
throw new InvalidOperationException("Unhandled CircuitState.");
}
default:
throw new InvalidOperationException("Unhandled CircuitState.");
}
}

public override void OnActionFailure(DelegateResult<TResult> outcome, Context context)
{
using (TimedLock.Lock(_lock))
using var _ = TimedLock.Lock(_lock);

_lastOutcome = outcome;

switch (_circuitState)
{
_lastOutcome = outcome;
case CircuitState.HalfOpen:
Break_NeedsLock(context);
return;

switch (_circuitState)
{
case CircuitState.HalfOpen:
case CircuitState.Closed:
_consecutiveFailureCount += 1;
if (_consecutiveFailureCount >= _exceptionsAllowedBeforeBreaking)
{
Break_NeedsLock(context);
return;

case CircuitState.Closed:
_consecutiveFailureCount += 1;
if (_consecutiveFailureCount >= _exceptionsAllowedBeforeBreaking)
{
Break_NeedsLock(context);
}
break;
}
break;

case CircuitState.Open:
case CircuitState.Isolated:
break; // A failure call result may arrive when the circuit is open, if it was placed before the circuit broke. We take no action; we do not want to duplicate-signal onBreak; we do not want to extend time for which the circuit is broken. We do not want to mask the fact that the call executed (as replacing its result with a Broken/IsolatedCircuitException would do).
case CircuitState.Open:
case CircuitState.Isolated:
break; // A failure call result may arrive when the circuit is open, if it was placed before the circuit broke. We take no action; we do not want to duplicate-signal onBreak; we do not want to extend time for which the circuit is broken. We do not want to mask the fact that the call executed (as replacing its result with a Broken/IsolatedCircuitException would do).

default:
throw new InvalidOperationException("Unhandled CircuitState.");
}
default:
throw new InvalidOperationException("Unhandled CircuitState.");
}
}
}
57 changes: 27 additions & 30 deletions src/Polly/Timeout/AsyncTimeoutEngine.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,45 +14,42 @@ internal static async Task<TResult> ImplementationAsync<TResult>(
cancellationToken.ThrowIfCancellationRequested();
TimeSpan timeout = timeoutProvider(context);

using (CancellationTokenSource timeoutCancellationTokenSource = new CancellationTokenSource())
{
using (CancellationTokenSource combinedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCancellationTokenSource.Token))
{
Task<TResult> actionTask = null;
CancellationToken combinedToken = combinedTokenSource.Token;
using var timeoutCancellationTokenSource = new CancellationTokenSource();
using var combinedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCancellationTokenSource.Token);

try
{
if (timeoutStrategy == TimeoutStrategy.Optimistic)
{
SystemClock.CancelTokenAfter(timeoutCancellationTokenSource, timeout);
return await action(context, combinedToken).ConfigureAwait(continueOnCapturedContext);
}
Task<TResult> actionTask = null;
CancellationToken combinedToken = combinedTokenSource.Token;

// else: timeoutStrategy == TimeoutStrategy.Pessimistic
try
{
if (timeoutStrategy == TimeoutStrategy.Optimistic)
{
SystemClock.CancelTokenAfter(timeoutCancellationTokenSource, timeout);
return await action(context, combinedToken).ConfigureAwait(continueOnCapturedContext);
}

Task<TResult> timeoutTask = timeoutCancellationTokenSource.Token.AsTask<TResult>();
// else: timeoutStrategy == TimeoutStrategy.Pessimistic

SystemClock.CancelTokenAfter(timeoutCancellationTokenSource, timeout);
Task<TResult> timeoutTask = timeoutCancellationTokenSource.Token.AsTask<TResult>();

actionTask = action(context, combinedToken);
SystemClock.CancelTokenAfter(timeoutCancellationTokenSource, timeout);

return await (await Task.WhenAny(actionTask, timeoutTask).ConfigureAwait(continueOnCapturedContext)).ConfigureAwait(continueOnCapturedContext);
actionTask = action(context, combinedToken);

}
catch (Exception ex)
{
// Note that we cannot rely on testing (operationCanceledException.CancellationToken == combinedToken || operationCanceledException.CancellationToken == timeoutCancellationTokenSource.Token)
// as either of those tokens could have been onward combined with another token by executed code, and so may not be the token expressed on operationCanceledException.CancellationToken.
if (ex is OperationCanceledException && timeoutCancellationTokenSource.IsCancellationRequested)
{
await onTimeoutAsync(context, timeout, actionTask, ex).ConfigureAwait(continueOnCapturedContext);
throw new TimeoutRejectedException("The delegate executed asynchronously through TimeoutPolicy did not complete within the timeout.", ex);
}
return await (await Task.WhenAny(actionTask, timeoutTask).ConfigureAwait(continueOnCapturedContext)).ConfigureAwait(continueOnCapturedContext);

throw;
}
}
catch (Exception ex)
{
// Note that we cannot rely on testing (operationCanceledException.CancellationToken == combinedToken || operationCanceledException.CancellationToken == timeoutCancellationTokenSource.Token)
// as either of those tokens could have been onward combined with another token by executed code, and so may not be the token expressed on operationCanceledException.CancellationToken.
if (ex is OperationCanceledException && timeoutCancellationTokenSource.IsCancellationRequested)
{
await onTimeoutAsync(context, timeout, actionTask, ex).ConfigureAwait(continueOnCapturedContext);
throw new TimeoutRejectedException("The delegate executed asynchronously through TimeoutPolicy did not complete within the timeout.", ex);
}

throw;
}
}

Expand Down
Loading

0 comments on commit 0f38c51

Please sign in to comment.