[BEEEP] [PM-2844] Add custom error codes for server API exceptions #70
Conversation
…sage from resource files if the error thrown is an error code
…odes for exceptions
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 file(s) reviewed, 15 comment(s)
Edit PR Review Bot Settings | Greptile
| public override void OnException(ExceptionContext context) | ||
| { | ||
| var errorMessage = "An error has occurred."; | ||
| var errorMessage = GetFormattedMessageFromErrorCode(context); |
There was a problem hiding this comment.
style: Consider adding a null check for context before calling GetFormattedMessageFromErrorCode
| var errorCode = alternativeErrorCode ?? context.Exception.Message; | ||
| var errorMessage = localizer[errorCode]; |
There was a problem hiding this comment.
logic: Using Exception.Message as errorCode could lead to unexpected behavior. Consider using a default error code instead
| if (errorMessage.ResourceNotFound is false) | ||
| { | ||
| return $"({errorCode}) {localizer[errorCode]}"; | ||
| } |
There was a problem hiding this comment.
logic: This logic might return '(ErrorMessage) ErrorMessage' for non-code errors. Consider refactoring to handle this case
| return $"({errorCode}) {localizer[errorCode]}"; | ||
| } | ||
|
|
||
| return context.Exception.Message; |
There was a problem hiding this comment.
logic: Returning Exception.Message as fallback may expose sensitive information. Consider using a generic error message
| if (user == null) | ||
| { | ||
| throw new BadRequestException("User not found."); | ||
| throw new BadRequestException(ErrorCodes.UserNotFound); |
There was a problem hiding this comment.
style: Consider using NotFoundException instead of BadRequestException for user not found scenario
| public static class ErrorCodes | ||
| { | ||
| public const string Error = "ERR000"; | ||
| public const string OrganizationNotFound = "ERR001"; | ||
| public const string OrganizationCannotUsePolicies = "ERR002"; | ||
| public const string UserNotFound = "ERR003"; | ||
| public const string ResourceNotFound = "ERR004"; | ||
| public const string InvalidToken = "ERR005"; | ||
| public const string Unauthorized = "ERR006"; | ||
| public const string PolicyRequiredByTrustedDeviceEncryption = "ERR007"; | ||
| public const string UnhandledError = "ERR500"; | ||
| } |
There was a problem hiding this comment.
style: Consider adding a comment explaining the purpose of this class and how it should be used
|
|
||
| public static class ErrorCodes | ||
| { | ||
| public const string Error = "ERR000"; |
There was a problem hiding this comment.
style: ERR000 seems too generic. Consider using a more specific name like 'GenericError' or 'UnspecifiedError'
| [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] | ||
| internal static System.Resources.ResourceManager ResourceManager { | ||
| get { | ||
| if (object.Equals(null, resourceMan)) { |
There was a problem hiding this comment.
style: Consider using 'resourceMan == null' instead of 'object.Equals(null, resourceMan)' for better readability and performance
| internal static string ERR000 { | ||
| get { | ||
| return ResourceManager.GetString("ERR000", resourceCulture); | ||
| } |
There was a problem hiding this comment.
style: ERR000 is placed out of order. Consider moving it to be the first error code for consistency
Type of change
Objective
This PR introduces centralized error management by creating a class with constant strings for expected exception error codes. It replaces hardcoded error messages with these error code constants, improving maintainability and consistency. Additionally, a resource file maps the error codes to their corresponding messages for easier localization and customization.
Code changes
Before you submit
dotnet format --verify-no-changes) (required)Greptile Summary
This pull request introduces a centralized error management system for the server API, focusing on consistent error handling and improved localization support.
ErrorCodesstatic class insrc/Core/Constants.cswith constant string fields for various error codesErrorMessages.en.resxresource file insrc/Core/Resources/to map error codes to localized messagesExceptionHandlerFilterAttribute.csto use the newGetFormattedMessageFromErrorCodemethod for error message localizationPolicyService.csandSyncController.csto replace hardcoded error messages withErrorCodesconstantsErrorMessages.en.Designer.csto provide programmatic access to localized error messages