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 request data logging for httprb/http version 4 #56

Merged
merged 2 commits into from
Feb 18, 2018

Conversation

tycooon
Copy link
Contributor

@tycooon tycooon commented Feb 14, 2018

Since version 3, httprb/http gem logs request data like this:
[httplog] Data: #<HTTP::Request::Body:0x00007ffa7e32ccd0>

Unfortunately, it's difficult to make version 3 work, but in version 4 (currently in development and only available in master) they added HTTP::Request::Body#source, so we can do stuff like source.to_s and source.rewind. This patch fixes cases for plaintext body and form body types. Note that we are just using to_s on source so that file bodies are going to be logged like #<File:0x00007ffabb2ba620> which I think is OK because we don't want to read the whole file.

Travis is not configured to test different gem versions for this repo, but I tested the patch locally and all specs pass if you add gem "http", github: "httprb/http" to the Gemfile (one spec fails if you update http gem to version 3).

@trusche
Copy link
Owner

trusche commented Feb 14, 2018

Aweseome, thanks @tycooon . I'll try to get that merged as soon as possible, need to figure out the Travis config.

@trusche trusche self-assigned this Feb 14, 2018
@tycooon
Copy link
Contributor Author

tycooon commented Feb 14, 2018

I think I know how to make this work in version 3, but this is going to use instance_variable_get. What do you think?
And I also know how to configure travis to test different httprb versions, do you want me to add it to this PR?

@trusche
Copy link
Owner

trusche commented Feb 14, 2018 via email

@tycooon
Copy link
Contributor Author

tycooon commented Feb 15, 2018

Done, please take a look.

@trusche trusche merged commit fa7bfc6 into trusche:master Feb 18, 2018
@trusche
Copy link
Owner

trusche commented Feb 19, 2018

I published v1.0.1 with this. Thanks again!

@tycooon tycooon deleted the add-http-4-request-data-support branch February 19, 2018 08:47
@tycooon tycooon restored the add-http-4-request-data-support branch February 26, 2018 10:36
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