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

Multipart post causes error in protocol-http #32

Closed
jakeonfire opened this issue Feb 7, 2024 · 9 comments · Fixed by socketry/multipart-post#107
Closed

Multipart post causes error in protocol-http #32

jakeonfire opened this issue Feb 7, 2024 · 9 comments · Fixed by socketry/multipart-post#107

Comments

@jakeonfire
Copy link
Contributor

when making a multipart post request the following error occurs:

undefined method `each' for #<Faraday::CompositeReadIO:0x0000ffffa2c07388 @parts=[#<Parts::FilePart:0x0000ffffa5b43840 @head="-------------RubyMultipartPost-de6d0df967e5e4b3647c154bd5ac8bc2\r\nContent-Disposition: form-data; name=\"Presentation\"; filename=\"local.path\"\r\nContent-Length: 330\r\nContent-Type: text/html\r\nContent-Transfer-Encoding: binary\r\n\r\n", @foot="\r\n", @length=556, @io=#<CompositeReadIO:0x0000ffffa2c07568 @ios=[#<StringIO:0x0000ffffa2c07608>, #<UploadIO:0x0000ffffa5ac8898 @content_type="text/html", @original_filename="local.path", @local_path="local.path", @io=#<StringIO:0x0000ffffa4150938>, @opts={}>, #<StringIO:0x0000ffffa2c075b8>], @index=0>>, #<Parts::EpiloguePart:0x0000ffffa2c07478 @part="-------------RubyMultipartPost-de6d0df967e5e4b3647c154bd5ac8bc2--\r\n\r\n", @io=#<StringIO:0x0000ffffa2c07400>>], @ios=[#<CompositeReadIO:0x0000ffffa2c07568 @ios=[#<StringIO:0x0000ffffa2c07608>, #<UploadIO:0x0000ffffa5ac8898 @content_type="text/html", @original_filename="local.path", @local_path="local.path", @io=#<StringIO:0x0000ffffa4150938>, @opts={}>, #<StringIO:0x0000ffffa2c075b8>], @index=0>, #<StringIO:0x0000ffffa2c07400>], @index=0>

/usr/local/bundle/gems/protocol-http-0.26.0/lib/protocol/http/body/buffered.rb:31:in `for'
/usr/local/bundle/gems/protocol-http-0.26.0/lib/protocol/http/body/buffered.rb:24:in `wrap'
/usr/local/bundle/gems/async-http-faraday-0.12.0/lib/async/http/faraday/adapter.rb:99:in `block in call'
/usr/local/bundle/gems/async-2.8.1/lib/kernel/sync.rb:20:in `Sync'
/usr/local/bundle/gems/async-http-faraday-0.12.0/lib/async/http/faraday/adapter.rb:88:in `call'
/usr/local/bundle/gems/faraday_middleware-0.13.1/lib/faraday_middleware/response/follow_redirects.rb:87:in `perform_with_redirection'
/usr/local/bundle/gems/faraday_middleware-0.13.1/lib/faraday_middleware/response/follow_redirects.rb:75:in `call'
/usr/local/bundle/gems/faraday-0.17.6/lib/faraday/request/multipart.rb:15:in `call'
/usr/local/bundle/gems/faraday-0.17.6/lib/faraday/rack_builder.rb:143:in `build_response'
/usr/local/bundle/gems/faraday-0.17.6/lib/faraday/connection.rb:387:in `run_request'
/usr/local/bundle/gems/faraday-0.17.6/lib/faraday/connection.rb:175:in `post'
/app/lib/shimmy/request_helper.rb:465:in `block in make_request'
/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'

gems used:

  • async-http-faraday (0.12.0)
  • faraday (0.17.6)

code to reproduce:

conn = Faraday.new do |c|
  c.request :multipart
  c.adapter :async_http
end
task = Async do
  conn.post("https://httpbin.org/post") do |req|
    req.body = { "file" => Faraday::UploadIO.new(StringIO.new("file content"), "text/plain", "file.txt") }
  end
end
task.wait
@jakeonfire
Copy link
Contributor Author

jakeonfire commented Feb 7, 2024

verified this issue exists with faraday (1.10.3) as well, in which case you can use:

conn = Faraday.new do |c|
  c.request :multipart
  c.adapter :async_http
end
task = Async do
  conn.post("https://httpbin.org/post") do |req|
    req.body = { "file" => Faraday::Multipart::FilePart.new(StringIO.new("file content"), "text/plain", "file.txt") }
  end
end
task.wait

@jakeonfire
Copy link
Contributor Author

jakeonfire commented Feb 7, 2024

separately, my app memory is now running amuck. i'm using falcon and async-http-faraday (the app is basically a proxy to 3rd party REST APIs), and it looks like this combination is causing a memory leak. is using both gems redundant vs just using falcon? or shouldasync-http-faraday provide extra efficiencies even when running on a falcon server?

Screenshot 2024-02-06 at 9 48 33 PM

you can see me using fewer and fewer falcon processes, where it was fine with 6 before the introduction of async-http-faraday.

@ioquatix
Copy link
Member

ioquatix commented Feb 7, 2024

That does look like a memory leak. Are you able to share more details of the app? I have a memory profiling tool, would you be willing to capture a dump?

@jakeonfire
Copy link
Contributor Author

jakeonfire commented Feb 7, 2024

more details, hmm... the app (rails 7.1) accepts requests from a client (another app of ours), proxies them (in the loosest sense) to various 3rd party APIs (on a couple dozen different hosts), then formats the response back to the client. we are using async-http-faraday to make those calls to 3rd party APIs. there is no manual parallelization within a request, we are just trying to support more connections from the client with less hardware, seeing as there is a lot of IO wait time when connecting to the 3rd party APIs.

i would be willing to capture a dump and private-gist it to you.

also sorry to hijack my own thread, but just want to make sure you saw my first two messages were about a separate (non-memory) issue.

@jakeonfire
Copy link
Contributor Author

jakeonfire commented Feb 7, 2024

imagine a vanilla rails app, where controller actions might look like:

class SomeController < ApplicationController
  def update
    response = request_handler.do_post("https://microsoft.com/update_thing", params[:post_body])
    render json: response.body
  end

  def request_handler
    @_request_handler ||= RequestHandler.new
  end
end

and RequestHandler something like:

class RequestHandler
  def do_post(url, body)
    task = Async do
      conn.post(url) do |req|
        req.body = body
      end
    end
    response = task.wait
  end

  def conn
    Faraday.new do |c|
      c.adapter :async_http
    end
  end
end

the app is using falcon host or falcon virtual with the rack configuration, a preload of the rails environment, and endpoint Async::HTTP::Endpoint.parse("http://0.0.0.0:3000"). pretty simple.

@jakeonfire
Copy link
Contributor Author

and fwiw i've tried with both falcon + (non-async) faraday, as well as unicorn (don't ask) + async-http-faraday and both configurations have no memory issues.

@ioquatix
Copy link
Member

ioquatix commented Feb 7, 2024

I can reproduce the original issue, so let me fix that to start with.

@ioquatix
Copy link
Member

ioquatix commented Feb 7, 2024

This is fixed in v0.13.0 of async-http-faraday, along with v2.4.0 of multipart-post.

Can you try it out and let me know?

@jakeonfire
Copy link
Contributor Author

Can you try it out and let me know?

yep, it works 🥳 i didn't realize you also work on multipart-post, how fortunate!

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 a pull request may close this issue.

2 participants