-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: don't stop the pipeline on internal stage errs #453
Conversation
Codecov Report
@@ Coverage Diff @@
## main #453 +/- ##
=======================================
Coverage ? 74.09%
=======================================
Files ? 239
Lines ? 23225
Branches ? 0
=======================================
Hits ? 17208
Misses ? 6017
Partials ? 0
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Discussion on whether |
9a66b07
to
5a8ad6c
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.
LGTM - nits.
@@ -21,6 +21,7 @@ pub enum StageError { | |||
Database(#[from] DbError), | |||
#[error("Stage encountered a execution error in block {block}: {error}.")] | |||
/// The stage encountered a execution error | |||
// TODO: Probably redundant, should be rolled into `Validation` |
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.
agreed
@@ -28,6 +28,7 @@ pub struct SendersStage { | |||
pub commit_threshold: u64, | |||
} | |||
|
|||
// TODO(onbjerg): Should unwind |
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.
In which cases?
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.
All cases I think. If we can't recover the sender then the transaction is invalid (assuming our recover algo is not broken)
Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>
Differentiate between fatal errors that should stop the pipeline because no progress can be made, and errors that are recoverable. Stages should feel free to emit errors to make unit testing them easier, and the pipeline should catch them, evaluate if they are fatal or not and act accordingly