Improve error handling #21
Conversation
Signed-off-by: Rainer Schoenberger <[email protected]>
Signed-off-by: Rainer Schoenberger <[email protected]>
rainerschoe
force-pushed
the
2024-12-06_error_handling
branch
from
5d69ba2 to
c9b346f
Compare
| .into_iter() | ||
| .find(|targeting_rule| targeting_rule_applies_to_entity(segments, targeting_rule, entity)) | ||
|
|
||
| for targeting_rule in targeting_rules.into_iter(){ |
There was a problem hiding this comment.
I failed to find a nice way of propagating errors out of iterator chains. So in many cases like this I needed to fall-back to explicit loops. Any ideas?
| if targeting_rule_applies_to_entity(segments, &targeting_rule, entity) | ||
| .map_err( | ||
| |e|{ | ||
| // This terminates the use of anyhow in this module, converting all errors: |
There was a problem hiding this comment.
This terminates anyhow use, We might lose some information (stacktrace) here.
| url = "2.5.2" | ||
| http = "1.1.0" | ||
| thiserror = "2.0.4" | ||
| anyhow = "1.0.94" |
There was a problem hiding this comment.
We don't want anyhow as a dependency for the library
| // For chaining errors creating useful error messages: | ||
| use anyhow::{Context, Result as AnyhowResult}; | ||
| use anyhow::anyhow; |
There was a problem hiding this comment.
We don't really want anyhow in library code. It hides most of information about the actual error. Let's reserve anyhow for application.
| } | ||
| crate::models::ValueKind::Boolean => { | ||
| Value::Boolean(model_value.0.as_bool().ok_or(Error::ProtocolError)?) | ||
| Value::Boolean(model_value.0.as_bool().ok_or(Error::ProtocolError("Expected Boolean".into()))?) |
There was a problem hiding this comment.
If we want to, later we can add more typed information (ErrorBooleanExpected, ErrorStringExpected) so the string message is just for humans... and we can also include what was the value we were trying to parse as boolean.
…hey pass Signed-off-by: Rainer Schoenberger <[email protected]>
rainerschoe
force-pushed
the
2024-12-06_error_handling
branch
from
4bee4ec to
c15af55
Compare
unwrap,panicandexpectusesI experimented using
anyhowto create error chains providing the user with more readable error messages.For now I terminate
anyhowusage in segment evaluation code and use thethiserrortypes for everything user-facing.Feedback appreciated.
The behavior differs from the go implementation in strictness. I'd appreciate Input from @saikumar1607 on what is the intended behavior for error handling in entity evaluation. Is it fine to have the strict enforcement as implemented here?