-
Couldn't load subscription status.
- Fork 0
added NotFoundBusinessException #12
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request enhances error handling by introducing nullable parameters for error messages and adding a new exception type for not-found scenarios. Key changes include:
- Updated exception handling in interceptors and command handler decorators to account for NotFoundBusinessException.
- Added NotFoundBusinessException with a default not-found message.
- Modified ResultMessagesExtensions and ResultMessage to support nullable parameters.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Facade/Default/Interceptors/ExceptionHandlingInterceptor.cs | Adjusts error handling logic by passing error messages as parameters. |
| src/Facade/Default/Honamic.Framework.Facade.csproj | Adds reference to the Domain.Abstractions project. |
| src/Core/Domain.Abstractions/NotFoundBusinessException.cs | Introduces NotFoundBusinessException with a default message. |
| src/Core/Applications/CommandHandlerDecorators/ExceptionCommandHandlerDecorator.cs | Adds new case handling for NotFoundBusinessException. |
| src/Core/Applications.Abstractions/Results/ResultMessagesExtensions.cs | Updates AppendError methods to accept nullable string values. |
| src/Core/Applications.Abstractions/Results/ResultMessage.cs | Changes constructor and properties for nullable error message parameters. |
Comments suppressed due to low confidence (1)
src/Core/Applications.Abstractions/Results/ResultMessage.cs:5
- The parameter order in the updated ResultMessage constructor has been reversed compared to its previous version. This causes the 'field' and 'code' values to be swapped when called by the AppendError methods; please revert the order or update all call sites accordingly.
public ResultMessage(ResultMessageType type, string message, string? code = null, string? field = null)
…tructor to place `field` before `code`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances error handling by introducing a new NotFoundBusinessException and updating related error reporting methods to support nullable parameters.
- Updates exception handling in interceptors and command handler decorators to include NotFound scenarios.
- Refactors ResultMessagesExtensions and ResultMessage to use nullable types for improved error messaging.
- Updates project references and build workflow configuration to support the changes.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Facade/Default/Interceptors/ExceptionHandlingInterceptor.cs | Adds NotFoundBusinessException handling and updates error method calls |
| src/Facade/Default/Honamic.Framework.Facade.csproj | Adds a project reference for domain abstractions |
| src/Core/Domain.Abstractions/NotFoundBusinessException.cs | Introduces a new exception class for not found scenarios |
| src/Core/Applications/CommandHandlerDecorators/ExceptionCommandHandlerDecorator.cs | Incorporates NotFoundBusinessException in command error handling |
| src/Core/Applications.Abstractions/Results/ResultMessagesExtensions.cs | Updates AppendError overloads to accept nullable parameters |
| src/Core/Applications.Abstractions/Results/ResultMessage.cs | Updates properties to allow null values for field and code |
| .github/workflows/build.yml | Refines build workflow trigger configuration |
| switch (ex) | ||
| { | ||
| case UnauthenticatedException: | ||
| rawResult?.SetStatusAsUnauthenticated(); | ||
| rawResult?.AppendError(ex.Message, "Exception"); | ||
| rawResult.SetStatusAsUnauthenticated(ex.Message); | ||
| break; |
Copilot
AI
May 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider ensuring that the null handling of 'rawResult' is consistent across all exception cases; some cases use the null-conditional operator while others do not, so validating the non-null assumption here might help avoid potential issues.
| rawResult?.SetStatusAsUnhandledExceptionWithSorryError(); | ||
| rawResult?.AppendError(ex.ToString(), "Exception"); | ||
| rawResult?.AppendError(ex.Message, "Exception"); |
Copilot
AI
May 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review the inconsistent use of the null-conditional operator on 'rawResult'; aligning its usage across all cases will improve code clarity and reduce ambiguity in error handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances error handling by introducing a new NotFoundBusinessException and updating error reporting to support nullable error message parameters. Key changes include:
- Introducing NotFoundBusinessException with a default error message.
- Updating ExceptionHandlingInterceptor and ExceptionCommandHandlerDecorator to handle the new exception type.
- Modifying ResultMessage and AppendError extension methods to support nullable string parameters.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Facade/Default/Interceptors/ExceptionHandlingInterceptor.cs | Updated exception switch cases for improved error reporting. |
| src/Facade/Default/Honamic.Framework.Facade.csproj | Added a project reference for domain abstractions. |
| src/Core/Domain.Abstractions/NotFoundBusinessException.cs | Introduced the new exception class with a default message. |
| src/Core/Applications/CommandHandlerDecorators/ExceptionCommandHandlerDecorator.cs | Added handling for NotFoundBusinessException in command handling. |
| src/Core/Applications.Abstractions/Results/ResultMessagesExtensions.cs | Updated AppendError method signatures and added a new status setter. |
| src/Core/Applications.Abstractions/Results/ResultMessage.cs | Modified the ResultMessage model for nullable error fields. |
| .github/workflows/build.yml | Adjusted workflow triggers to restrict events to the main branch. |
added NotFoundBusinessException
PR Classification
Enhancement of error handling and message reporting in the application.
PR Summary
This pull request introduces nullable parameters for error messages and adds a new exception type for better error handling. Key changes include:
ResultMessage.cs: Updated to use nullable string types forcodeandfieldparameters.ResultMessagesExtensions.cs: ModifiedAppendErrormethods to accept nullable parameters.NotFoundBusinessException.cs: Introduced a new exception class for handling not found scenarios.ExceptionCommandHandlerDecorator.cs: Added handling forNotFoundBusinessExceptionto set appropriate result status.ExceptionHandlingInterceptor.cs: Enhanced error logging and added handling forNotFoundBusinessException.