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

fix fetch token price to Binance API-2 #526

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Conversation

rubenmarcus
Copy link
Member

@rubenmarcus rubenmarcus commented Jul 29, 2024

PR Type

Bug fix, Tests


Description

  • Refactored the nearPrice function to consolidate API fetch logic into a single fetchPrice function.
  • Improved error handling by adding fallback logic to try the secondary API if the primary API call fails.
  • Enhanced test coverage by updating test cases to handle individual API failures and resetting mocks before each test.
  • Added logging to provide more insight into API call failures.

Changes walkthrough 📝

Relevant files
Tests
nearPrice.test.ts
Update nearPrice tests for improved API failure handling 

packages/data/src/api/nearPrice/nearPrice.test.ts

  • Added beforeEach to reset mocks before each test.
  • Modified test cases to handle individual API failures.
  • Updated mock responses to match expected structures.
  • +20/-8   
    Bug fix
    nearPrice.ts
    Refactor and improve error handling in nearPrice API         

    packages/data/src/api/nearPrice/nearPrice.ts

  • Refactored API fetch functions into a single fetchPrice function.
  • Implemented fallback logic to handle primary API failure.
  • Added logging for API call failures.
  • +11/-16 

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

    @mintbase-codium-pr-agent
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Error Handling
    The error handling in the fetchPrice function might not correctly handle network or parsing errors that do not throw exceptions but result in invalid data. Consider adding checks for the response status and parsed data validity.

    Test Coverage
    Ensure that the tests cover scenarios where the fetch function returns a non-exception error, such as a network error or a non-200 status code, which are common in real-world scenarios.

    @mintbase-codium-pr-agent
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Error handling
    Add error handling for asynchronous operations

    Add error handling for the asynchronous call to nearPrice() to ensure that errors
    are properly managed and not just undefined.

    packages/data/src/api/nearPrice/nearPrice.test.ts [20]

    -const { data, error } = await nearPrice();
    +let data, error;
    +try {
    +  ({ data, error } = await nearPrice());
    +} catch (e) {
    +  error = e;
    +}
     
    Suggestion importance[1-10]: 9

    Why: Adding error handling for asynchronous calls is crucial for robust code. This suggestion ensures that errors are properly managed, improving the reliability and debuggability of the tests.

    9
    Improve error logging for better debugging and maintenance

    Refactor the error handling in the catch block to log more informative messages and
    handle specific error types if possible.

    packages/data/src/api/nearPrice/nearPrice.ts [21]

    -console.log('First API call failed, trying second API', err);
    +if (err instanceof NetworkError) {
    +  console.error('Network error occurred:', err.message);
    +} else {
    +  console.error('Unexpected error occurred:', err);
    +}
     
    Suggestion importance[1-10]: 8

    Why: Enhanced error logging provides more informative messages, aiding in debugging and maintenance. This is a significant improvement for understanding and resolving issues quickly.

    8
    Maintainability
    Replace hardcoded values with variables for better maintainability

    Replace the hardcoded '123' string with a variable or constant to avoid duplication
    and improve maintainability.

    packages/data/src/api/nearPrice/nearPrice.test.ts [21]

    -expect(data).toBe('123');
    +const expectedPrice = '123';
    +expect(data).toBe(expectedPrice);
     
    Suggestion importance[1-10]: 7

    Why: This suggestion improves maintainability by avoiding hardcoded values, making the code easier to update and understand. However, it is a minor improvement and does not address any critical issues.

    7
    Best practice
    Use descriptive names for function parameters

    Use a more descriptive function name for the tokenPrice parameter in fetchPrice to
    clarify its purpose.

    packages/data/src/api/nearPrice/nearPrice.ts [19]

    -const res = await fetchPrice<NearPriceData>(BINANCE_API, data => data)
    +const res = await fetchPrice<NearPriceData>(BINANCE_API, extractPriceFromData)
     
    Suggestion importance[1-10]: 6

    Why: Using more descriptive names for function parameters improves code readability and maintainability. While beneficial, this change is relatively minor and does not address any functional issues.

    6

    Copy link
    Member

    @sainthiago sainthiago left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    🙏

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

    Successfully merging this pull request may close these issues.

    2 participants