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

Enhance parse_response Method to Improve Error Handling and Logging #153

Merged

Conversation

armando-rodriguez-cko
Copy link
Contributor

@armando-rodriguez-cko armando-rodriguez-cko commented Nov 18, 2024

Description

This pull request introduces significant improvements and fixes to the Checkout Ruby SDK, focusing on error handling, response parsing, and code readability.

Key Changes

  1. Refactored Method Definitions:

    • Streamlined method signatures to use a single-line format for better readability.
  2. Enhanced Error Handling:

    • Updated CheckoutApiException to include detailed error messages and structured error details.
    • Improved parse_error_details to gracefully handle invalid JSON and added robust logging for unexpected exceptions.
  3. Improved Response Parsing:

    • Added parse_body to manage responses based on their content type (JSON, CSV, or plain text).
    • Simplified parse_response logic to handle various response formats and edge cases effectively.
  4. Added Comprehensive Tests:

    • New test cases in api_client_spec.rb cover:
      • Valid and invalid JSON responses.
      • Error responses with 4xx and 5xx status codes.
      • Parsing of response metadata and body content.
  5. New Klarna Payment Context Handling:

    • Integrated create_payment_contexts_klarna in the helper for better Klarna-related test coverage.
  6. General Improvements:

    • Refactored headers management for consistency.
    • Updated build_multipart_request to improve parameter formatting and consistency.
    • Cleaned up test files, including contexts_integration_spec.rb and reports_integration_spec.rb.

Motivation

This PR addresses an identified issue where error responses (HTTP status code > 400) were not logged, creating challenges for debugging. It aims to enhance the SDK's robustness, improve error visibility, and make the code more maintainable.

User Feedback

"We have been noticing an issue for a while, where Checkout Ruby SDK does not log response information when there is an error in a request (status code > 400). Are there any plans to improve the SDK to add this functionality? Maybe that is intended for some reason? We are reviewing whether we should implement our own custom handling for such scenarios."

This PR directly responds to the above feedback by addressing logging and error handling gaps.

Testing

  • Unit Tests:

    • Added new test cases for different response scenarios in api_client_spec.rb.
    • Covered scenarios including:
      • Valid JSON responses.
      • Empty or nil response bodies.
      • Error responses with various HTTP status codes.
  • Integration Tests:

    • Enhanced tests in contexts_integration_spec.rb and reports_integration_spec.rb to validate changes in response handling and error logging.

@armando-rodriguez-cko armando-rodriguez-cko requested a review from a team November 18, 2024 18:10
@armando-rodriguez-cko armando-rodriguez-cko force-pushed the feature/improve-error-management-response branch 2 times, most recently from ef64c60 to 3cd4f92 Compare November 19, 2024 12:09
@armando-rodriguez-cko armando-rodriguez-cko marked this pull request as draft November 19, 2024 12:35
@armando-rodriguez-cko armando-rodriguez-cko force-pushed the feature/improve-error-management-response branch 3 times, most recently from 3a07432 to 68ed2dc Compare November 19, 2024 13:29
@armando-rodriguez-cko armando-rodriguez-cko marked this pull request as ready for review November 19, 2024 13:29
@armando-rodriguez-cko armando-rodriguez-cko force-pushed the feature/improve-error-management-response branch from 68ed2dc to 42e1643 Compare November 19, 2024 13:40
@armando-rodriguez-cko armando-rodriguez-cko force-pushed the feature/improve-error-management-response branch from 42e1643 to 563d359 Compare November 19, 2024 13:55
@armando-rodriguez-cko armando-rodriguez-cko merged commit 7141ca3 into master Nov 19, 2024
2 checks passed
@armando-rodriguez-cko armando-rodriguez-cko deleted the feature/improve-error-management-response branch November 19, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants