Use detailed error - Remove anyhow #25
Conversation
Signed-off-by: Javier G. Sogo <[email protected]>
Signed-off-by: Javier G. Sogo <[email protected]>
src/errors.rs
Outdated
| #[derive(Debug, Error)] | ||
| #[error("Operation '{}' '{}' '{}' failed to evaluate: {}", segment_rule.attribute_name, segment_rule.operator, value, source)] | ||
| pub(crate) struct SegmentEvaluationErrorKind { | ||
| pub(crate) segment: Segment, | ||
| pub(crate) segment_rule: SegmentRule, | ||
| pub(crate) value: String, | ||
| pub(crate) source: CheckOperatorErrorDetail, | ||
| } |
There was a problem hiding this comment.
We should be more specific with the data we are using here and store just what we need, otherwise this object is too big.
There was a problem hiding this comment.
yes, I think for the segment and segment_rule probably the IDs are sufficient. -> follow up PR
| // let msg = rule.unwrap_err().to_string(); | ||
| // assert!(msg.contains("'a2'")); | ||
| // assert!(msg.contains("'0'")); | ||
| // assert!(msg.contains("'non_existing_segment_id'")); | ||
| // assert!(msg.contains("not found")); |
There was a problem hiding this comment.
In the error types chain we are missing the entity
There was a problem hiding this comment.
yes, I wanted to add it, but then thought, let's not over-engineer it. probably the user will know which entity he is evaluating. so maybe it is not required in the error
src/errors.rs
Outdated
| #[error(transparent)] | ||
| SegmentEvaluationFailed(#[from] SegmentEvaluationErrorKind), |
There was a problem hiding this comment.
I think we can declare this as follows, and remove one indirection
| #[error(transparent)] | |
| SegmentEvaluationFailed(#[from] SegmentEvaluationErrorKind), | |
| #[error("Operation '{}' '{}' '{}' failed to evaluate: {}", segment_rule.attribute_name, segment_rule.operator, value, source)] | |
| SegmentEvaluationFailed { | |
| pub(crate) segment: Segment, | |
| pub(crate) segment_rule: SegmentRule, | |
| pub(crate) value: String, | |
| pub(crate) source: CheckOperatorErrorDetail, | |
| }, |
There was a problem hiding this comment.
We tried this yesterday, but it did not work, as we are not allowed to add visibility qualifiers into enum declaration :-(
| else { | ||
| panic!("Error type mismatch!"); | ||
| }; | ||
| assert_eq!(error.segment.name, ""); |
There was a problem hiding this comment.
Not sure why this is empty (we have a Segment object). Is name actually empty?
There was a problem hiding this comment.
I was going with segment ID for debugging always. in the unit-tests the name is empty string. only in the "real" json test data it is set.
Signed-off-by: Javier G. Sogo <[email protected]>
Signed-off-by: Javier G. Sogo <[email protected]>
|
@rainerschoe , I've hidden the errors into a |
Co-authored-by: Rainer Schoenberger <[email protected]> Signed-off-by: Javier G. Sogo <[email protected]>
No description provided.