feat(katana): exit on block production error #2629
Conversation
WalkthroughOhayo, sensei! This pull request introduces several modifications primarily focused on error handling within the block production process. The Changes
Possibly related PRs
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
crates/katana/pipeline/src/stage/sequencing.rs (2)
Line range hint
55-62: Excellent error handling improvement, sensei!The change to return
Result<(), BlockProductionError>aligns perfectly with the goal of explicit error handling during block production.Consider adding a debug log before spawning the task to help with debugging:
fn run_block_production(&self) -> TaskHandle<Result<(), BlockProductionError>> { let pool = self.pool.clone(); let miner = TransactionMiner::new(pool.add_listener()); let block_producer = self.block_producer.clone(); let service = BlockProductionTask::new(pool, miner, block_producer); + tracing::debug!(target: "pipeline", "Spawning block production task"); self.task_spawner.build_task().name("Block production").spawn(service) }
Line range hint
82-89: Enhance error logging for block production failures, sensei!The current error logging could be more specific for block production errors to better serve the PR's goal.
Consider this improvement:
res = messaging => { error!(target: "pipeline", reason = ?res, "Messaging task finished unexpectedly."); }, res = block_production => { - error!(target: "pipeline", reason = ?res, "Block production task finished unexpectedly."); + match res { + Ok(Ok(())) => error!(target: "pipeline", "Block production task finished unexpectedly with success."), + Ok(Err(e)) => error!(target: "pipeline", error = ?e, "Block production failed with error."), + Err(e) => error!(target: "pipeline", error = ?e, "Block production task panicked."), + } }This change provides:
- Clear distinction between task failures and block production errors
- Better error context for debugging
- Proper handling of both Result layers (TaskHandle and BlockProductionError)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
crates/katana/core/src/service/block_producer.rs(0 hunks)crates/katana/core/src/service/mod.rs(3 hunks)crates/katana/pipeline/src/stage/sequencing.rs(2 hunks)crates/katana/storage/db/src/mdbx/tx.rs(1 hunks)
💤 Files with no reviewable changes (1)
- crates/katana/core/src/service/block_producer.rs
🔇 Additional comments (5)
crates/katana/pipeline/src/stage/sequencing.rs (2)
6-6: Ohayo! Clean import addition, sensei!
The BlockProductionError import is properly placed with related block_producer imports.
Line range hint 55-89: Verify storage error handling, sensei!
The PR objectives mention issues with running out of storage. Let's verify that storage-related errors are properly propagated through BlockProductionError.
✅ Verification successful
Storage error handling is properly implemented, sensei!
The BlockProductionError correctly propagates storage-related errors through ProviderError, which handles various storage scenarios including:
- Missing block data (hash/number)
- Parsing errors
- General provider errors
The implementation properly handles these through the error chain:
- Storage layer errors -> ProviderError
- ProviderError -> BlockProductionError (via #[from] derive)
- BlockProductionError -> Stage error handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for storage-related error definitions and handling
echo "Checking BlockProductionError definition for storage errors..."
rg -A 5 "pub enum BlockProductionError"
echo "Checking storage error handling in block production..."
ast-grep --pattern 'match $_ {
$$$
Err($_) => $$$
$$$
}'
Length of output: 831
Script:
#!/bin/bash
# Let's check for storage-related error handling in the provider
echo "Checking ProviderError definition..."
rg -A 10 "pub enum ProviderError"
# Check block producer implementation for storage handling
echo "Checking block producer implementation..."
ast-grep --pattern 'impl BlockProducer {
$$$
fn $_($$$) -> Result<$_, $_> {
$$$
}
$$$
}'
# Check if there are any storage-related modules
echo "Checking for storage modules..."
fd -t f "storage.*\.rs$"
Length of output: 3007
crates/katana/core/src/service/mod.rs (3)
8-8: Importing BlockProductionError for enhanced error handling
Ohayo, sensei! Including use block_producer::BlockProductionError; is a necessary addition. This import ensures that the BlockProductionError type is available for the improved error propagation in the block production task.
51-51: Changing Output type to Result<(), BlockProductionError>
Ohayo, sensei! Updating the Output type in the Future implementation to Result<(), BlockProductionError> is a wise move. This change allows the BlockProductionTask to return errors, enabling better error handling upstream.
72-72: Returning errors immediately upon block production failure
Ohayo, sensei! Modifying the poll method to return Poll::Ready(Err(error)) ensures that the task exits promptly when a block production error occurs. This adjustment prevents silent failures and enhances the robustness of the service.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2629 +/- ##
==========================================
- Coverage 56.91% 56.89% -0.03%
==========================================
Files 397 397
Lines 49466 49467 +1
==========================================
- Hits 28154 28144 -10
- Misses 21312 21323 +11 ☔ View full report in Codecov by Sentry. |
resolves #1650
currently, katana silently fails when an error happen during block production. this create some issues in the context of Slot hosting as unexpected behaviours can happen when the the instance is running out of storage. returning immediately upon failing makes it easier to detect this issue
Summary by CodeRabbit
BlockProductionError, improving error handling in block production tasks.putmethod to prevent potential panics and provide better context for database write errors.