URL-encoded bodies are not readable - request errors during processing

[jakeonfire]

it seems setting the body to a hash for url_encoded requests results in a String body in the async faraday adapter where it is expecting a Readable?

49.64s     warn: Async::Task: Writing #<Async::HTTP::Faraday::BodyWrapper:0x0000ffffa59bc440> to #<Async::HTTP::Protocol::HTTP2::Response::Stream:0x0000ffffa758a1e0>. [oid=0x23a0] [ec=0x23b4] [pid=149] [2024-02-07 20:54:59 +0000]
               | Task may have ended with unhandled exception.
               |   NoMethodError: undefined method `read' for "foo=42":String
               |   → /usr/local/bundle/gems/async-http-faraday-0.13.0/lib/async/http/faraday/adapter.rb:34 in `read'
               |     /usr/local/bundle/gems/async-http-0.63.0/lib/async/http/protocol/http2/output.rb:83 in `passthrough'
               |     /usr/local/bundle/gems/async-2.8.1/lib/async/task.rb:161 in `block in run'
               |     /usr/local/bundle/gems/async-2.8.1/lib/async/task.rb:331 in `block in schedule'

code to repro:

task = Async do
  conn = Faraday.new do |c|
    c.request :url_encoded
    c.adapter :async_http
  end
  conn.post("https://httpbin.org/post") do |req|
    req.body = {foo: 42}
  end
ensure
  conn&.close
end
task.wait

[ioquatix]

@iMacTia is there documentation about what valid body can be and what to do with it i.e. how to get a consistent readable interface?

[jakeonfire]

work-around for now:

task = Async do
  conn = Faraday.new do |c|
    c.adapter :async_http
  end
  conn.post("https://httpbin.org/post") do |req|
    req.body = StringIO.new({foo: 42}.to_query)
    req.headers[Faraday::Request::UrlEncoded::CONTENT_TYPE] ||= Faraday::Request::UrlEncoded.mime_type
  end
ensure
  conn&.close
end
task.wait

[ioquatix]

I think I found the solution, I'll try to make a release within a few hours including the fix.

[jakeonfire]

also seeing this with e.g.

task = Async do
  conn = Faraday.new do |c|
    c.request :json
    c.adapter :async_http
  end
  conn.post("https://httpbin.org/post") do |req|
    req.body = {foo: 42}
  end
ensure
  conn&.close
end
task.wait

fwiw. almost certainly has the same fix, though.

[ioquatix]

Do you mind trying out v0.13.1 which was just released. It should fix the issue.

[jakeonfire]

v0.13.1 fixes the issue 🥳 thanks!

[iMacTia]

Thank you for the quick fix @ioquatix 🙌

I'm surprised this never happened before because to my knowledge it's fairly common for Faraday to pass a string as the request body for post/put/patch methods 🤔

Is there documentation about what valid body can be and what to do with it
Not really, because what the adapter expects can vary wildly. As I said though, to my knowledge, String is almost always what is given to the adapter (after middleware takes whatever the input is, like a Hash and turns that into a string).

[ioquatix]

It seems like it's at least either:

1. A String instance, or
2. An object that responds to #read.

[iMacTia]

mmmh it seems like Net::HTTP supports #read as well, but you need to pass it explicitly as body_stream.

To be fair, Faraday have just recently added first-class support for RESPONSE streaming, this looks more like we want to support request body streaming. Might be a good addition to the API.

Adapters like async-http will support it transparently, while others like net-http would require some extra code

[ioquatix]

For sure it seems needed for multipart post - because that is a rich object which responds to #read so I'm not sure how other clients are handling it (it was simply assigned to #body).

[iMacTia]

Actually yes, Net::HTTP does support streams, but you also have to set Content-Length in that case or you'll get the following error back:

`send_request_with_body_stream': Content-Length not given and Transfer-Encoding is not `chunked' (ArgumentError)

And this is exactly what faraday-multipart does: https://github.com/lostisland/faraday-multipart/blob/main/lib/faraday/multipart/middleware.rb#L63

So yeah, it seems like adapters should support both. Documentation should definitely point this out more explicitly

[iMacTia]

Opened lostisland/faraday#1554, thank you so much for the discussion 🙌 !