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

Error codes for each error variant in Core/Validator #184

Open
2 tasks
cdisselkoen opened this issue Jul 14, 2023 · 4 comments
Open
2 tasks

Error codes for each error variant in Core/Validator #184

cdisselkoen opened this issue Jul 14, 2023 · 4 comments
Assignees
Labels
backlog We hope to work on this in the future feature-request Request for a new feature work-in-progress Implementation work in progress

Comments

@cdisselkoen
Copy link
Contributor

Category

User level API changes

Describe the feature you'd like to request

Every error variant (eg PolicySetError::ExpectedStatic) should have an assigned error code, which shows up in both the error message and the rustdoc for that variant. We should systematically assign these error codes, maybe including letters and/or numbers.

Describe the solution you'd like

I'm happy to take arguments about what the error codes should look like, but here's an initial proposal:

  • First letter of the error code indicates a general category: V for validation-related errors, A for errors thrown by is_authorized(), etc. (Not sure exactly how many categories we should have.)
  • Within each category, we can have 100-series codes be a subcategory, 200-series codes another subcategory, etc
  • For example, A100-series codes could correspond to variants of EvaluationError

miette provides nice way to expose these by adding code(...) to the #[diagnostic(...)] attribute.

miette recommends error codes of a form like cedar_policy_core::parse::parse_err. But there's nothing saying we have to obey that. We could also combine this with the above, e.g., cedar::A100 or cedar_policy::A100.

This should not be a breaking change, it should only be a change to error messages and docstrings.

Describe alternatives you've considered

We could instead just have textual error codes that are basically the error struct and variant name. This gives us a little less flexibility to refactor our error structs in the future. I'm not sure whether our users would prefer to look up (or Google) a code like "A100" or a textual code like "EvaluationError::IntegerOverflow".

Additional context

No response

Is this something that you'd be interested in working on?

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change
@cdisselkoen cdisselkoen added pending-triage Hasn't been triaged yet feature-request Request for a new feature labels Jul 14, 2023
@andrewmwells-amazon andrewmwells-amazon added backlog We hope to work on this in the future and removed pending-triage Hasn't been triaged yet labels Jul 14, 2023
@cdisselkoen
Copy link
Contributor Author

We should do this after #182

@sarahcec sarahcec added the 3.1 Features for 3.1 label Jan 2, 2024
@khieta khieta added 3.2 Features for 3.2 and removed 3.1 Features for 3.1 labels Feb 6, 2024
@shaobo-he-aws shaobo-he-aws self-assigned this Mar 15, 2024
@shaobo-he-aws shaobo-he-aws added the work-in-progress Implementation work in progress label Mar 21, 2024
@shaobo-he-aws
Copy link
Contributor

After discussion, we agreed that the error codes are namespaced with the last component being a descriptive identifier. For instance, an type error raised by the authorizer is evaluation::type_error.

@cdisselkoen
Copy link
Contributor Author

In that case, it might make sense to pause this until #745 is done (in case that would end up changing some of the proposed codes).

@shaobo-he-aws
Copy link
Contributor

We decided to implement this feature after version 4.0.

@shaobo-he-aws shaobo-he-aws removed the 3.2 Features for 3.2 label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to work on this in the future feature-request Request for a new feature work-in-progress Implementation work in progress
Projects
None yet
Development

No branches or pull requests

5 participants