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

chore: improve error message when tempStorage kind doesn't match #1882

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

marcospb19-cw
Copy link
Contributor

@marcospb19-cw marcospb19-cw commented Nov 25, 2024

PR Type

Enhancement


Description

  • Improved error handling for unknown temporary storage kinds by providing a more informative error message that includes valid options.
  • Enhanced the TemporaryStorageKind enum with strum derive macros (Display, VariantNames) for better variant handling and string conversion.
  • Added Copy and Parser trait implementations to TemporaryStorageKind for improved usability.
  • Introduced the strum crate dependency to leverage its functionality for enum operations.

Changes walkthrough 📝

Relevant files
Enhancement
mod.rs
Enhance TemporaryStorageKind enum and improve error handling

src/eth/storage/temporary/mod.rs

  • Added strum crate dependency for enum variant handling
  • Enhanced TemporaryStorageKind enum with strum derive macros
  • Improved error message for unknown temporary storage kind
  • Added Copy and Parser trait implementations for TemporaryStorageKind
  • +4/-2     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling
    The error message for unknown temporary storage kinds now includes valid options. Ensure this doesn't expose sensitive information and that the error is properly logged with the original input.

    Enum Derivation
    The TemporaryStorageKind enum now derives several new traits. Verify that these derivations are necessary and don't introduce any unintended side effects or performance impacts.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Improve the readability of error messages by presenting valid options in a more user-friendly format

    Consider using Self::VARIANTS.join(", ") instead of {:?} to display the valid values
    as a comma-separated list without quotes and brackets.

    src/eth/storage/temporary/mod.rs [105]

    -s => Err(anyhow!("unknown temporary storage kind: \"{}\" - valid values are {:?}", s, Self::VARIANTS)),
    +s => Err(anyhow!("unknown temporary storage kind: \"{}\" - valid values are {}", s, Self::VARIANTS.join(", "))),
    Suggestion importance[1-10]: 7

    Why: The suggestion improves error message clarity by presenting valid options in a more readable format. This enhances user experience and debugging, making it easier to understand available choices.

    7

    @marcospb19-cw marcospb19-cw merged commit 84f5d3d into main Nov 25, 2024
    34 checks passed
    @marcospb19-cw marcospb19-cw deleted the improve-error-message branch November 25, 2024 14:20
    @gabriel-aranha-cw
    Copy link
    Contributor

    Final benchmark:
    RPS Stats: Max: 1123.00, Min: 621.00, Avg: 896.72, StdDev: 68.83
    TPS Stats: Max: 1049.00, Min: 709.00, Avg: 869.22, StdDev: 70.37
    Plot: View Plot

    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.

    2 participants