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

Improve request body io-alike objects handling #469

Open
ixti opened this issue Apr 24, 2018 · 2 comments
Open

Improve request body io-alike objects handling #469

ixti opened this issue Apr 24, 2018 · 2 comments
Assignees

Comments

@ixti
Copy link
Member

ixti commented Apr 24, 2018

As of #468 pipes will cause failure:

reader, writer = IO.pipe
writer.puts "abc"
writer.close

HTTP.post(url, :body => reader)

That is because reader is an IO that supports rewind, but it's illegal on pipes:

Errno::ESPIPE: Illegal seek

So, I think we should become more strict about what is allowed to be used as request body.

@httprb/core comments and thoughts are welcomed and appreciated!

@ixti ixti self-assigned this Apr 24, 2018
@janko
Copy link
Member

janko commented Apr 24, 2018

I think it would be nice to support pipes and other non-rewindable objects (another example would be Down::ChunkedIO), because HTTP.rb itself doesn't need the body to be rewindable. Is there any existing or potential features that would benefit from the constraint that the request body needs to be rewindable?

I think that Webmock, which is the one that motivated this change, could just replace the original request body object with the accumulated string, once it reads the original request body. For me that seems like the ideal solution.

@ixti
Copy link
Member Author

ixti commented Apr 24, 2018

I agree that we should not limit body to be rewindable. We just need to raise proper error if response body can't be rewinded upon re-usage attempt.

About WebMock in fact I agree that we should improve integration. I was thinking that I can "better" patch request in webmock... And in fact after some thinking - I guess we can/should extract webmock integration into a gem under httprb - so that adapter will be "in-sync" with http gem itself.

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

No branches or pull requests

2 participants