Polly
Polly copied to clipboard
Revise Avoid compiler gotchas mixing sync and async execution
Is your feature request related to a specific problem? Or an existing feature? Please describe.
Specific problem.
A user can call Policy.Execute
on a Func<Task>
without any signal they've done so. They may not notice the issue until they have a real issue that was not retrying as expected.
Describe your proposed or preferred solution: RE: https://github.com/App-vNext/Polly/wiki/Avoid-compiler-gotchas-mixing-sync-and-async-execution I think we could introduce a Roslyn analyzer packaged with Polly that could detect this and raise the issue to the developer.
Describe any alternative options you've considered:
Alternatively, I could see us introducing an overload to signal with an ObsoleteAttribute
, though that may be too heavy-handed for users (CS0618
is a major analyzer to fail, and technically the mechanism isn't obsolete) -- note this approach we would need to handle every Execute overload.
[Obsolete("Use an AsyncPolicy and ExecuteAsync for async retries")]
public static Task Execute(
this Policy policy,
Func<Task> action) => policy.Execute(action);
[Obsolete("Use an AsyncPolicy and ExecuteAsync for async retries")]
public static Task<TResult> Execute<TResult>(
this Policy policy,
Func<Task<TResult>> action) => policy.Execute(action);
Any additional info? We should update the linked wiki if changed. There may be cases where a user actually does only want to retry the sync (Task creation) part of an operation.
The following seems to be appx what we'd need in an Analyzer
public override void Initialize(AnalysisContext context)
{
// ...
context.RegisterSyntaxNodeAction(AnalyzeSyntax, SyntaxKind.InvocationExpression);
}
private void AnalyzeSyntax(SyntaxNodeAnalysisContext context)
{
var invocationExpression = (InvocationExpressionSyntax)context.Node;
// TODO: Filter as much as we can on the syntax to reduce overhead - or use the SemanticModel Register hook?
var symbol = context.SemanticModel.GetSymbolInfo(invocationExpression).Symbol as IMethodSymbol;
if (symbol == null) return;
// Internet has better ways of comparing the INamedTypeSymbol to a Type
if (string.Equals(symbol.ContainingType?.ToString(), "Polly.Policy", StringComparison.Ordinal))
{
if (symbol.ContainingType.ContainingAssembly?.Identity?.Name?.Contains("Polly") == true)
{
if (string.Equals(symbol.Name, "Execute", StringComparison.Ordinal))
{
var typeArg = symbol.TypeArguments.SingleOrDefault();
if (typeArg == null) return; // not match
if (typeArg.ToString().StartsWith("System.Threading.Tasks.Task", StringComparison.Ordinal))
{
// This means they called Policy.Execute with a Task or Task{T}
context.ReportDiagnostic(
Diagnostic.Create(
Rule,
context.Node.GetLocation(),
typeArg.ToString()));
}
}
}
}
}
(needs 'productionization')
I believe this use case is improved in Polly v8 and doesn't require a custom analyzer.