Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analyzer request: Emit a warning for usage of CancellationToken as a parameter in orchestrator functions #1526

Closed
cgillum opened this issue Oct 9, 2020 · 2 comments · Fixed by #1634
Assignees
Labels
analyzer Issues with the Roslyn analyzer

Comments

@cgillum
Copy link
Member

cgillum commented Oct 9, 2020

Is your feature request related to a problem? Please describe.
.NET Function Apps allow customers to bind to a CancellationToken parameter to detect when the functions host is being shut down. We recently received a question about whether this should be used for things like the CreateTimer API. My thinking is that customers should never use this CancellationToken parameter in orchestrator function apps. I can't think of a scenario where it would not create problems.

Describe the solution you'd like
It would be great to have an analyzer rule that emits a warning if someone has a CancellationToken parameter in their orchestrator function.

@ghost ghost added the Needs: Triage 🔍 label Oct 9, 2020
@cgillum cgillum added analyzer Issues with the Roslyn analyzer and removed Needs: Triage 🔍 labels Oct 9, 2020
@ConnorMcMahon
Copy link
Contributor

ConnorMcMahon commented Oct 9, 2020

I have definitely had a customer cancel a timer in a loop, and it caused the timer to always immediately return, resulting in an infinite loop in their code.

It was very hard to debug, so I am all for this.

@ConnorMcMahon
Copy link
Contributor

ConnorMcMahon commented Jan 13, 2021

EDIT: I misinterpreted this based on the title to mean all CancellationToken usage in orchestrator functions. I agree we should block this with the analyzer.

I think there are some other cancellation token usage patterns that we want to discourage in orchestrator functions, but we will want to understand what those exact scenarios before we attempt to write an analyzer for them.

Original Text below:

I'm now not 100% positive that we want to emit a warning for all uses of CancellationTokens.

While I can't think of any cases where using a cancellation token would actually be helpful with the orchestration APIs, I'm fairly certain there are many cases where it is harmless. In fact, I think some examples of that are in our own samples/documentation, and we should definitely fix those if the cancellation tokens aren't actually providing value.

Introducing a warning for these harmless cases may cause customers with large code bases to turn off all of the Durable analyzers, as we have seen this happen in the past where we introduced false positives.

@ConnorMcMahon ConnorMcMahon changed the title Analyzer request: Emit a warning for usage of CancellationToken in orchestrator functions Analyzer request: Emit a warning for usage of CancellationToken as a parameter in orchestrator functions Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer Issues with the Roslyn analyzer
Projects
None yet
3 participants