Implement #745 for api::SchemaError
#876
Conversation
Signed-off-by: Shaobo He <shaobohe@amazon.com>
shaobo-he-aws
requested review from
cdisselkoen,
john-h-kastner-aws,
andrewmwells-amazon,
khieta and
aaronjeline
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
cedar-policy/src/api/err.rs
Outdated
| pub fn get_node_in_cycle(&self) -> SmolStr { | ||
| self.node.to_smolstr() | ||
| } |
There was a problem hiding this comment.
Would it make sense to return the EntityUid here?
There was a problem hiding this comment.
I think we'd better not expose implementation details here, which include what nodes are.
There was a problem hiding this comment.
I think it's sensible for a CycleInActionHierarchyError to have a getter which tells users the EntityUid of the action that was in the cycle. I agree we shouldn't expose implementation details about nodes, but I think returning an EntityUid is much more helpful to users than a string.
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
|
Pushed a new design.
A drawback of this design is that we can't write precise getter APIs for sub-error types. For instance, entity UIDs have different types in |
cedar-policy-validator/src/err.rs
Outdated
| /// Error thrown by the `serde_json` crate during serialization | ||
| #[error(transparent)] | ||
| #[diagnostic(transparent)] | ||
| // no #[from], because if you just have a serde_json::Error you should choose between JsonDeserialization and JsonSerialization appropriately |
There was a problem hiding this comment.
Should the JsonDeserialization variant also not have #[from]?
There was a problem hiding this comment.
Good catch. Fixed.
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Description of changes
This PR also changes the signature of the validator's
SchemaError.Issue #, if available
#745
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy(e.g., changes to the signature of an existing API).I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec(choose one, and delete the other options):