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

Handle response when http_component is set and include logging of x-fb-debug, x-fb-rev #648

Closed
wants to merge 4 commits into from

Conversation

pri1012
Copy link
Contributor

@pri1012 pri1012 commented Mar 29, 2019

Description:

This PR includes changes which will help use the latest koala version when http_component is set

Changes included:

  • Parsing of response: when http_component is set, we receive Koala::Http_service response instead of a hash and generate_results method handles only JSON response. This affects all batch request.
  • include x-fb-trace-id and x-fb-debug x-fb-rev as part of the error_message returned

[X] My PR has tests and they pass!

@codealchemy
Copy link

@ylecuyer bumping this for consideration when you have a moment 🙏

@ylecuyer
Copy link
Collaborator

ylecuyer commented Nov 9, 2023

I checked the code and the PR is mixing 3 things afaics. Would you mind splitting it and making on per per change, namely:

  1. adding a new http_component that would return the body of the response
  2. adding the new debug headers
  3. returning custom error when the body is nil

this way review will be easier and changes will appear clearly in the changelog

@pri1012
Copy link
Contributor Author

pri1012 commented Jan 10, 2024

@ylecuyer
1 & 3 : #685, 2 separate commits
2: #686

cc @codealchemy

@pri1012
Copy link
Contributor Author

pri1012 commented Mar 27, 2024

@ylecuyer I have now fixed the failing units tests. Could you please help review the below PRs.

  1. Add new http_component that would return the body of the response and custom error messages. #685
    2: Include logging of x-fb-debug, x-fb-rev #648 #686
    cc @arsduo

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.

None yet

3 participants