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

[PLA-1986] Support to parse storage during migration #63

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

leonardocustodio
Copy link
Contributor

@leonardocustodio leonardocustodio commented Sep 10, 2024

PR Type

enhancement, error handling


Description

  • Enhanced the tankStorageData and fuelTankAccountStorageData methods by adding try-catch blocks to handle exceptions during the decoding process.
  • Updated the decoding logic to first attempt using the 'V1010' version of the data format, providing a fallback to the previous version if an exception occurs.

Changes walkthrough 📝

Relevant files
Error handling
Decoder.php
Add exception handling for data decoding in storage methods

src/Services/Processor/Substrate/Codec/Decoder.php

  • Added try-catch blocks to handle exceptions during data decoding.
  • Modified tankStorageData method to attempt decoding with
    'TankStorageDataV1010' first.
  • Modified fuelTankAccountStorageData method to attempt decoding with
    'FuelTankAccountStorageDataV1010' first.
  • +10/-2   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Exception Handling
    The try-catch blocks added for decoding data do not log or handle the exception beyond fallback decoding. Consider adding logging or more specific error handling to diagnose issues better.

    Exception Handling
    Similar to the previous issue, the exception handling for fuelTankAccountStorageData does not include error logging or detailed handling, which might be necessary for troubleshooting and maintenance.

    Copy link

    github-actions bot commented Sep 10, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add error logging within the exception handling to aid in debugging

    Consider adding logging for the exception handling to record the error details. This
    can help in monitoring and troubleshooting issues during the decoding process.

    src/Services/Processor/Substrate/Codec/Decoder.php [71-74]

     try {
         $decoded = $this->codec->process('FuelTankAccountStorageDataV1010', new ScaleBytes($data));
    -} catch (\Exception) {
    +} catch (\Exception $e) {
    +    error_log('Error decoding data: ' . $e->getMessage());
         $decoded = $this->codec->process('FuelTankAccountStorageData', new ScaleBytes($data));
     }
     
    Suggestion importance[1-10]: 8

    Why: Adding logging to the exception handling block is a practical enhancement that aids in monitoring and troubleshooting by capturing error details, which is beneficial for maintaining the system.

    8
    Best practice
    Replace generic exception handling with specific exception types for better error management

    Replace the generic \Exception catch block with more specific exception types to
    handle different error scenarios more effectively. This will help in debugging and
    maintaining the code by providing clearer insights into what types of errors are
    expected and how they are handled.

    src/Services/Processor/Substrate/Codec/Decoder.php [31-34]

     try {
         $decoded = $this->codec->process('TankStorageDataV1010', new ScaleBytes($data));
    -} catch (\Exception) {
    +} catch (\SpecificExceptionType $e) {
    +    // Handle specific exception
    +    $decoded = $this->codec->process('TankStorageData', new ScaleBytes($data));
    +} catch (\AnotherExceptionType $e) {
    +    // Handle another type of exception
         $decoded = $this->codec->process('TankStorageData', new ScaleBytes($data));
     }
     
    Suggestion importance[1-10]: 7

    Why: Using specific exception types instead of a generic \Exception can improve error handling and debugging by providing more detailed information about the errors. However, the suggestion does not specify which exceptions to catch, which limits its immediate applicability.

    7

    @leonardocustodio leonardocustodio merged commit da66074 into master Sep 10, 2024
    5 of 7 checks passed
    @leonardocustodio leonardocustodio deleted the PLA-1986 branch September 10, 2024 01:53
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Development

    Successfully merging this pull request may close these issues.

    2 participants