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

refactor: Overuse of validation_error - Proposal for More Specialized Error Codes #207

Open
adia-dev opened this issue Oct 23, 2024 · 0 comments

Comments

@adia-dev
Copy link

Hi OpenFGA team 👋🏿 ,

I’ve noticed that the validation_error code is being used quite broadly, almost as a default or fallback error, in situations where more specific error codes would be much more appropriate. This generalization can make it harder to understand what went wrong without having to read/parse the error message.

Issue:
The overuse of validation_error where more specialized codes would be clearer creates challenges, especially for client systems that rely on distinct error codes to handle different types of issues. It becomes difficult to distinguish between different kinds of failures, as they’re all grouped under the same generic error.

After looking through the codebase, I realized that there are already specific error codes defined for many cases, but they aren’t consistently being used. As a result, various types of errors (e.g., format issues, type not found, invalid inputs...) are all returned under validation_error, when they could—and should—be handled with more appropriate error codes.

Examples:

  1. StoreId Format Error:

    Right now, when the StoreId format is incorrect, we get:

    {
        "code": "validation_error",
        "message": "invalid CheckRequest.StoreId: value does not match regex pattern \"^[ABCDEFGHJKMNPQRSTVWXYZ0-9]{26}$\""
    }

    A more specific code like store_id_invalid_pattern would be clearer and easier to handle:

    {
        "code": "store_id_invalid_pattern",
        "message": "invalid CheckRequest.StoreId: value does not match regex pattern \"^[ABCDEFGHJKMNPQRSTVWXYZ0-9]{26}$\""
    }
  2. Type Not Found:

    When a type isn’t found in the model when performing a check request, the current error is:

    {
        "code": "validation_error",
        "message": "type 'manga' not found"
    }

    It would be more helpful to return something like type_not_found:

    {
        "code": "type_not_found",
        "message": "type 'manga' not found"
    }

Proposed Solution:
I suggest updating the codebase to return more specialized error codes wherever applicable, rather than defaulting to validation_error. This change would make error handling much more precise for both clients and developers, without having to rely on parsing messages to figure out what went wrong.

Benefits:

  • Clarity and Actionability: Specific error codes would give clear insight into the issue, making it easier to debug.
  • Improved Developer Experience: Developers would be able to quickly identify and address specific problems without sifting through error messages.
  • Better Client-Side Handling: Clients that rely on error codes to handle retries, fallbacks, or other conditional logic would benefit from more reliable and targeted error codes.

Potential Drawback:

  • Breaking Change: This update would technically be a breaking change, as many errors currently using validation_error would be assigned new, more specific codes. The change would need to be communicated in the release notes and included in a migration guide.

Implementation:
I’ve already tested this locally by modifying some areas to return more specialized error codes, and it instantly made things clearer and easier to work with.

Next Steps:
I’m happy to help make these changes by submitting a PR if this aligns with your team’s priorities. However, I also understand if it’s more efficient for the team to implement this directly as I would have to do it in parallel with my day job.

Looking forward to your thoughts!

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

No branches or pull requests

1 participant