Skip to content
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

refactor(katana): make gas oracle a critical task #2859

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

kariy
Copy link
Member

@kariy kariy commented Jan 2, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling and logging in the gas oracle worker
    • Updated node stopping mechanism to better handle potential errors
  • Refactor

    • Enhanced error reporting and log severity for critical system components

The changes focus on improving system robustness and error management, with specific updates to logging and error handling in core infrastructure components.

Copy link

coderabbitai bot commented Jan 2, 2025

Walkthrough

Ohayo, sensei! The pull request introduces modifications to error handling and logging in the Katana project. The changes focus on improving error reporting in the L1GasOracle implementation and updating the node stopping mechanism. The modifications enhance the robustness of error logging and streamline the process of stopping a node by directly handling the stop method's result.

Changes

File Change Summary
crates/katana/core/src/backend/gas_oracle.rs - Replaced info logging with error logging
- Updated task spawning to use critical() instead of graceful_shutdown()
- Improved error logging with error! macro and specific target
- Minor error variable naming improvements
crates/katana/node/src/exit.rs - Modified NodeStoppedFuture::new() method
- Replaced stopped() method call with stop() method
- Updated error handling for node stopping process

Sequence Diagram

sequenceDiagram
    participant GasOracle
    participant TaskSpawner
    participant Logger

    GasOracle->>TaskSpawner: Run worker
    TaskSpawner-->>Logger: Log critical errors
    alt Error Occurs
        TaskSpawner->>Logger: Log detailed error
    end

    Note over GasOracle, Logger: Enhanced Error Reporting
Loading

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ccea4b and 06665e1.

📒 Files selected for processing (2)
  • crates/katana/core/src/backend/gas_oracle.rs (3 hunks)
  • crates/katana/node/src/exit.rs (1 hunks)
🔇 Additional comments (6)
crates/katana/node/src/exit.rs (1)

21-21: Ohayo sensei! Good enhancement in stopping the node gracefully.
Switching from an awaited stopped() call to invoking handle.rpc.stop()? directly streamlines error handling by returning a Result. Just make sure any up-chain callers properly handle the error, if thrown.

crates/katana/core/src/backend/gas_oracle.rs (5)

12-12: Ohayo sensei! Import of error macro is on point.
This import clarifies where the error macro is coming from and ensures consistency in the project's logging approach.


86-86: Ohayo sensei! Confirm the usage of .critical() here.
Switching from a possibly more graceful approach to .critical() ensures the worker task is treated with high priority, but confirm there's an appropriate process handling if the worker fails.


89-92: Ohayo sensei! Excellent use of .inspect_err(...).
This approach elegantly captures errors in the async block. Keep verifying that no critical error escapes unhandled, potentially blocking the system.


180-180: Ohayo sensei! Handling update_gas_price failure is crucial.
The if let Err(error) = ... approach ensures we won’t silently ignore issues. Good job capturing these scenarios.


186-186: Ohayo sensei! Logging errors with specific target is helpful.
Using error!(target: "gas_oracle", ...) improves clarity for debugging. Keep it consistent in your logs for easier correlation of issues.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@kariy kariy changed the title chore(katana): log out error when gas oracle fail refactor(katana): make gas oracle as critical task Jan 2, 2025
@kariy kariy changed the title refactor(katana): make gas oracle as critical task refactor(katana): make gas oracle a critical task Jan 2, 2025
Copy link

codecov bot commented Jan 2, 2025

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 55.83%. Comparing base (3ccea4b) to head (06665e1).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/katana/core/src/backend/gas_oracle.rs 0.00% 7 Missing ⚠️
crates/katana/node/src/exit.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2859   +/-   ##
=======================================
  Coverage   55.83%   55.83%           
=======================================
  Files         446      446           
  Lines       57725    57728    +3     
=======================================
+ Hits        32228    32232    +4     
+ Misses      25497    25496    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant