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.

Sign up for GitHub

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

Jump to bottom

Add analyzers that help prevent developer gotchas #1426

Open
WhitWaldo opened this issue Dec 15, 2024 · 6 comments · May be fixed by #1448 or #1477
Open

Add analyzers that help prevent developer gotchas #1426

WhitWaldo opened this issue Dec 15, 2024 · 6 comments · May be fixed by #1448 or #1477
Assignees
@WhitWaldo
Labels
area/client help wanted Extra attention is needed kind/enhancement New feature or request
Milestone
v1.16

Comments

@WhitWaldo
Copy link
Contributor

WhitWaldo commented Dec 15, 2024 •
edited
Loading

Describe the feature

This is intended more as a parent issue to track progress for individual analyzers, but the goal is that we reduce some of the opportunities for developers to inadvertently misconfigure things that we might catch at build time and reduce runtime exceptions. Here's a short and evolving list of opportunities here:

Package Name Description Pull Request Fixes
Dapr.Actors Actor cross-serialization errors Ensure that when developers retain DataContract serialization or opt into JSON-based serialization, they're not inadvertently cross-serializing in a way that breaks the serializers and deserializers for either approach. #1441
Dapr.Actors Actor DI registration Identify actors that are referenced but not registered in DI. #1441
Dapr.Actors Don't use UseHttpsRedirection with actors Warn when using UseHttpsRedirection middleware as this breaks Dapr actor discovery
Dapr.Actors Ensure MapActorsHandlers is in app startup When AddActors is found, validate that MapActorsHandlers is in place #1441
Dapr.Workflows Workflow/activity DI registration Identify workflows and workflow activities that are referenced, but not registered in DI #1440
Dapr.Workflows Prevent DI injection errors Ensure that during startup, those resources that require a app.UseX() have it specified in Program.cs
Dapr.Workflows Validate inputs/outputs in workflows and activities Validate that the input and output types to and from a workflow and workflow activity match on either side of the operation. #1399
Dapr.Client (current) / Dapr.PubSub (future) Ensure MapSubscribeHandler is in app startup when programmatic subscriptions are used When programmatic subscriptions are used (e.g. WithTopic or a Topic attribute), validate that MapSubscribeHandler is in place** - thanks to @ngruson for the idea #1448

There are a great many analyzers and code fix providers in the Azure Functions Durable Extension repository that could be adapted for use in Dapr Workflows as well.

Release Note

RELEASE NOTE: ADD Analyzers to help prevent runtime errors
@WhitWaldo WhitWaldo added kind/enhancement New feature or request area/client labels Dec 15, 2024
@WhitWaldo WhitWaldo added this to the v1.16 milestone Dec 15, 2024
@WhitWaldo WhitWaldo added the help wanted Extra attention is needed label Dec 16, 2024
@jev-e
Copy link
Contributor

jev-e commented Dec 31, 2024

I'd be interested in looking at this; although partially motivated by self improvement as my knowledge of Roslyn analysers isn't super strong, but an area I'd like to improve.

This was referenced Jan 9, 2025
Open
Open
Open
@siri-varma
Copy link
Contributor

Planning to add an analyzer to make sure MapDaprScheduledJobHandler is in place to receive invocations for the jobs api. Let me know what you folks think ?

@WhitWaldo
Copy link
Contributor Author

Planning to add an analyzer to make sure MapDaprScheduledJobHandler is in place to receive invocations for the jobs api. Let me know what you folks think ?

I didn't realize analyzers can look up installed packages. If that's the case, sounds like a great idea.

Could take it a step farther, though a future release might render it obsolete. Because only the app that schedules a job can receive job invocations, you could do a scan for all the job names that are registered in the app and validate there's a corresponding mapped handler for each.

@siri-varma siri-varma linked a pull request Mar 1, 2025 that will close this issue
Open
3 tasks
@marcduiker
Copy link
Contributor

Can we add an analyzer that checks the input and output argument types when calling activities from the workflow? And if the activity class name matches? See the DurableTaskAnalyzers.

@WhitWaldo
Copy link
Contributor Author

Can we add an analyzer that checks the input and output argument types when calling activities from the workflow? And if the activity class name matches? See the DurableTaskAnalyzers.

I added the former to the list. Can you get into more detail on the latter ask and how it differs from "Identify workflows and workflow activities that are referenced, but not registered in DI" which is already in the table?

I'll go through the linked analyzers and get them added as well. Good find!

@marcduiker
Copy link
Contributor

Can we add an analyzer that checks the input and output argument types when calling activities from the workflow? And if the activity class name matches? See the DurableTaskAnalyzers.

I added the former to the list. Can you get into more detail on the latter ask and how it differs from "Identify workflows and workflow activities that are referenced, but not registered in DI" which is already in the table?

I'll go through the linked analyzers and get them added as well. Good find!

Yes, the other use case matches what you describe here 👍 .

@mikeee mikeee moved this from Backlog to In progress in v1.16 Release Tracking Board Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@WhitWaldo WhitWaldo

Labels
area/client help wanted Extra attention is needed kind/enhancement New feature or request
Projects
v1.16 Release Tracking Board
Status: In progress
Milestone
v1.16
4 participants
@WhitWaldo @marcduiker @siri-varma @jev-e