Skip to content

Error handling for failures on registered actions for Custom Checks #10914

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

Merged
merged 6 commits into from
Nov 11, 2024

Conversation

maridematte
Copy link
Member

Fixes #10522

Context

Second PR for that issue. This PR covers the cases in a Custom Check where there is an exception while registering actions, and when running said actions.
Since actions are individuals, we do not deregister the whole analyzer when one fails.

It has been decided to expose these errors are warnings instead to not break the build where they show up.

Changes Made

Added some try catch statements so errors do not break builds. Messages have been added to each case for more detail as we do not preserve anything else from the exception.

Testing

Reenabled unit test and added the previously commented out cases.

Notes

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good!
I have two suggestions for fixing - let's either discuss or fix - then I'm ready to signoff

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely achieving the goal - but let's make the code little more efficient and less repetitive

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@maridematte maridematte enabled auto-merge (squash) November 11, 2024 09:37
@maridematte maridematte merged commit f637ded into dotnet:main Nov 11, 2024
10 checks passed
@maridematte maridematte deleted the 10522 branch March 10, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better Check error message
3 participants