-
Notifications
You must be signed in to change notification settings - Fork 20
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 handling refactor #1280
Error handling refactor #1280
Conversation
cbce3c5
to
0cb760e
Compare
0cb760e
to
b12ac5c
Compare
b12ac5c
to
63e993b
Compare
EncodeProto(#[from] prost::EncodeError), | ||
#[error("proto decode error: {0}")] | ||
DecodeProto(#[from] prost::DecodeError), | ||
#[error(transparent)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: in the previous code we have this comment:
#[error("intent error: {0}")] Intent(#[from] IntentError),
is this change intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably be doing more transparent error wrapping, since the additional comments usually add little incremental value. But this one was an accident, and that refactor could be handled separately.
63e993b
to
3a3d31b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup🧹🙌
3a3d31b
to
70cf5c5
Compare
tl;dr
MessageProcessingError
intoGroupMessageProcessingError
that is more narrowly scopedGroupError
to handle errors when processing Welcomesprocess_for_id
take a generic error type to support bothRetryableError
explicit and don't use_ => false