From 0bcb7ddbb6ae493ce221968fb5e1bb7584b01ea6 Mon Sep 17 00:00:00 2001 From: pete gamache Date: Thu, 2 Jul 2020 15:51:39 -0400 Subject: [PATCH 01/11] upgrade to Cowboy 2.8 --- mix.exs | 4 ++-- mix.lock | 13 +++++++------ test/mojito_test.exs | 16 ++++++++++++++++ test/support/mojito_test_server.ex | 20 ++++++-------------- 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/mix.exs b/mix.exs index be67a46..4518e42 100644 --- a/mix.exs +++ b/mix.exs @@ -51,9 +51,9 @@ defmodule Mojito.MixProject do {:poolboy, "~> 1.5"}, {:ex_spec, "~> 2.0", only: :test}, {:jason, "~> 1.0", only: :test}, - {:cowboy, "~> 1.1", only: :test}, + {:cowboy, "~> 2.0", only: :test}, {:plug, "~> 1.3", only: :test}, - {:plug_cowboy, "~> 1.0", only: :test}, + {:plug_cowboy, "~> 2.0", only: :test}, {:ex_doc, "~> 0.18", only: :dev, runtime: false}, {:dialyxir, "~> 0.5", only: :dev, runtime: false} ] diff --git a/mix.lock b/mix.lock index 63a6e44..a44da8e 100644 --- a/mix.lock +++ b/mix.lock @@ -1,7 +1,7 @@ %{ "castore": {:hex, :castore, "0.1.4", "e7fd082c0e755716826a20b95c479f5dd42f536f9d12b8da8f47c92f1d4aed58", [:mix], [], "hexpm", "7fe7a9308ac5810083770843b74c3b5b1003cc0626610dc1d859da101c9647c6"}, - "cowboy": {:hex, :cowboy, "1.1.2", "61ac29ea970389a88eca5a65601460162d370a70018afe6f949a29dca91f3bb0", [:rebar3], [{:cowlib, "~> 1.0.2", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "~> 1.3.2", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "f4763bbe08233eceed6f24bc4fcc8d71c17cfeafa6439157c57349aa1bb4f17c"}, - "cowlib": {:hex, :cowlib, "1.0.2", "9d769a1d062c9c3ac753096f868ca121e2730b9a377de23dec0f7e08b1df84ee", [:make], [], "hexpm", "db622da03aa039e6366ab953e31186cc8190d32905e33788a1acb22744e6abd2"}, + "cowboy": {:hex, :cowboy, "2.8.0", "f3dc62e35797ecd9ac1b50db74611193c29815401e53bac9a5c0577bd7bc667d", [:rebar3], [{:cowlib, "~> 2.9.1", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "~> 1.7.1", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "4643e4fba74ac96d4d152c75803de6fad0b3fa5df354c71afdd6cbeeb15fac8a"}, + "cowlib": {:hex, :cowlib, "2.9.1", "61a6c7c50cf07fdd24b2f45b89500bb93b6686579b069a89f88cb211e1125c78", [:rebar3], [], "hexpm", "e4175dc240a70d996156160891e1c62238ede1729e45740bdd38064dad476170"}, "dialyxir": {:hex, :dialyxir, "0.5.1", "b331b091720fd93e878137add264bac4f644e1ddae07a70bf7062c7862c4b952", [:mix], [], "hexpm", "6c32a70ed5d452c6650916555b1f96c79af5fc4bf286997f8b15f213de786f73"}, "earmark": {:hex, :earmark, "1.3.1", "73812f447f7a42358d3ba79283cfa3075a7580a3a2ed457616d6517ac3738cb9", [:mix], [], "hexpm", "000aaeff08919e95e7aea13e4af7b2b9734577b3e6a7c50ee31ee88cab6ec4fb"}, "ex_doc": {:hex, :ex_doc, "0.19.3", "3c7b0f02851f5fc13b040e8e925051452e41248f685e40250d7e40b07b9f8c10", [:mix], [{:earmark, "~> 1.2", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.10", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm", "0e11d67e662142fc3945b0ee410c73c8c956717fbeae4ad954b418747c734973"}, @@ -14,10 +14,11 @@ "mime": {:hex, :mime, "1.3.1", "30ce04ab3175b6ad0bdce0035cba77bba68b813d523d1aac73d9781b4d193cf8", [:mix], [], "hexpm", "6cbe761d6a0ca5a31a0931bf4c63204bceb64538e664a8ecf784a9a6f3b875f1"}, "mint": {:hex, :mint, "1.0.0", "ca5ab33497ba2bdcc42f6cdd3927420a6159116be87c8173658e93c8746703da", [:mix], [{:castore, "~> 0.1.0", [hex: :castore, repo: "hexpm", optional: true]}], "hexpm", "b8943ef1e630879538dd6620bfc189d4d75fab3ad39f3fe9c50539879f7efd84"}, "nimble_parsec": {:hex, :nimble_parsec, "0.5.0", "90e2eca3d0266e5c53f8fbe0079694740b9c91b6747f2b7e3c5d21966bba8300", [:mix], [], "hexpm", "5c040b8469c1ff1b10093d3186e2e10dbe483cd73d79ec017993fb3985b8a9b3"}, - "plug": {:hex, :plug, "1.7.2", "d7b7db7fbd755e8283b6c0a50be71ec0a3d67d9213d74422d9372effc8e87fd1", [:mix], [{:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}], "hexpm", "de9825f21c6fd6adfdeae8f9c80dcd88c1e58301f06bf13d659b7e606b88abe0"}, - "plug_cowboy": {:hex, :plug_cowboy, "1.0.0", "2e2a7d3409746d335f451218b8bb0858301c3de6d668c3052716c909936eb57a", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, repo: "hexpm", optional: false]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "01d201427a8a1f4483be2465a98b45f5e82263327507fe93404a61c51eb9e9a8"}, - "plug_crypto": {:hex, :plug_crypto, "1.0.0", "18e49317d3fa343f24620ed22795ec29d4a5e602d52d1513ccea0b07d8ea7d4d", [:mix], [], "hexpm", "73c1682f0e414cfb5d9b95c8e8cd6ffcfdae699e3b05e1db744e58b7be857759"}, + "plug": {:hex, :plug, "1.10.3", "c9cebe917637d8db0e759039cc106adca069874e1a9034fd6e3fdd427fd3c283", [:mix], [{:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "01f9037a2a1de1d633b5a881101e6a444bcabb1d386ca1e00bb273a1f1d9d939"}, + "plug_cowboy": {:hex, :plug_cowboy, "2.3.0", "149a50e05cb73c12aad6506a371cd75750c0b19a32f81866e1a323dda9e0e99d", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "bc595a1870cef13f9c1e03df56d96804db7f702175e4ccacdb8fc75c02a7b97e"}, + "plug_crypto": {:hex, :plug_crypto, "1.1.2", "bdd187572cc26dbd95b87136290425f2b580a116d3fb1f564216918c9730d227", [:mix], [], "hexpm", "6b8b608f895b6ffcfad49c37c7883e8df98ae19c6a28113b02aa1e9c5b22d6b5"}, "poolboy": {:hex, :poolboy, "1.5.2", "392b007a1693a64540cead79830443abf5762f5d30cf50bc95cb2c1aaafa006b", [:rebar3], [], "hexpm", "dad79704ce5440f3d5a3681c8590b9dc25d1a561e8f5a9c995281012860901e3"}, - "ranch": {:hex, :ranch, "1.3.2", "e4965a144dc9fbe70e5c077c65e73c57165416a901bd02ea899cfd95aa890986", [:rebar3], [], "hexpm", "6e56493a862433fccc3aca3025c946d6720d8eedf6e3e6fb911952a7071c357f"}, + "ranch": {:hex, :ranch, "1.7.1", "6b1fab51b49196860b733a49c07604465a47bdb78aa10c1c16a3d199f7f8c881", [:rebar3], [], "hexpm", "451d8527787df716d99dc36162fca05934915db0b6141bbdac2ea8d3c7afc7d7"}, + "telemetry": {:hex, :telemetry, "0.4.2", "2808c992455e08d6177322f14d3bdb6b625fbcfd233a73505870d8738a2f4599", [:rebar3], [], "hexpm", "2d1419bd9dda6a206d7b5852179511722e2b18812310d304620c7bd92a13fcef"}, "xhttp": {:git, "https://github.com/ericmj/xhttp.git", "d0606462cba650fdb91b61e779f38e4e9b6383f6", []}, } diff --git a/test/mojito_test.exs b/test/mojito_test.exs index 1f00d00..1a2f42e 100644 --- a/test/mojito_test.exs +++ b/test/mojito_test.exs @@ -192,6 +192,22 @@ defmodule MojitoTest do assert("12" == Headers.get(response.headers, "content-length")) end + it "can use HTTP/1.1" do + assert({:ok, response} = get("/", protocols: [:http1])) + assert(200 == response.status_code) + assert("Hello world!" == response.body) + assert(12 == response.size) + assert("12" == Headers.get(response.headers, "content-length")) + end + + it "can use HTTP/2" do + assert({:ok, response} = get("/", protocols: [:http2])) + assert(200 == response.status_code) + assert("Hello world!" == response.body) + assert(12 == response.size) + assert("12" == Headers.get(response.headers, "content-length")) + end + it "can make HTTPS requests" do assert({:ok, response} = get_ssl("/")) assert(200 == response.status_code) diff --git a/test/support/mojito_test_server.ex b/test/support/mojito_test_server.ex index 28131e6..184e0cf 100644 --- a/test/support/mojito_test_server.ex +++ b/test/support/mojito_test_server.ex @@ -3,20 +3,12 @@ defmodule Mojito.TestServer do def start(_type, _args) do children = [ - Plug.Adapters.Cowboy.child_spec( - :http, - Mojito.TestServer.PlugRouter, - [], - port: Application.get_env(:mojito, :test_server_http_port) - ), - Plug.Adapters.Cowboy.child_spec( - :https, - Mojito.TestServer.PlugRouter, - [], + {Plug.Cowboy, scheme: :http, plug: Mojito.TestServer.PlugRouter, + port: Application.get_env(:mojito, :test_server_http_port)}, + {Plug.Cowboy, scheme: :https, plug: Mojito.TestServer.PlugRouter, port: Application.get_env(:mojito, :test_server_https_port), keyfile: File.cwd!() <> "/test/support/key.pem", - certfile: File.cwd!() <> "/test/support/cert.pem" - ) + certfile: File.cwd!() <> "/test/support/cert.pem"} ] Supervisor.start_link(children, strategy: :one_for_one) @@ -50,8 +42,8 @@ defmodule Mojito.TestServer.PlugRouter do {:ok, req} = :cowboy_req.reply( 200, - [{"connection", "close"}], - fn socket, transport -> transport.send(socket, "close") end, + %{"connection" => "close"}, + "close", req ) From bafae70b57d82d5c339b2d3c0b3347ed95e8444d Mon Sep 17 00:00:00 2001 From: pete gamache Date: Thu, 2 Jul 2020 16:25:06 -0400 Subject: [PATCH 02/11] upgrade Mint to 1.1; add breaking test for #54 --- mix.exs | 2 +- mix.lock | 4 ++-- test/mojito_test.exs | 13 +++++++++++++ test/support/mojito_test_server.ex | 16 ++++++++++------ 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/mix.exs b/mix.exs index 4518e42..c3abcc2 100644 --- a/mix.exs +++ b/mix.exs @@ -46,7 +46,7 @@ defmodule Mojito.MixProject do defp deps do [ - {:mint, "~> 1.0"}, + {:mint, "~> 1.1"}, {:castore, "~> 0.1"}, {:poolboy, "~> 1.5"}, {:ex_spec, "~> 2.0", only: :test}, diff --git a/mix.lock b/mix.lock index a44da8e..b551b40 100644 --- a/mix.lock +++ b/mix.lock @@ -1,5 +1,5 @@ %{ - "castore": {:hex, :castore, "0.1.4", "e7fd082c0e755716826a20b95c479f5dd42f536f9d12b8da8f47c92f1d4aed58", [:mix], [], "hexpm", "7fe7a9308ac5810083770843b74c3b5b1003cc0626610dc1d859da101c9647c6"}, + "castore": {:hex, :castore, "0.1.6", "2da0dccb3eacb67841d11790598ff03cd5caee861e01fad61dce1376b5da28e6", [:mix], [], "hexpm", "f874c510b720d31dd6334e9ae5c859a06a3c9e67dfe1a195c512e57588556d3f"}, "cowboy": {:hex, :cowboy, "2.8.0", "f3dc62e35797ecd9ac1b50db74611193c29815401e53bac9a5c0577bd7bc667d", [:rebar3], [{:cowlib, "~> 2.9.1", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "~> 1.7.1", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "4643e4fba74ac96d4d152c75803de6fad0b3fa5df354c71afdd6cbeeb15fac8a"}, "cowlib": {:hex, :cowlib, "2.9.1", "61a6c7c50cf07fdd24b2f45b89500bb93b6686579b069a89f88cb211e1125c78", [:rebar3], [], "hexpm", "e4175dc240a70d996156160891e1c62238ede1729e45740bdd38064dad476170"}, "dialyxir": {:hex, :dialyxir, "0.5.1", "b331b091720fd93e878137add264bac4f644e1ddae07a70bf7062c7862c4b952", [:mix], [], "hexpm", "6c32a70ed5d452c6650916555b1f96c79af5fc4bf286997f8b15f213de786f73"}, @@ -12,7 +12,7 @@ "makeup": {:hex, :makeup, "0.8.0", "9cf32aea71c7fe0a4b2e9246c2c4978f9070257e5c9ce6d4a28ec450a839b55f", [:mix], [{:nimble_parsec, "~> 0.5.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "5fbc8e549aa9afeea2847c0769e3970537ed302f93a23ac612602e805d9d1e7f"}, "makeup_elixir": {:hex, :makeup_elixir, "0.13.0", "be7a477997dcac2e48a9d695ec730b2d22418292675c75aa2d34ba0909dcdeda", [:mix], [{:makeup, "~> 0.8", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "adf0218695e22caeda2820eaba703fa46c91820d53813a2223413da3ef4ba515"}, "mime": {:hex, :mime, "1.3.1", "30ce04ab3175b6ad0bdce0035cba77bba68b813d523d1aac73d9781b4d193cf8", [:mix], [], "hexpm", "6cbe761d6a0ca5a31a0931bf4c63204bceb64538e664a8ecf784a9a6f3b875f1"}, - "mint": {:hex, :mint, "1.0.0", "ca5ab33497ba2bdcc42f6cdd3927420a6159116be87c8173658e93c8746703da", [:mix], [{:castore, "~> 0.1.0", [hex: :castore, repo: "hexpm", optional: true]}], "hexpm", "b8943ef1e630879538dd6620bfc189d4d75fab3ad39f3fe9c50539879f7efd84"}, + "mint": {:hex, :mint, "1.1.0", "1fd0189edd9e3ffdbd7fcd8bc3835902b987a63ec6c4fd1aa8c2a56e2165f252", [:mix], [{:castore, "~> 0.1.0", [hex: :castore, repo: "hexpm", optional: true]}], "hexpm", "5bfd316c3789340b682d5679a8116bcf2112e332447bdc20c1d62909ee45f48d"}, "nimble_parsec": {:hex, :nimble_parsec, "0.5.0", "90e2eca3d0266e5c53f8fbe0079694740b9c91b6747f2b7e3c5d21966bba8300", [:mix], [], "hexpm", "5c040b8469c1ff1b10093d3186e2e10dbe483cd73d79ec017993fb3985b8a9b3"}, "plug": {:hex, :plug, "1.10.3", "c9cebe917637d8db0e759039cc106adca069874e1a9034fd6e3fdd427fd3c283", [:mix], [{:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "01f9037a2a1de1d633b5a881101e6a444bcabb1d386ca1e00bb273a1f1d9d939"}, "plug_cowboy": {:hex, :plug_cowboy, "2.3.0", "149a50e05cb73c12aad6506a371cd75750c0b19a32f81866e1a323dda9e0e99d", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "bc595a1870cef13f9c1e03df56d96804db7f702175e4ccacdb8fc75c02a7b97e"}, diff --git a/test/mojito_test.exs b/test/mojito_test.exs index 1a2f42e..8a98cff 100644 --- a/test/mojito_test.exs +++ b/test/mojito_test.exs @@ -221,6 +221,13 @@ defmodule MojitoTest do assert({:error, %Error{reason: :timeout}} = get("/wait1", timeout: 100)) end + it "can http/2 via Mint" do + port = Application.get_env(:mojito, :test_server_http_port) + + assert {:ok, mint_conn} = + Mint.HTTP.connect(:http, "localhost", port, protocols: [:http2]) + end + it "handles timeouts even on long requests" do port = Application.get_env(:mojito, :test_server_http_port) {:ok, conn} = Mojito.Conn.connect("http://localhost:#{port}") @@ -366,6 +373,12 @@ defmodule MojitoTest do assert({:ok, response} = get_ssl("/close", pool: false)) assert("close" == response.body) end + + it "can POST big bodies" do + one_meg = String.duplicate("x", 0x100000) + body = %{data: one_meg} |> Jason.encode!() + assert({:ok, response} = post("/post", body, protocols: [:http2])) + end end context "external tests" do diff --git a/test/support/mojito_test_server.ex b/test/support/mojito_test_server.ex index 184e0cf..4986970 100644 --- a/test/support/mojito_test_server.ex +++ b/test/support/mojito_test_server.ex @@ -3,12 +3,16 @@ defmodule Mojito.TestServer do def start(_type, _args) do children = [ - {Plug.Cowboy, scheme: :http, plug: Mojito.TestServer.PlugRouter, - port: Application.get_env(:mojito, :test_server_http_port)}, - {Plug.Cowboy, scheme: :https, plug: Mojito.TestServer.PlugRouter, - port: Application.get_env(:mojito, :test_server_https_port), - keyfile: File.cwd!() <> "/test/support/key.pem", - certfile: File.cwd!() <> "/test/support/cert.pem"} + {Plug.Cowboy, + scheme: :http, + plug: Mojito.TestServer.PlugRouter, + port: Application.get_env(:mojito, :test_server_http_port)}, + {Plug.Cowboy, + scheme: :https, + plug: Mojito.TestServer.PlugRouter, + port: Application.get_env(:mojito, :test_server_https_port), + keyfile: File.cwd!() <> "/test/support/key.pem", + certfile: File.cwd!() <> "/test/support/cert.pem"} ] Supervisor.start_link(children, strategy: :one_for_one) From 77b00d645c2925d41dc23cd107a82a1e4350c0b3 Mon Sep 17 00:00:00 2001 From: pete gamache Date: Thu, 2 Jul 2020 18:19:45 -0400 Subject: [PATCH 03/11] checkpoint --- lib/mojito/conn.ex | 74 +++++++++++++++++++++++++++++-- lib/mojito/pool/poolboy/single.ex | 6 ++- lib/mojito/request/single.ex | 29 ++---------- lib/mojito/response.ex | 27 +++++++++++ mix.lock | 13 +++--- test/mojito_test.exs | 37 +++++++++++----- 6 files changed, 140 insertions(+), 46 deletions(-) diff --git a/lib/mojito/conn.ex b/lib/mojito/conn.ex index c57163f..10eb562 100644 --- a/lib/mojito/conn.ex +++ b/lib/mojito/conn.ex @@ -55,6 +55,9 @@ defmodule Mojito.Conn do """ @spec request(t, Mojito.request()) :: {:ok, t, reference} | {:error, any} def request(conn, request) do + max_body_size = request.opts[:max_body_size] + response = %Mojito.Response{size: max_body_size} + with {:ok, relative_url, auth_headers} <- Utils.get_relative_url_and_auth_headers(request.url), {:ok, mint_conn, request_ref} <- @@ -63,9 +66,74 @@ defmodule Mojito.Conn do method_to_string(request.method), relative_url, auth_headers ++ request.headers, - request.body - ) do - {:ok, %{conn | conn: mint_conn}, request_ref} + :stream + ), + {:ok, mint_conn, response} <- + stream_request_body(mint_conn, request_ref, response, request.body) do + {:ok, %{conn | conn: mint_conn}, request_ref, response} + end + end + + defp stream_request_body(mint_conn, request_ref, response, nil) do + stream_request_body(mint_conn, request_ref, response, "") + end + + defp stream_request_body(mint_conn, request_ref, response, "") do + with {:ok, mint_conn, _ref} <- + Mint.HTTP.stream_request_body(mint_conn, request_ref, :eof) do + {:ok, mint_conn, request_ref, response} + end + end + + defp stream_request_body( + %Mint.HTTP1{} = mint_conn, + request_ref, + response, + body + ) do + {chunk, rest} = String.split_at(body, 65535) + + with {:ok, mint_conn} <- + Mint.HTTP.stream_request_body(mint_conn, request_ref, chunk) do + stream_request_body(mint_conn, request_ref, response, rest) + end + end + + defp stream_request_body( + %Mint.HTTP2{} = mint_conn, + request_ref, + response, + body + ) do + chunk_size = + min( + Mint.HTTP2.get_window_size(mint_conn, {:request, request_ref}), + Mint.HTTP2.get_window_size(mint_conn, :connection) + ) + + {chunk, rest} = String.split_at(body, chunk_size) + if "" == rest, do: IO.puts("last one") + + with {:ok, mint_conn} <- + Mint.HTTP.stream_request_body(mint_conn, request_ref, chunk) do + {mint_conn, response} = + if "" != rest do + {:ok, mint_conn, resps} = + receive do + msg -> Mint.HTTP.stream(mint_conn, msg) + end + + response = Mojito.Response.apply_resps(response, resps) + {mint_conn, response} + else + {mint_conn, response} + end + + if response.complete do + {:ok, mint_conn, response} + else + stream_request_body(mint_conn, request_ref, response, rest) + end end end diff --git a/lib/mojito/pool/poolboy/single.ex b/lib/mojito/pool/poolboy/single.ex index 30f4fa1..44d1b8c 100644 --- a/lib/mojito/pool/poolboy/single.ex +++ b/lib/mojito/pool/poolboy/single.ex @@ -1,6 +1,8 @@ defmodule Mojito.Pool.Poolboy.Single do @moduledoc false + require Logger + ## Mojito.Pool.Poolboy.Single provides an HTTP request connection pool based on ## Mojito and Poolboy. ## @@ -155,7 +157,9 @@ defmodule Mojito.Pool.Poolboy.Single do rescue e -> {:error, e} catch - :exit, _ -> {:error, :checkout_timeout} + :exit, e -> + e |> inspect |> Logger.error() + {:error, :checkout_timeout} after Process.flag(:trap_exit, old_trap_exit) end diff --git a/lib/mojito/request/single.ex b/lib/mojito/request/single.ex index 5e1d136..4b08ad2 100644 --- a/lib/mojito/request/single.ex +++ b/lib/mojito/request/single.ex @@ -26,10 +26,10 @@ defmodule Mojito.Request.Single do def request(%Request{} = req) do with {:ok, req} <- Request.validate_request(req), {:ok, conn} <- Conn.connect(req.url, req.opts), - {:ok, conn, _ref} <- Conn.request(conn, req) do + {:ok, conn, _ref, response} <- Conn.request(conn, req) do timeout = req.opts[:timeout] || Config.timeout() max_body_size = req.opts[:max_body_size] - receive_response(conn, %Response{size: max_body_size}, timeout) + receive_response(conn, response, timeout) end end @@ -75,7 +75,7 @@ defmodule Mojito.Request.Single do case Mint.HTTP.stream(conn.conn, msg) do {:ok, mint_conn, resps} -> conn = %{conn | conn: mint_conn} - response = apply_resps(response, resps) + response = Response.apply_resps(response, resps) case response do %{complete: true} -> @@ -95,27 +95,4 @@ defmodule Mojito.Request.Single do receive_response(conn, response, new_timeout.()) end end - - defp apply_resps(response, []), do: response - - defp apply_resps(response, [mint_resp | rest]) do - apply_resp(response, mint_resp) |> apply_resps(rest) - end - - defp apply_resp(response, {:status, _request_ref, status_code}) do - %{response | status_code: status_code} - end - - defp apply_resp(response, {:headers, _request_ref, headers}) do - %{response | headers: headers} - end - - defp apply_resp(response, {:data, _request_ref, chunk}) do - {:ok, resp} = Utils.put_chunk(response, chunk) - resp - end - - defp apply_resp(response, {:done, _request_ref}) do - %{response | complete: true, body: :erlang.iolist_to_binary(response.body)} - end end diff --git a/lib/mojito/response.ex b/lib/mojito/response.ex index c37ed50..c1e9aeb 100644 --- a/lib/mojito/response.ex +++ b/lib/mojito/response.ex @@ -8,4 +8,31 @@ defmodule Mojito.Response do size: 0 @type t :: Mojito.response() + + @doc ~S""" + Applies responses received from `Mint.HTTP.stream/2` to a %Mojito.Response{}. + """ + @spec apply_resps(t, [Mint.Types.response()]) :: t + def apply_resps(response, []), do: response + + def apply_resps(response, [mint_resp | rest]) do + apply_resp(response, mint_resp) |> apply_resps(rest) + end + + defp apply_resp(response, {:status, _request_ref, status_code}) do + %{response | status_code: status_code} + end + + defp apply_resp(response, {:headers, _request_ref, headers}) do + %{response | headers: headers} + end + + defp apply_resp(response, {:data, _request_ref, chunk}) do + {:ok, resp} = Utils.put_chunk(response, chunk) + resp + end + + defp apply_resp(response, {:done, _request_ref}) do + %{response | complete: true, body: :erlang.iolist_to_binary(response.body)} + end end diff --git a/mix.lock b/mix.lock index b551b40..f9bac36 100644 --- a/mix.lock +++ b/mix.lock @@ -3,17 +3,18 @@ "cowboy": {:hex, :cowboy, "2.8.0", "f3dc62e35797ecd9ac1b50db74611193c29815401e53bac9a5c0577bd7bc667d", [:rebar3], [{:cowlib, "~> 2.9.1", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "~> 1.7.1", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "4643e4fba74ac96d4d152c75803de6fad0b3fa5df354c71afdd6cbeeb15fac8a"}, "cowlib": {:hex, :cowlib, "2.9.1", "61a6c7c50cf07fdd24b2f45b89500bb93b6686579b069a89f88cb211e1125c78", [:rebar3], [], "hexpm", "e4175dc240a70d996156160891e1c62238ede1729e45740bdd38064dad476170"}, "dialyxir": {:hex, :dialyxir, "0.5.1", "b331b091720fd93e878137add264bac4f644e1ddae07a70bf7062c7862c4b952", [:mix], [], "hexpm", "6c32a70ed5d452c6650916555b1f96c79af5fc4bf286997f8b15f213de786f73"}, - "earmark": {:hex, :earmark, "1.3.1", "73812f447f7a42358d3ba79283cfa3075a7580a3a2ed457616d6517ac3738cb9", [:mix], [], "hexpm", "000aaeff08919e95e7aea13e4af7b2b9734577b3e6a7c50ee31ee88cab6ec4fb"}, - "ex_doc": {:hex, :ex_doc, "0.19.3", "3c7b0f02851f5fc13b040e8e925051452e41248f685e40250d7e40b07b9f8c10", [:mix], [{:earmark, "~> 1.2", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.10", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm", "0e11d67e662142fc3945b0ee410c73c8c956717fbeae4ad954b418747c734973"}, + "earmark": {:hex, :earmark, "1.4.9", "837e4c1c5302b3135e9955f2bbf52c6c52e950c383983942b68b03909356c0d9", [:mix], [{:earmark_parser, ">= 1.4.9", [hex: :earmark_parser, repo: "hexpm", optional: false]}], "hexpm", "0d72df7d13a3dc8422882bed5263fdec5a773f56f7baeb02379361cb9e5b0d8e"}, + "earmark_parser": {:hex, :earmark_parser, "1.4.9", "819bda2049e6ee1365424e4ced1ba65806eacf0d2867415f19f3f80047f8037b", [:mix], [], "hexpm", "8bf54fddabf2d7e137a0c22660e71b49d5a0a82d1fb05b5af62f2761cd6485c4"}, + "ex_doc": {:hex, :ex_doc, "0.22.1", "9bb6d51508778193a4ea90fa16eac47f8b67934f33f8271d5e1edec2dc0eee4c", [:mix], [{:earmark, "~> 1.4.0", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm", "d957de1b75cb9f78d3ee17820733dc4460114d8b1e11f7ee4fd6546e69b1db60"}, "ex_spec": {:hex, :ex_spec, "2.0.1", "8bdbd6fa85995fbf836ed799571d44be6f9ebbcace075209fd0ad06372c111cf", [:mix], [], "hexpm", "b44fe5054497411a58341ece5bf7756c219d9d6c1303b5ac467f557a0a4c31ac"}, "freedom_formatter": {:hex, :freedom_formatter, "1.0.0", "b19be4a845082d05d32bb23765e8e7bdc6d51decac13ab64ae44b0f6bf8a66d1", [:mix], [], "hexpm"}, "fuzzyurl": {:hex, :fuzzyurl, "1.0.1", "389780519adccfc3582ecce8c6608c1619f029367abfdc9d4b6e46491d2fa8d5", [:mix], [], "hexpm"}, - "jason": {:hex, :jason, "1.1.2", "b03dedea67a99223a2eaf9f1264ce37154564de899fd3d8b9a21b1a6fd64afe7", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fdf843bca858203ae1de16da2ee206f53416bbda5dc8c9e78f43243de4bc3afe"}, - "makeup": {:hex, :makeup, "0.8.0", "9cf32aea71c7fe0a4b2e9246c2c4978f9070257e5c9ce6d4a28ec450a839b55f", [:mix], [{:nimble_parsec, "~> 0.5.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "5fbc8e549aa9afeea2847c0769e3970537ed302f93a23ac612602e805d9d1e7f"}, - "makeup_elixir": {:hex, :makeup_elixir, "0.13.0", "be7a477997dcac2e48a9d695ec730b2d22418292675c75aa2d34ba0909dcdeda", [:mix], [{:makeup, "~> 0.8", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "adf0218695e22caeda2820eaba703fa46c91820d53813a2223413da3ef4ba515"}, + "jason": {:hex, :jason, "1.2.1", "12b22825e22f468c02eb3e4b9985f3d0cb8dc40b9bd704730efa11abd2708c44", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b659b8571deedf60f79c5a608e15414085fa141344e2716fbd6988a084b5f993"}, + "makeup": {:hex, :makeup, "1.0.3", "e339e2f766d12e7260e6672dd4047405963c5ec99661abdc432e6ec67d29ef95", [:mix], [{:nimble_parsec, "~> 0.5", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "2e9b4996d11832947731f7608fed7ad2f9443011b3b479ae288011265cdd3dad"}, + "makeup_elixir": {:hex, :makeup_elixir, "0.14.1", "4f0e96847c63c17841d42c08107405a005a2680eb9c7ccadfd757bd31dabccfb", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "f2438b1a80eaec9ede832b5c41cd4f373b38fd7aa33e3b22d9db79e640cbde11"}, "mime": {:hex, :mime, "1.3.1", "30ce04ab3175b6ad0bdce0035cba77bba68b813d523d1aac73d9781b4d193cf8", [:mix], [], "hexpm", "6cbe761d6a0ca5a31a0931bf4c63204bceb64538e664a8ecf784a9a6f3b875f1"}, "mint": {:hex, :mint, "1.1.0", "1fd0189edd9e3ffdbd7fcd8bc3835902b987a63ec6c4fd1aa8c2a56e2165f252", [:mix], [{:castore, "~> 0.1.0", [hex: :castore, repo: "hexpm", optional: true]}], "hexpm", "5bfd316c3789340b682d5679a8116bcf2112e332447bdc20c1d62909ee45f48d"}, - "nimble_parsec": {:hex, :nimble_parsec, "0.5.0", "90e2eca3d0266e5c53f8fbe0079694740b9c91b6747f2b7e3c5d21966bba8300", [:mix], [], "hexpm", "5c040b8469c1ff1b10093d3186e2e10dbe483cd73d79ec017993fb3985b8a9b3"}, + "nimble_parsec": {:hex, :nimble_parsec, "0.6.0", "32111b3bf39137144abd7ba1cce0914533b2d16ef35e8abc5ec8be6122944263", [:mix], [], "hexpm", "27eac315a94909d4dc68bc07a4a83e06c8379237c5ea528a9acff4ca1c873c52"}, "plug": {:hex, :plug, "1.10.3", "c9cebe917637d8db0e759039cc106adca069874e1a9034fd6e3fdd427fd3c283", [:mix], [{:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "01f9037a2a1de1d633b5a881101e6a444bcabb1d386ca1e00bb273a1f1d9d939"}, "plug_cowboy": {:hex, :plug_cowboy, "2.3.0", "149a50e05cb73c12aad6506a371cd75750c0b19a32f81866e1a323dda9e0e99d", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "bc595a1870cef13f9c1e03df56d96804db7f702175e4ccacdb8fc75c02a7b97e"}, "plug_crypto": {:hex, :plug_crypto, "1.1.2", "bdd187572cc26dbd95b87136290425f2b580a116d3fb1f564216918c9730d227", [:mix], [], "hexpm", "6b8b608f895b6ffcfad49c37c7883e8df98ae19c6a28113b02aa1e9c5b22d6b5"}, diff --git a/test/mojito_test.exs b/test/mojito_test.exs index 8a98cff..3f61913 100644 --- a/test/mojito_test.exs +++ b/test/mojito_test.exs @@ -221,13 +221,6 @@ defmodule MojitoTest do assert({:error, %Error{reason: :timeout}} = get("/wait1", timeout: 100)) end - it "can http/2 via Mint" do - port = Application.get_env(:mojito, :test_server_http_port) - - assert {:ok, mint_conn} = - Mint.HTTP.connect(:http, "localhost", port, protocols: [:http2]) - end - it "handles timeouts even on long requests" do port = Application.get_env(:mojito, :test_server_http_port) {:ok, conn} = Mojito.Conn.connect("http://localhost:#{port}") @@ -374,10 +367,34 @@ defmodule MojitoTest do assert("close" == response.body) end - it "can POST big bodies" do - one_meg = String.duplicate("x", 0x100000) - body = %{data: one_meg} |> Jason.encode!() + it "can POST big bodies over HTTP/1" do + big = String.duplicate("x", 5_000_000) + body = %{name: big} + assert({:ok, response} = post("/post", body, protocols: [:http1])) + assert({:ok, map} = Jason.decode(response.body)) + assert(%{"name" => big} == map) + end + + it "can POST big bodies over HTTP/2" do + big = String.duplicate("x", 5_000_000) + body = %{name: big} assert({:ok, response} = post("/post", body, protocols: [:http2])) + assert({:ok, map} = Jason.decode(response.body)) + assert(%{"name" => big} == map) + end + + it "handles response chunks arriving during stream_request_body" do + ## sending a body this big will trigger a 500 error in Cowboy + ## because we have not configured it otherwise + big = String.duplicate("x", 100_000_000) + body = %{name: big} + + assert( + {:ok, response} = + post("/post", body, protocols: [:http2], timeout: 15_000) + ) + + assert(500 == response.status_code) end end From e764eb25e8da2a818de3d85f9764baec75c37d0c Mon Sep 17 00:00:00 2001 From: pete gamache Date: Fri, 3 Jul 2020 12:23:12 -0400 Subject: [PATCH 04/11] tests pass --- lib/mojito/conn.ex | 8 ++++---- lib/mojito/conn_server.ex | 39 ++++++++++++++++++++++-------------- lib/mojito/request/single.ex | 2 -- lib/mojito/response.ex | 2 ++ test/mojito_test.exs | 2 +- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/lib/mojito/conn.ex b/lib/mojito/conn.ex index 10eb562..7789a3b 100644 --- a/lib/mojito/conn.ex +++ b/lib/mojito/conn.ex @@ -56,7 +56,7 @@ defmodule Mojito.Conn do @spec request(t, Mojito.request()) :: {:ok, t, reference} | {:error, any} def request(conn, request) do max_body_size = request.opts[:max_body_size] - response = %Mojito.Response{size: max_body_size} + response = %Mojito.Response{body: [], size: max_body_size} with {:ok, relative_url, auth_headers} <- Utils.get_relative_url_and_auth_headers(request.url), @@ -79,9 +79,9 @@ defmodule Mojito.Conn do end defp stream_request_body(mint_conn, request_ref, response, "") do - with {:ok, mint_conn, _ref} <- + with {:ok, mint_conn} <- Mint.HTTP.stream_request_body(mint_conn, request_ref, :eof) do - {:ok, mint_conn, request_ref, response} + {:ok, mint_conn, response} end end @@ -112,7 +112,6 @@ defmodule Mojito.Conn do ) {chunk, rest} = String.split_at(body, chunk_size) - if "" == rest, do: IO.puts("last one") with {:ok, mint_conn} <- Mint.HTTP.stream_request_body(mint_conn, request_ref, chunk) do @@ -124,6 +123,7 @@ defmodule Mojito.Conn do end response = Mojito.Response.apply_resps(response, resps) + {mint_conn, response} else {mint_conn, response} diff --git a/lib/mojito/conn_server.ex b/lib/mojito/conn_server.ex index c1f964f..713a3cb 100644 --- a/lib/mojito/conn_server.ex +++ b/lib/mojito/conn_server.ex @@ -166,21 +166,30 @@ defmodule Mojito.ConnServer do ) :: {:ok, state, reference} | {:error, any} defp start_request(state, request, reply_to, response_ref) do with {:ok, state} <- ensure_connection(state, request.url, request.opts), - {:ok, conn, request_ref} <- Conn.request(state.conn, request) do - response = %Response{body: [], size: request.opts[:max_body_size]} - responses = state.responses |> Map.put(request_ref, response) - reply_tos = state.reply_tos |> Map.put(request_ref, reply_to) - response_refs = state.response_refs |> Map.put(request_ref, response_ref) - - state = %{ - state - | conn: conn, - responses: responses, - reply_tos: reply_tos, - response_refs: response_refs - } - - {:ok, state, request_ref} + {:ok, conn, request_ref, response} <- Conn.request(state.conn, request) do + case response do + %{complete: true} -> + ## Request was completed by server during stream_request_body + respond(reply_to, {:ok, response}, response_ref) + {:ok, %{state | conn: conn}, request_ref} + + _ -> + responses = state.responses |> Map.put(request_ref, response) + reply_tos = state.reply_tos |> Map.put(request_ref, reply_to) + + response_refs = + state.response_refs |> Map.put(request_ref, response_ref) + + state = %{ + state + | conn: conn, + responses: responses, + reply_tos: reply_tos, + response_refs: response_refs + } + + {:ok, state, request_ref} + end end end diff --git a/lib/mojito/request/single.ex b/lib/mojito/request/single.ex index 4b08ad2..3b90412 100644 --- a/lib/mojito/request/single.ex +++ b/lib/mojito/request/single.ex @@ -4,7 +4,6 @@ defmodule Mojito.Request.Single do @moduledoc false alias Mojito.{Config, Conn, Error, Request, Response} - alias Mojito.Utils require Logger @doc ~S""" @@ -28,7 +27,6 @@ defmodule Mojito.Request.Single do {:ok, conn} <- Conn.connect(req.url, req.opts), {:ok, conn, _ref, response} <- Conn.request(conn, req) do timeout = req.opts[:timeout] || Config.timeout() - max_body_size = req.opts[:max_body_size] receive_response(conn, response, timeout) end end diff --git a/lib/mojito/response.ex b/lib/mojito/response.ex index c1e9aeb..9bc9be7 100644 --- a/lib/mojito/response.ex +++ b/lib/mojito/response.ex @@ -1,6 +1,8 @@ defmodule Mojito.Response do @moduledoc false + alias Mojito.Utils + defstruct status_code: nil, headers: [], body: "", diff --git a/test/mojito_test.exs b/test/mojito_test.exs index 3f61913..bbb0837 100644 --- a/test/mojito_test.exs +++ b/test/mojito_test.exs @@ -391,7 +391,7 @@ defmodule MojitoTest do assert( {:ok, response} = - post("/post", body, protocols: [:http2], timeout: 15_000) + post("/post", body, protocols: [:http2], timeout: 10_000) ) assert(500 == response.status_code) From fe576403eba1c8d5eec6c4e3a14f14cfe00b43cb Mon Sep 17 00:00:00 2001 From: pete gamache Date: Fri, 3 Jul 2020 12:45:35 -0400 Subject: [PATCH 05/11] removed redundant apply_resp implementations --- lib/mojito/conn.ex | 2 +- lib/mojito/conn_server.ex | 32 +++++++++++++++++--------------- lib/mojito/request/single.ex | 14 ++++---------- lib/mojito/response.ex | 33 +++++++++++++++++++++------------ 4 files changed, 43 insertions(+), 38 deletions(-) diff --git a/lib/mojito/conn.ex b/lib/mojito/conn.ex index 7789a3b..66dadc4 100644 --- a/lib/mojito/conn.ex +++ b/lib/mojito/conn.ex @@ -122,7 +122,7 @@ defmodule Mojito.Conn do msg -> Mint.HTTP.stream(mint_conn, msg) end - response = Mojito.Response.apply_resps(response, resps) + {:ok, response} = Mojito.Response.apply_resps(response, resps) {mint_conn, response} else diff --git a/lib/mojito/conn_server.ex b/lib/mojito/conn_server.ex index 713a3cb..1877d96 100644 --- a/lib/mojito/conn_server.ex +++ b/lib/mojito/conn_server.ex @@ -110,22 +110,24 @@ defmodule Mojito.ConnServer do apply_resp(state, resp) |> apply_resps(rest) end - defp apply_resp(state, {:status, request_ref, status}) do - response = Map.get(state.responses, request_ref) - response = response |> Map.put(:status_code, status) + defp apply_resp(state, {:status, request_ref, _status} = msg) do + {:ok, response} = + Map.get(state.responses, request_ref) + |> Response.apply_resp(msg) + %{state | responses: Map.put(state.responses, request_ref, response)} end - defp apply_resp(state, {:headers, request_ref, headers}) do - response = Map.get(state.responses, request_ref) - response = response |> Map.put(:headers, headers) + defp apply_resp(state, {:headers, request_ref, _headers} = msg) do + {:ok, response} = + Map.get(state.responses, request_ref) + |> Response.apply_resp(msg) + %{state | responses: Map.put(state.responses, request_ref, response)} end - defp apply_resp(state, {:data, request_ref, chunk}) do - response = Map.get(state.responses, request_ref) - - case Utils.put_chunk(response, chunk) do + defp apply_resp(state, {:data, request_ref, _chunk} = msg) do + case Map.get(state.responses, request_ref) |> Response.apply_resp(msg) do {:ok, response} -> %{state | responses: Map.put(state.responses, request_ref, response)} @@ -134,11 +136,11 @@ defmodule Mojito.ConnServer do end end - defp apply_resp(state, {:done, request_ref}) do - r = Map.get(state.responses, request_ref) - body = :erlang.list_to_binary(r.body) - size = byte_size(body) - response = %{r | complete: true, body: body, size: size} + defp apply_resp(state, {:done, request_ref} = msg) do + {:ok, response} = + Map.get(state.responses, request_ref) + |> Response.apply_resp(msg) + halt(state, request_ref, {:ok, response}) end diff --git a/lib/mojito/request/single.ex b/lib/mojito/request/single.ex index 3b90412..7510953 100644 --- a/lib/mojito/request/single.ex +++ b/lib/mojito/request/single.ex @@ -73,17 +73,11 @@ defmodule Mojito.Request.Single do case Mint.HTTP.stream(conn.conn, msg) do {:ok, mint_conn, resps} -> conn = %{conn | conn: mint_conn} - response = Response.apply_resps(response, resps) - case response do - %{complete: true} -> - {:ok, response} - - {:error, _} = err -> - err - - _ -> - receive_response(conn, response, new_timeout.()) + case Response.apply_resps(response, resps) do + {:ok, %{complete: true} = response} -> {:ok, response} + {:error, _} = err -> err + _ -> receive_response(conn, response, new_timeout.()) end {:error, _, e, _} -> diff --git a/lib/mojito/response.ex b/lib/mojito/response.ex index 9bc9be7..077d849 100644 --- a/lib/mojito/response.ex +++ b/lib/mojito/response.ex @@ -14,27 +14,36 @@ defmodule Mojito.Response do @doc ~S""" Applies responses received from `Mint.HTTP.stream/2` to a %Mojito.Response{}. """ - @spec apply_resps(t, [Mint.Types.response()]) :: t - def apply_resps(response, []), do: response + @spec apply_resps(t, [Mint.Types.response()]) :: {:ok, t} | {:error, any} + def apply_resps(response, []), do: {:ok, response} def apply_resps(response, [mint_resp | rest]) do - apply_resp(response, mint_resp) |> apply_resps(rest) + with {:ok, response} <- apply_resp(response, mint_resp) do + apply_resps(response, rest) + end end - defp apply_resp(response, {:status, _request_ref, status_code}) do - %{response | status_code: status_code} + @doc ~S""" + Applies a response received from `Mint.HTTP.stream/2` to a %Mojito.Response{}. + """ + @spec apply_resps(t, Mint.Types.response()) :: {:ok, t} | {:error, any} + def apply_resp(response, {:status, _request_ref, status_code}) do + {:ok, %{response | status_code: status_code}} end - defp apply_resp(response, {:headers, _request_ref, headers}) do - %{response | headers: headers} + def apply_resp(response, {:headers, _request_ref, headers}) do + {:ok, %{response | headers: headers}} end - defp apply_resp(response, {:data, _request_ref, chunk}) do - {:ok, resp} = Utils.put_chunk(response, chunk) - resp + def apply_resp(response, {:data, _request_ref, chunk}) do + with {:ok, response} <- Utils.put_chunk(response, chunk) do + {:ok, response} + end end - defp apply_resp(response, {:done, _request_ref}) do - %{response | complete: true, body: :erlang.iolist_to_binary(response.body)} + def apply_resp(response, {:done, _request_ref}) do + body = :erlang.iolist_to_binary(response.body) + size = byte_size(body) + {:ok, %{response | complete: true, body: body, size: size}} end end From 329d047d4d6f15fb3264e8aed0335241367faf84 Mon Sep 17 00:00:00 2001 From: pete gamache Date: Fri, 3 Jul 2020 12:49:54 -0400 Subject: [PATCH 06/11] moved put_chunk into Mojito.Response --- lib/mojito/response.ex | 29 +++++++++++++++++++++++++---- lib/mojito/utils.ex | 23 +---------------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/lib/mojito/response.ex b/lib/mojito/response.ex index 077d849..5e04863 100644 --- a/lib/mojito/response.ex +++ b/lib/mojito/response.ex @@ -1,7 +1,7 @@ defmodule Mojito.Response do @moduledoc false - alias Mojito.Utils + alias Mojito.{Error, Response} defstruct status_code: nil, headers: [], @@ -12,7 +12,7 @@ defmodule Mojito.Response do @type t :: Mojito.response() @doc ~S""" - Applies responses received from `Mint.HTTP.stream/2` to a %Mojito.Response{}. + Applies responses received from `Mint.HTTP.stream/2` to a `%Mojito.Response{}`. """ @spec apply_resps(t, [Mint.Types.response()]) :: {:ok, t} | {:error, any} def apply_resps(response, []), do: {:ok, response} @@ -24,7 +24,7 @@ defmodule Mojito.Response do end @doc ~S""" - Applies a response received from `Mint.HTTP.stream/2` to a %Mojito.Response{}. + Applies a response received from `Mint.HTTP.stream/2` to a `%Mojito.Response{}`. """ @spec apply_resps(t, Mint.Types.response()) :: {:ok, t} | {:error, any} def apply_resp(response, {:status, _request_ref, status_code}) do @@ -36,7 +36,7 @@ defmodule Mojito.Response do end def apply_resp(response, {:data, _request_ref, chunk}) do - with {:ok, response} <- Utils.put_chunk(response, chunk) do + with {:ok, response} <- put_chunk(response, chunk) do {:ok, response} end end @@ -46,4 +46,25 @@ defmodule Mojito.Response do size = byte_size(body) {:ok, %{response | complete: true, body: body, size: size}} end + + @doc ~S""" + Adds chunks to a response body, respecting the `response.size` field. + `response.size` should be set to the maximum number of bytes to accept + as the response body, or `nil` for no limit. + """ + @spec put_chunk(t, binary) :: {:ok, %Response{}} | {:error, any} + def put_chunk(%Response{size: nil} = response, chunk) do + {:ok, %{response | body: [response.body | [chunk]]}} + end + + def put_chunk(%Response{size: remaining} = response, chunk) do + case remaining - byte_size(chunk) do + over_limit when over_limit < 0 -> + {:error, %Error{reason: :max_body_size_exceeded}} + + new_remaining -> + {:ok, + %{response | body: [response.body | [chunk]], size: new_remaining}} + end + end end diff --git a/lib/mojito/utils.ex b/lib/mojito/utils.ex index ae016b5..199a69d 100644 --- a/lib/mojito/utils.ex +++ b/lib/mojito/utils.ex @@ -1,7 +1,7 @@ defmodule Mojito.Utils do @moduledoc false - alias Mojito.{Error, Response} + alias Mojito.Error @doc ~S""" Ensures that the return value errors are of the form @@ -101,25 +101,4 @@ defmodule Mojito.Utils do def protocol_to_transport(proto), do: {:error, "unknown protocol #{inspect(proto)}"} - - @doc ~S""" - Adds chunks to a response body, respecting the `response.size` field. - `response.size` should be set to the maximum number of bytes to accept - as the response body, or `nil` for no limit. - """ - @spec put_chunk(%Response{}, binary) :: {:ok, %Response{}} | {:error, any} - def put_chunk(%Response{size: nil} = response, chunk) do - {:ok, %{response | body: [response.body | [chunk]]}} - end - - def put_chunk(%Response{size: remaining} = response, chunk) do - case remaining - byte_size(chunk) do - over_limit when over_limit < 0 -> - {:error, %Error{reason: :max_body_size_exceeded}} - - new_remaining -> - {:ok, - %{response | body: [response.body | [chunk]], size: new_remaining}} - end - end end From 141df90f5d020849868659a6ad49d39aa5e69d65 Mon Sep 17 00:00:00 2001 From: pete gamache Date: Fri, 3 Jul 2020 12:54:48 -0400 Subject: [PATCH 07/11] test timeouts during stream_request_body --- lib/mojito/pool/poolboy/single.ex | 4 +--- test/mojito_test.exs | 10 ++++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/mojito/pool/poolboy/single.ex b/lib/mojito/pool/poolboy/single.ex index 44d1b8c..bc8c96a 100644 --- a/lib/mojito/pool/poolboy/single.ex +++ b/lib/mojito/pool/poolboy/single.ex @@ -157,9 +157,7 @@ defmodule Mojito.Pool.Poolboy.Single do rescue e -> {:error, e} catch - :exit, e -> - e |> inspect |> Logger.error() - {:error, :checkout_timeout} + :exit, e -> {:error, :checkout_timeout} after Process.flag(:trap_exit, old_trap_exit) end diff --git a/test/mojito_test.exs b/test/mojito_test.exs index bbb0837..8d97907 100644 --- a/test/mojito_test.exs +++ b/test/mojito_test.exs @@ -396,6 +396,16 @@ defmodule MojitoTest do assert(500 == response.status_code) end + + it "handles timeouts during stream_request_body" do + big = String.duplicate("x", 5_000_000) + body = %{name: big} + + assert( + {:error, %{reason: :timeout}} = + post("/post", body, protocols: [:http2], timeout: 100) + ) + end end context "external tests" do From 78873a8594a857ca5df805af743844efdf3c3f42 Mon Sep 17 00:00:00 2001 From: pete gamache Date: Fri, 3 Jul 2020 13:09:07 -0400 Subject: [PATCH 08/11] remove unused variable --- lib/mojito/pool/poolboy/single.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mojito/pool/poolboy/single.ex b/lib/mojito/pool/poolboy/single.ex index bc8c96a..f0bf2c5 100644 --- a/lib/mojito/pool/poolboy/single.ex +++ b/lib/mojito/pool/poolboy/single.ex @@ -157,7 +157,7 @@ defmodule Mojito.Pool.Poolboy.Single do rescue e -> {:error, e} catch - :exit, e -> {:error, :checkout_timeout} + :exit, _ -> {:error, :checkout_timeout} after Process.flag(:trap_exit, old_trap_exit) end From e11b29fd65264dbc46452ccf3cf0d8690263e2de Mon Sep 17 00:00:00 2001 From: pete gamache Date: Fri, 3 Jul 2020 13:22:29 -0400 Subject: [PATCH 09/11] update Travis config to use Elixir 1.9 and 1.10 --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1e058e3..f8b9da7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: elixir elixir: - - 1.7 - - 1.8 + - 1.9 + - 1.10 script: - "MIX_ENV=test mix do deps.get, compile, test" From f8b5c47b950668ff2e9d92c51879de4848a8005b Mon Sep 17 00:00:00 2001 From: pete gamache Date: Fri, 3 Jul 2020 14:53:20 -0400 Subject: [PATCH 10/11] update OTP in Travis --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index f8b9da7..dff8807 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,8 @@ language: elixir elixir: - - 1.9 - 1.10 +otp_release: + - 22.3 script: - "MIX_ENV=test mix do deps.get, compile, test" From 740bcf62d8945eea64c56f99081e2c0891d14095 Mon Sep 17 00:00:00 2001 From: pete gamache Date: Fri, 3 Jul 2020 15:07:29 -0400 Subject: [PATCH 11/11] fix booboo in request/single.ex --- .travis.yml | 2 +- lib/mojito/pool/poolboy/single.ex | 2 -- lib/mojito/request/single.ex | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index dff8807..7aa0f48 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,5 +4,5 @@ elixir: otp_release: - 22.3 script: - - "MIX_ENV=test mix do deps.get, compile, test" + - "MIX_ENV=test mix do deps.get, compile, test --trace" diff --git a/lib/mojito/pool/poolboy/single.ex b/lib/mojito/pool/poolboy/single.ex index f0bf2c5..30f4fa1 100644 --- a/lib/mojito/pool/poolboy/single.ex +++ b/lib/mojito/pool/poolboy/single.ex @@ -1,8 +1,6 @@ defmodule Mojito.Pool.Poolboy.Single do @moduledoc false - require Logger - ## Mojito.Pool.Poolboy.Single provides an HTTP request connection pool based on ## Mojito and Poolboy. ## diff --git a/lib/mojito/request/single.ex b/lib/mojito/request/single.ex index 7510953..00a7cd9 100644 --- a/lib/mojito/request/single.ex +++ b/lib/mojito/request/single.ex @@ -76,8 +76,8 @@ defmodule Mojito.Request.Single do case Response.apply_resps(response, resps) do {:ok, %{complete: true} = response} -> {:ok, response} - {:error, _} = err -> err - _ -> receive_response(conn, response, new_timeout.()) + {:ok, response} -> receive_response(conn, response, new_timeout.()) + err -> err end {:error, _, e, _} ->