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

Replace hackney with httpc #311

Merged
merged 8 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ jobs:
- uses: actions/cache@v3
with:
path: deps
key: ${{ runner.os }}-mix-${{ hashFiles(format('{0}{1}', github.workspace, '/mix.lock')) }}
key: ${{ runner.os }}-${{ matrix.elixir }}-otp${{ matrix.otp }}-mix-${{ hashFiles(format('{0}{1}', github.workspace, '/mix.lock')) }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parroty having a single cache for all OTP versions causes issues when testing across different versions. Most projects I've used actually go with a per-version cache, so this is what this change is.

restore-keys: |
${{ runner.os }}-mix-
${{ runner.os }}-${{ matrix.elixir }}-otp${{ matrix.otp }}-mix-
- run: mix deps.get
- run: mix coveralls.github
148 changes: 114 additions & 34 deletions lib/excoveralls/poster.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,10 @@ defmodule ExCoveralls.Poster do
@file_name "excoveralls.post.json.gz"

@doc """
Create a temporarily json file and post it to server using hackney library.
Then, remove the file after it's completed.
Compresses the given `json` and posts it to the coveralls server.
"""
def execute(json, options \\ []) do
File.write!(@file_name, json |> :zlib.gzip())
response = send_file(@file_name, options)
File.rm!(@file_name)

case response do
case json |> :zlib.gzip() |> upload_zipped_json(options) do
{:ok, message} ->
IO.puts(message)

Expand All @@ -22,44 +17,129 @@ defmodule ExCoveralls.Poster do
end
end

defp send_file(file_name, options) do
Application.ensure_all_started(:hackney)
defp upload_zipped_json(content, options) do
Application.ensure_all_started(:ssl)
Application.ensure_all_started(:httpc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why does :httpc needs to be started explicitly? Starting :inets.start() and :ssl.start() typically was sufficient for me in the past 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably is sufficient, I just overlooked this. PRs welcome 😉

Copy link
Owner

@parroty parroty Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment. Do you see specific negative implication for this extra start logic? (Just a small overhead?).

I'll be publishing the release later today (after going back home), but wondering whether to update this before the release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parroty httpc is not an app. Just merge #318 before releasing 😉

Application.ensure_all_started(:inets)

endpoint = options[:endpoint] || "https://coveralls.io"

response =
:hackney.request(
:post,
"#{endpoint}/api/v1/jobs",
[],
{:multipart,
[
{:file, file_name, {"form-data", [{"name", "json_file"}, {"filename", file_name}]},
[{"Content-Type", "gzip/json"}]}
]},
[{:recv_timeout, 10_000}]
)

case response do
{:ok, status_code, _, _} when status_code in 200..299 ->
multipart_boundary =
"---------------------------" <> Base.encode16(:crypto.strong_rand_bytes(8), case: :lower)

body =
[
"--#{multipart_boundary}",
"content-length: #{byte_size(content)}",
"content-disposition: form-data; name=json_file; filename=#{@file_name}",
"content-type: gzip/json",
"",
content,
"--#{multipart_boundary}--"
]
|> Enum.join("\r\n")

headers = [
{~c"Host", String.to_charlist(URI.parse(endpoint).host)},
{~c"User-Agent", ~c"excoveralls"},
{~c"Content-Length", String.to_charlist(Integer.to_string(byte_size(body)))},
{~c"Accept", ~c"*/*"}
]

# All header names and values MUST be charlists in older OTP versions. In newer versions,
# binaries are fine. This is hard to debug because httpc simply *hangs* on older OTP
# versions if you use a binary value.
if Enum.any?(headers, fn {_, val} -> not is_list(val) end) do
raise "all header names and values must be charlists"
end

request = {
String.to_charlist(endpoint) ++ ~c"/api/v1/jobs",
headers,
_content_type = ~c"multipart/form-data; boundary=#{multipart_boundary}",
body
}

http_options = [
timeout: 10_000,
ssl:
[
verify: :verify_peer,
depth: 2,
customize_hostname_check: [
match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
]
# https://erlef.github.io/security-wg/secure_coding_and_deployment_hardening/inets
] ++ cacert_option()
]

case :httpc.request(:post, request, http_options, sync: true, body_format: :binary) do
{:ok, {{_protocol, status_code, _status_message}, _headers, _body}}
when status_code in 200..299 ->
{:ok, "Successfully uploaded the report to '#{endpoint}'."}

{:ok, 500 = _status_code, _, _client} ->
{:ok, "API endpoint `#{endpoint}` is not available and return internal server error! Ignoring upload"}
{:ok, 405 = _status_code, _, _client} ->
{:ok, {{_protocol, 500, _status_message}, _headers, _body}} ->
{:ok,
"API endpoint `#{endpoint}` is not available and return internal server error! Ignoring upload"}

{:ok, {{_protocol, 405, _status_message}, _headers, _body}} ->
{:ok, "API endpoint `#{endpoint}` is not available due to maintenance! Ignoring upload"}
{:ok, status_code, _, client} ->
{:ok, body} = :hackney.body(client)

{:ok, {{_protocol, status_code, _status_message}, _headers, body}} ->
{:error,
"Failed to upload the report to '#{endpoint}' (reason: status_code = #{status_code}, body = #{
body
})."}
"Failed to upload the report to '#{endpoint}' (reason: status_code = #{status_code}, body = #{body})."}

{:error, reason} when reason in [:timeout, :connect_timeout] ->
{:ok, "Unable to upload the report to '#{endpoint}' due to a timeout. Not failing the build."}
{:error, reason} when reason in [:timeout, :connect_timeout] ->
{:ok,
"Unable to upload the report to '#{endpoint}' due to a timeout. Not failing the build."}

{:error, reason} ->
{:error, "Failed to upload the report to '#{endpoint}' (reason: #{inspect(reason)})."}
end
end

# TODO: remove this once we depend on an Elixir version that requires OTP 25+.
if System.otp_release() >= "25" do
defp cacert_option do
if Code.ensure_loaded?(CAStore) do
[cacertfile: String.to_charlist(CAStore.file_path())]
else
case :public_key.cacerts_load() do
:ok ->
[cacerts: :public_key.cacerts_get()]

{:error, reason} ->
raise ExCoveralls.ReportUploadError,
message: """
Failed to load OS certificates. We tried to use OS certificates because we
couldn't find the :castore library. If you want to use :castore, please add

{:castore, "~> 1.0"}

to your dependencies. Otherwise, make sure you can load OS certificates by
running :public_key.cacerts_load() and checking the result. The error we
got was:

#{inspect(reason)}
"""
end
end
end
else
defp cacert_option do
if Code.ensure_loaded?(CAStore) do
[cacertfile: String.to_charlist(CAStore.file_path())]
else
raise ExCoveralls.ReportUploadError,
message: """
Failed to use any SSL certificates. We didn't find the :castore library,
and we couldn't use OS certificates because that requires OTP 25 or later.
If you want to use :castore, please add

{:castore, "~> 1.0"}

"""
end
end
end
end
7 changes: 4 additions & 3 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,17 @@ defmodule ExCoveralls.Mixfile do
end

def application do
[extra_applications: [:eex, :tools, :xmerl]]
[extra_applications: [:eex, :tools, :xmerl, :inets, :ssl, :public_key]]
end

defp elixirc_paths(:test), do: ["lib", "test/fixtures/test_missing.ex"]
defp elixirc_paths(_), do: ["lib"]

def deps do
defp deps do
[
{:castore, "~> 1.0", optional: true},
{:jason, "~> 1.0"},
{:hackney, "~> 1.16"},
{:bypass, "~> 2.1.0", only: :test},
{:ex_doc, ">= 0.0.0", only: :dev, runtime: false},
{:meck, "~> 0.8", only: :test},
{:mock, "~> 0.3.6", only: :test},
Expand Down
20 changes: 11 additions & 9 deletions mix.lock
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
%{
"certifi": {:hex, :certifi, "2.9.0", "6f2a475689dd47f19fb74334859d460a2dc4e3252a3324bd2111b8f0429e7e21", [:rebar3], [], "hexpm", "266da46bdb06d6c6d35fde799bcb28d36d985d424ad7c08b5bb48f5b5cdd4641"},
"earmark": {:hex, :earmark, "1.4.3", "364ca2e9710f6bff494117dbbd53880d84bebb692dafc3a78eb50aa3183f2bfd", [:mix], [], "hexpm", "8cf8a291ebf1c7b9539e3cddb19e9cef066c2441b1640f13c34c1d3cfc825fec"},
"bypass": {:hex, :bypass, "2.1.0", "909782781bf8e20ee86a9cabde36b259d44af8b9f38756173e8f5e2e1fabb9b1", [:mix], [{:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.0", [hex: :plug_cowboy, repo: "hexpm", optional: false]}, {:ranch, "~> 1.3", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "d9b5df8fa5b7a6efa08384e9bbecfe4ce61c77d28a4282f79e02f1ef78d96b80"},
"castore": {:hex, :castore, "1.0.3", "7130ba6d24c8424014194676d608cb989f62ef8039efd50ff4b3f33286d06db8", [:mix], [], "hexpm", "680ab01ef5d15b161ed6a95449fac5c6b8f60055677a8e79acf01b27baa4390b"},
"cowboy": {:hex, :cowboy, "2.10.0", "ff9ffeff91dae4ae270dd975642997afe2a1179d94b1887863e43f681a203e26", [:make, :rebar3], [{:cowlib, "2.12.1", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "1.8.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "3afdccb7183cc6f143cb14d3cf51fa00e53db9ec80cdcd525482f5e99bc41d6b"},
"cowboy_telemetry": {:hex, :cowboy_telemetry, "0.4.0", "f239f68b588efa7707abce16a84d0d2acf3a0f50571f8bb7f56a15865aae820c", [:rebar3], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7d98bac1ee4565d31b62d59f8823dfd8356a169e7fcbb83831b8a5397404c9de"},
"cowlib": {:hex, :cowlib, "2.12.1", "a9fa9a625f1d2025fe6b462cb865881329b5caff8f1854d1cbc9f9533f00e1e1", [:make, :rebar3], [], "hexpm", "163b73f6367a7341b33c794c4e88e7dbfe6498ac42dcd69ef44c5bc5507c8db0"},
"earmark_parser": {:hex, :earmark_parser, "1.4.12", "b245e875ec0a311a342320da0551da407d9d2b65d98f7a9597ae078615af3449", [:mix], [], "hexpm", "711e2cc4d64abb7d566d43f54b78f7dc129308a63bc103fbd88550d2174b3160"},
"ex_doc": {:hex, :ex_doc, "0.23.0", "a069bc9b0bf8efe323ecde8c0d62afc13d308b1fa3d228b65bca5cf8703a529d", [:mix], [{:earmark_parser, "~> 1.4.0", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm", "f5e2c4702468b2fd11b10d39416ddadd2fcdd173ba2a0285ebd92c39827a5a16"},
"hackney": {:hex, :hackney, "1.18.1", "f48bf88f521f2a229fc7bae88cf4f85adc9cd9bcf23b5dc8eb6a1788c662c4f6", [:rebar3], [{:certifi, "~> 2.9.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~> 6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~> 1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~> 1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.3.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~> 1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "a4ecdaff44297e9b5894ae499e9a070ea1888c84afdd1fd9b7b2bc384950128e"},
"idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"},
"jason": {:hex, :jason, "1.1.1", "d3ccb840dfb06f2f90a6d335b536dd074db748b3e7f5b11ab61d239506585eb2", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "639645cfac325e34938167b272bae0791fea3a34cf32c29525abf1d323ed4c18"},
"makeup": {:hex, :makeup, "1.0.5", "d5a830bc42c9800ce07dd97fa94669dfb93d3bf5fcf6ea7a0c67b2e0e4a7f26c", [:mix], [{:nimble_parsec, "~> 0.5 or ~> 1.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cfa158c02d3f5c0c665d0af11512fed3fba0144cf1aadee0f2ce17747fba2ca9"},
"makeup_elixir": {:hex, :makeup_elixir, "0.15.0", "98312c9f0d3730fde4049985a1105da5155bfe5c11e47bdc7406d88e01e4219b", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.1", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "75ffa34ab1056b7e24844c90bfc62aaf6f3a37a15faa76b07bc5eba27e4a8b4a"},
"meck": {:hex, :meck, "0.8.13", "ffedb39f99b0b99703b8601c6f17c7f76313ee12de6b646e671e3188401f7866", [:rebar3], [], "hexpm", "d34f013c156db51ad57cc556891b9720e6a1c1df5fe2e15af999c84d6cebeb1a"},
"metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
"mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"},
"mime": {:hex, :mime, "2.0.5", "dc34c8efd439abe6ae0343edbb8556f4d63f178594894720607772a041b04b02", [:mix], [], "hexpm", "da0d64a365c45bc9935cc5c8a7fc5e49a0e0f9932a761c55d6c52b142780a05c"},
"mock": {:hex, :mock, "0.3.6", "e810a91fabc7adf63ab5fdbec5d9d3b492413b8cda5131a2a8aa34b4185eb9b4", [:mix], [{:meck, "~> 0.8.13", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "bcf1d0a6826fb5aee01bae3d74474669a3fa8b2df274d094af54a25266a1ebd2"},
"nimble_parsec": {:hex, :nimble_parsec, "1.1.0", "3a6fca1550363552e54c216debb6a9e95bd8d32348938e13de5eda962c0d7f89", [:mix], [], "hexpm", "08eb32d66b706e913ff748f11694b17981c0b04a33ef470e33e11b3d3ac8f54b"},
"parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"},
"plug": {:hex, :plug, "1.14.2", "cff7d4ec45b4ae176a227acd94a7ab536d9b37b942c8e8fa6dfc0fff98ff4d80", [:mix], [{:mime, "~> 1.0 or ~> 2.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.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "842fc50187e13cf4ac3b253d47d9474ed6c296a8732752835ce4a86acdf68d13"},
"plug_cowboy": {:hex, :plug_cowboy, "2.6.1", "9a3bbfceeb65eff5f39dab529e5cd79137ac36e913c02067dba3963a26efe9b2", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:cowboy_telemetry, "~> 0.3", [hex: :cowboy_telemetry, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "de36e1a21f451a18b790f37765db198075c25875c64834bcc82d90b309eb6613"},
"plug_crypto": {:hex, :plug_crypto, "1.2.5", "918772575e48e81e455818229bf719d4ab4181fcbf7f85b68a35620f78d89ced", [:mix], [], "hexpm", "26549a1d6345e2172eb1c233866756ae44a9609bd33ee6f99147ab3fd87fd842"},
"ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
"sax_map": {:hex, :sax_map, "1.0.1", "51a9382d741504c34d49118fb36d691c303d042e1da88f8edae8ebe75fe74435", [:mix], [{:saxy, "~> 1.0", [hex: :saxy, repo: "hexpm", optional: false]}], "hexpm", "a7c57c25d23bfc3ce93cf93400dcfb447fe463d27ee8c6913545161e78dc487a"},
"saxy": {:hex, :saxy, "0.10.0", "38879f46a595862c22114792c71379355ecfcfa0f713b1cfcc59e1d4127f1f55", [:mix], [], "hexpm", "da130ed576e9f53d1a986ec5bd2fa72c1599501ede7d7a2dceb81acf53bf9790"},
"ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"},
"unicode_util_compat": {:hex, :unicode_util_compat, "0.7.0", "bc84380c9ab48177092f43ac89e4dfa2c6d62b40b8bd132b1059ecc7232f9a78", [:rebar3], [], "hexpm", "25eee6d67df61960cf6a794239566599b09e17e668d3700247bc498638152521"},
"telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"},
}
51 changes: 33 additions & 18 deletions test/poster_test.exs
Original file line number Diff line number Diff line change
@@ -1,36 +1,51 @@
defmodule PosterTest do
use ExUnit.Case
import Mock
import ExUnit.CaptureIO

test_with_mock "post json", :hackney, [request: fn(_, _, _, _, _) -> {:ok, 200, "", ""} end] do

setup do
bypass = Bypass.open()
%{bypass: bypass, endpoint: "http://localhost:#{bypass.port}"}
end

test "successfully posting JSON", %{bypass: bypass, endpoint: endpoint} do
Bypass.expect(bypass, fn conn ->
assert conn.method == "POST"
assert {"host", "localhost"} in conn.req_headers
Plug.Conn.resp(conn, 200, "")
end)

assert capture_io(fn ->
ExCoveralls.Poster.execute("json")
end) =~ ~r/Successfully uploaded/
ExCoveralls.Poster.execute("{}", endpoint: endpoint)
end) =~ "Successfully uploaded"
end

test_with_mock "post json fails", :hackney, [request: fn(_, _, _, _, _) -> {:error, "failed"} end] do
test "post JSON fails", %{bypass: bypass, endpoint: endpoint} do
Bypass.down(bypass)

assert_raise ExCoveralls.ReportUploadError, fn ->
ExCoveralls.Poster.execute("json")
ExCoveralls.Poster.execute("{}", endpoint: endpoint)
end
end

test_with_mock "post json timeout", :hackney, [request: fn(_, _, _, _, _) -> {:error, :timeout} end,
request: fn(_, _, _, _, _) -> {:error, :connect_timeout} end] do
assert capture_io(fn ->
assert ExCoveralls.Poster.execute("json") == :ok
end) =~ ~r/timeout/
end

test_with_mock "post json fails due internal server error", :hackney, [request: fn(_, _, _, _, _) -> {:ok, 500, "", ""} end] do
test "post JSON fails due internal server error", %{bypass: bypass, endpoint: endpoint} do
Bypass.expect(bypass, fn conn ->
assert conn.method == "POST"
Plug.Conn.resp(conn, 500, "")
end)

assert capture_io(fn ->
assert ExCoveralls.Poster.execute("json") == :ok
assert ExCoveralls.Poster.execute("{}", endpoint: endpoint) == :ok
end) =~ ~r/internal server error/
end

test_with_mock "post json fails due to maintenance", :hackney, [request: fn(_, _, _, _, _) -> {:ok, 405, "", ""} end] do
test "post JSON fails due to maintenance", %{bypass: bypass, endpoint: endpoint} do
Bypass.expect(bypass, fn conn ->
assert conn.method == "POST"
Plug.Conn.resp(conn, 405, "")
end)

assert capture_io(fn ->
assert ExCoveralls.Poster.execute("json") == :ok
assert ExCoveralls.Poster.execute("{}", endpoint: endpoint) == :ok
end) =~ ~r/maintenance/
end
end