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

Implement exception handler service; fixes #345 #346

Open
wants to merge 9 commits into
base: development
Choose a base branch
from

Conversation

lxatstariongroup
Copy link
Contributor

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the COMET-SDK code style guidelines
  • I have provided test coverage for my change (where applicable)

Description

@lxatstariongroup lxatstariongroup changed the title [WIP] Feat/gh345 implement exception handler service Implement exception handler service; fixes #345 Jun 27, 2024
@lxatstariongroup lxatstariongroup self-assigned this Jun 27, 2024
@lxatstariongroup
Copy link
Contributor Author

lxatstariongroup commented Jun 28, 2024

Small comment about implementation:
The system will execute all connected handlers every time, regardless of the result (ishandled) of an individual handler in case multiple handlers are executed. So it does not stop the process when an individual handler returns IsHandled = true.
The reason is that, due to DI registration/resolving of IExceptionHandlers, we don't have control over the order in which the IExceptionHandlers are executed.

Fyi: the IsHandled is documented as:

        /// <returns>a boolean value indicating if the <see cref="Exception"/> was handled or not, so it could be thrown again</returns>

Do you agree on that?

Copy link
Contributor

@antoineatstariongroup antoineatstariongroup left a comment

Choose a reason for hiding this comment

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

Check file comment

CDP4Common.Tests/CDP4Common - Backup.Tests.csproj Outdated Show resolved Hide resolved
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.

Implement ExceptionHandlerService in CDP4Common and use in CDP4Dal.Session
2 participants