[BEEEP] [PM-2844] Add custom error codes for server API exceptions #70

lizard-boy · 2024-10-19T09:08:51Z

Type of change

- [ ] Bug fix
- [ ] New feature development
- [X] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

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

file.ext: Description of what was changed and why

Before you submit

Please check for formatting errors (dotnet format --verify-no-changes) (required)
If making database changes - make sure you also update Entity Framework queries and/or migrations
Please add unit tests where it makes sense to do so (encouraged but not required)
If this change requires a documentation update - notify the documentation team
If this change has particular deployment requirements - notify the DevOps team

Greptile Summary

This pull request introduces a centralized error management system for the server API, focusing on consistent error handling and improved localization support.

Added ErrorCodes static class in src/Core/Constants.cs with constant string fields for various error codes
Created ErrorMessages.en.resx resource file in src/Core/Resources/ to map error codes to localized messages
Modified ExceptionHandlerFilterAttribute.cs to use the new GetFormattedMessageFromErrorCode method for error message localization
Updated PolicyService.cs and SyncController.cs to replace hardcoded error messages with ErrorCodes constants
Added ErrorMessages.en.Designer.cs to provide programmatic access to localized error messages

…ources file

…sage from resource files if the error thrown is an error code

…odes for exceptions

greptile-apps

_{7 file(s) reviewed, 15 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2024-10-19T09:09:11Z

src/Api/Utilities/ExceptionHandlerFilterAttribute.cs

@@ -19,7 +23,7 @@ public ExceptionHandlerFilterAttribute(bool publicApi)

    public override void OnException(ExceptionContext context)
    {
-        var errorMessage = "An error has occurred.";
+        var errorMessage = GetFormattedMessageFromErrorCode(context);


style: Consider adding a null check for context before calling GetFormattedMessageFromErrorCode

greptile-apps · 2024-10-19T09:09:11Z

src/Api/Utilities/ExceptionHandlerFilterAttribute.cs

+        var errorCode = alternativeErrorCode ?? context.Exception.Message;
+        var errorMessage = localizer[errorCode];


logic: Using Exception.Message as errorCode could lead to unexpected behavior. Consider using a default error code instead

greptile-apps · 2024-10-19T09:09:12Z

src/Api/Utilities/ExceptionHandlerFilterAttribute.cs

+        if (errorMessage.ResourceNotFound is false)
+        {
+            return $"({errorCode}) {localizer[errorCode]}";
+        }


logic: This logic might return '(ErrorMessage) ErrorMessage' for non-code errors. Consider refactoring to handle this case

greptile-apps · 2024-10-19T09:09:12Z

src/Api/Utilities/ExceptionHandlerFilterAttribute.cs

+            return $"({errorCode}) {localizer[errorCode]}";
+        }
+
+        return context.Exception.Message;


logic: Returning Exception.Message as fallback may expose sensitive information. Consider using a generic error message

greptile-apps · 2024-10-19T09:09:36Z

src/Api/Vault/Controllers/SyncController.cs

@@ -59,7 +60,7 @@ public async Task<SyncResponseModel> Get([FromQuery] bool excludeDomains = false
        var user = await _userService.GetUserByPrincipalAsync(User);
        if (user == null)
        {
-            throw new BadRequestException("User not found.");
+            throw new BadRequestException(ErrorCodes.UserNotFound);


style: Consider using NotFoundException instead of BadRequestException for user not found scenario

greptile-apps · 2024-10-19T09:09:53Z

src/Core/Constants.cs

+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";
+}


style: Consider adding a comment explaining the purpose of this class and how it should be used

greptile-apps · 2024-10-19T09:09:54Z

src/Core/Constants.cs

+
+public static class ErrorCodes
+{
+    public const string Error = "ERR000";


style: ERR000 seems too generic. Consider using a more specific name like 'GenericError' or 'UnspecifiedError'

greptile-apps · 2024-10-19T09:10:24Z

src/Core/Resources/ErrorMessages.en.Designer.cs

+        [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)]
+        internal static System.Resources.ResourceManager ResourceManager {
+            get {
+                if (object.Equals(null, resourceMan)) {


style: Consider using 'resourceMan == null' instead of 'object.Equals(null, resourceMan)' for better readability and performance

greptile-apps · 2024-10-19T09:10:24Z

src/Core/Resources/ErrorMessages.en.Designer.cs

+        internal static string ERR000 {
+            get {
+                return ResourceManager.GetString("ERR000", resourceCulture);
+            }


style: ERR000 is placed out of order. Consider moving it to be the first error code for consistency

r-tome added 4 commits July 28, 2023 15:14

[PM-2844] Created ErrorMessages resource file

d5d5fe0

[PM-2844] Created constants class with error codes that match the res…

d9f65bc

…ources file

[PM-2844] Updated ExceptionHandlerFilterAttribute to obtain error mes…

e1f20ad

…sage from resource files if the error thrown is an error code

[PM-2844] Updated SyncController and PolicyService to use some ErrorC…

b3e41cd

…odes for exceptions

greptile-apps bot reviewed Oct 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEEEP] [PM-2844] Add custom error codes for server API exceptions #70

[BEEEP] [PM-2844] Add custom error codes for server API exceptions #70

lizard-boy commented Oct 19, 2024 •

edited by greptile-apps bot

Loading

greptile-apps bot left a comment

greptile-apps bot Oct 19, 2024

greptile-apps bot Oct 19, 2024

greptile-apps bot Oct 19, 2024

greptile-apps bot Oct 19, 2024

greptile-apps bot Oct 19, 2024

greptile-apps bot Oct 19, 2024

greptile-apps bot Oct 19, 2024

greptile-apps bot Oct 19, 2024

greptile-apps bot Oct 19, 2024

		var errorCode = alternativeErrorCode ?? context.Exception.Message;
		var errorMessage = localizer[errorCode];

[BEEEP] [PM-2844] Add custom error codes for server API exceptions #70

Are you sure you want to change the base?

[BEEEP] [PM-2844] Add custom error codes for server API exceptions #70

Conversation

lizard-boy commented Oct 19, 2024 • edited by greptile-apps bot Loading

Type of change

Objective

Code changes

Before you submit

Greptile Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 19, 2024

Choose a reason for hiding this comment

lizard-boy commented Oct 19, 2024 •

edited by greptile-apps bot

Loading