use Req as http client (#81)

* use `Req` as http client * Update integration_test.exs * fix tests * drop support for elixir 1.12 * CI * remove `finch` as direct dependency * remove retry of failed request; Req handles this * integration test for timeout error * Update integration_test.exs * Update CHANGELOG.md
balexand · May 10, 2024 · f836b67 · f836b67
1 parent 537dcd8
commit f836b67
Show file tree

Hide file tree

Showing 13 changed files with 100 additions and 195 deletions.
diff --git a/.github/workflows/elixir-format.yml b/.github/workflows/elixir-format.yml
@@ -0,0 +1,22 @@
+name: Check Elixir format
+
+on: push
+
+jobs:
+  build:
+    name: Check Elixir format
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Elixir
+        uses: erlef/setup-beam@v1
+        with:
+          elixir-version: "1.16"
+          otp-version: "26"
+
+      - name: Install dependencies
+        run: mix deps.get
+
+      - name: Check format
+        run: mix format --check-formatted
diff --git a/.github/workflows/elixir.yml b/.github/workflows/elixir.yml
@@ -18,8 +18,6 @@ jobs:
             otp-version: "25"
           - elixir-version: "1.13"
             otp-version: "24"
-          - elixir-version: "1.12"
-            otp-version: "24"
 
     steps:
       - uses: actions/checkout@v4
@@ -43,5 +41,3 @@ jobs:
           ELIXIR_SANITY_TEST_TOKEN: ${{ secrets.ELIXIR_SANITY_TEST_TOKEN }}
         if: env.ELIXIR_SANITY_TEST_TOKEN
         run: mix test --warnings-as-errors --only integration
-      - name: Check format
-        run: mix format --check-formatted
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Changed
+- (BREAKING) Switch HTTP client from `finch` to `req` (https://github.com/balexand/sanity/pull/81). This introduces the following breaking changes:
+  - The `headers` field of the `Sanity.Response` now returns a map instead of a list of tuples. See https://hexdocs.pm/req/changelog.html#change-headers-to-be-maps for details.
+  - The `:max_attempts` and `:retry_delay` options have been removed from `Sanity.request/2`. `Req` handles retries for us.
+  - The `source` field in the `Sanity.Error` exception may now contain a `Req.Response` struct instead of a `Finch.Response`.
 
 ## [1.3.0] - 2023-07-19
 ### Changed

diff --git a/lib/sanity.ex b/lib/sanity.ex
@@ -36,32 +36,20 @@ defmodule Sanity do
       doc: "Sanity dataset.",
       required: true
     ],
-    finch_mod: [
-      type: :atom,
-      default: Finch,
-      doc: false
-    ],
     http_options: [
       type: :keyword_list,
       default: [receive_timeout: 30_000],
-      doc: "Options to be passed to `Finch.request/3`."
-    ],
-    max_attempts: [
-      type: :pos_integer,
-      default: 1,
-      doc:
-        "Number of attempts to make before returning error. Requests receiving an HTTP status code of 4xx will not be retried."
+      doc: "Options to be passed to `Req.request/2`."
     ],
     project_id: [
       type: :string,
       doc: "Sanity project ID.",
       required: true
     ],
-    retry_delay: [
-      type: :pos_integer,
-      default: 1_000,
-      doc:
-        "Delay in ms to wait before retrying after an error. Applies if `max_attempts` is greater than `1`."
+    req_mod: [
+      type: :atom,
+      default: Req,
+      doc: false
     ],
     token: [
       type: :string,
@@ -226,7 +214,7 @@ defmodule Sanity do
       []
 
       iex> Sanity.result!(%Sanity.Response{body: %{}, status: 200})
-      ** (Sanity.Error) %Sanity.Response{body: %{}, headers: nil, status: 200}
+      ** (Sanity.Error) %Sanity.Response{body: %{}, headers: %{}, status: 200}
   """
   @spec result!(Response.t()) :: any()
   def result!(%Response{body: %{"result" => result}}), do: result
@@ -251,54 +239,27 @@ defmodule Sanity do
       ) do
     opts = NimbleOptions.validate!(opts, @request_options_schema)
 
-    finch_mod = Keyword.fetch!(opts, :finch_mod)
-    http_options = Keyword.fetch!(opts, :http_options)
-
-    url = "#{url_for(request, opts)}?#{URI.encode_query(query_params)}"
-
-    result =
-      Finch.build(method, url, headers(opts) ++ headers, body)
-      |> finch_mod.request(Sanity.Finch, http_options)
-
-    case {opts[:max_attempts], result} do
-      {_, {:ok, %Finch.Response{body: body, headers: headers, status: status}}}
+    Keyword.merge(Keyword.fetch!(opts, :http_options),
+      body: body,
+      headers: headers(opts) ++ headers,
+      method: method,
+      url: "#{url_for(request, opts)}?#{URI.encode_query(query_params)}"
+    )
+    |> Keyword.fetch!(opts, :req_mod).request()
+    |> case do
+      {:ok, %Req.Response{body: body, headers: headers, status: status}}
       when status in 200..299 ->
-        {:ok, %Response{body: Jason.decode!(body), headers: headers, status: status}}
+        {:ok, %Response{body: body, headers: headers, status: status}}
 
-      {_, {:ok, %Finch.Response{body: body, headers: headers, status: status} = resp}}
+      {:ok, %Req.Response{body: %{} = body, headers: headers, status: status}}
       when status in 400..499 ->
-        if json_resp?(headers) do
-          {:error, %Response{body: Jason.decode!(body), headers: headers, status: status}}
-        else
-          raise %Sanity.Error{source: resp}
-        end
-
-      {max_attempts, {_, error_or_response}} when max_attempts > 1 ->
-        Logger.warning(
-          "retrying failed request in #{opts[:retry_delay]}ms: #{inspect(error_or_response)}"
-        )
-
-        :timer.sleep(opts[:retry_delay])
-
-        opts =
-          opts
-          |> Keyword.update!(:max_attempts, &(&1 - 1))
-          |> Keyword.update!(:retry_delay, &(&1 * 2))
+        {:error, %Response{body: body, headers: headers, status: status}}
 
-        request(request, opts)
-
-      {_, {_, error_or_response}} ->
+      {_, error_or_response} ->
         raise %Sanity.Error{source: error_or_response}
     end
   end
 
-  defp json_resp?(headers) do
-    Enum.any?(headers, fn
-      {"content-type", value} -> String.contains?(value, "application/json")
-      {_name, _value} -> false
-    end)
-  end
-
   @doc """
   Like `request/2`, but raises a `Sanity.Error` instead of returning and error tuple.
 
@@ -343,8 +304,7 @@ defmodule Sanity do
     request_opts: [
       type: :keyword_list,
       required: true,
-      doc:
-        "Options to be passed to `request/2`. If `max_attempts` is omitted then it will default to `3`."
+      doc: "Options to be passed to `request/2`."
     ],
     variables: [
       type: {:map, {:or, [:atom, :string]}, :any},
@@ -371,10 +331,7 @@ defmodule Sanity do
   @impl true
   @spec stream(Keyword.t()) :: Enumerable.t()
   def stream(opts) do
-    opts =
-      opts
-      |> NimbleOptions.validate!(@stream_options_schema)
-      |> Keyword.update!(:request_opts, &Keyword.put_new(&1, :max_attempts, 3))
+    opts = NimbleOptions.validate!(opts, @stream_options_schema)
 
     case Map.take(opts[:variables], [:pagination_last_id, "pagination_last_id"]) |> Map.keys() do
       [] -> nil

diff --git a/lib/sanity/application.ex b/lib/sanity/application.ex
diff --git a/lib/sanity/error.ex b/lib/sanity/error.ex
@@ -3,7 +3,7 @@ defmodule Sanity.Error do
   Error that may occur while making a request to the Sanity API. The `source` field will be one of
   the following:
 
-    * `%Finch.Response{}` - If response with an unsupported HTTP status (like 5xx) is received.
+    * `%Req.Response{}` - If response with an unsupported HTTP status (like 5xx) is received.
     * `%Mint.TransportError{}` - If a network error such as a timeout occurred.
     * `%Sanity.Response{}` - If a 4xx response is received during a call to `Sanity.request!/2`.
   """

diff --git a/lib/sanity/response.ex b/lib/sanity/response.ex
@@ -1,5 +1,5 @@
 defmodule Sanity.Response do
   @type t :: %Sanity.Response{}
 
-  defstruct [:body, :headers, :status]
+  defstruct body: %{}, headers: %{}, status: nil
 end
diff --git a/mix.exs b/mix.exs
@@ -7,7 +7,7 @@ defmodule Sanity.MixProject do
     [
       app: :sanity,
       version: @version,
-      elixir: "~> 1.12",
+      elixir: "~> 1.13",
       elixirc_paths: elixirc_paths(Mix.env()),
       description: "Client library for Sanity CMS.",
       start_permanent: Mix.env() == :prod,
@@ -28,17 +28,16 @@ defmodule Sanity.MixProject do
   # Run "mix help compile.app" to learn about applications.
   def application do
     [
-      extra_applications: [:logger],
-      mod: {Sanity.Application, []}
+      extra_applications: [:logger]
     ]
   end
 
   # Run "mix help deps" to learn about dependencies.
   defp deps do
     [
-      {:finch, "~> 0.5"},
       {:jason, "~> 1.2"},
       {:nimble_options, "~> 0.5 or ~> 1.0"},
+      {:req, "~> 0.4"},
 
       # dev/test
       {:dialyxir, "~> 1.0", only: [:dev], runtime: false},

diff --git a/mix.lock b/mix.lock
@@ -14,7 +14,9 @@
   "mint": {:hex, :mint, "1.6.0", "88a4f91cd690508a04ff1c3e28952f322528934be541844d54e0ceb765f01d5e", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1 or ~> 0.2.0", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "3c5ae85d90a5aca0a49c0d8b67360bbe407f3b54f1030a111047ff988e8fefaa"},
   "mox": {:hex, :mox, "1.1.0", "0f5e399649ce9ab7602f72e718305c0f9cdc351190f72844599545e4996af73c", [:mix], [], "hexpm", "d44474c50be02d5b72131070281a5d3895c0e7a95c780e90bc0cfe712f633a13"},
   "nimble_options": {:hex, :nimble_options, "1.1.0", "3b31a57ede9cb1502071fade751ab0c7b8dbe75a9a4c2b5bbb0943a690b63172", [:mix], [], "hexpm", "8bbbb3941af3ca9acc7835f5655ea062111c9c27bcac53e004460dfd19008a99"},
+  "nimble_ownership": {:hex, :nimble_ownership, "0.3.1", "99d5244672fafdfac89bfad3d3ab8f0d367603ce1dc4855f86a1c75008bce56f", [:mix], [], "hexpm", "4bf510adedff0449a1d6e200e43e57a814794c8b5b6439071274d248d272a549"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
   "nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"},
+  "req": {:hex, :req, "0.4.14", "103de133a076a31044e5458e0f850d5681eef23dfabf3ea34af63212e3b902e2", [:mix], [{:aws_signature, "~> 0.3.2", [hex: :aws_signature, repo: "hexpm", optional: true]}, {:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.17", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 1.6 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:nimble_ownership, "~> 0.2.0 or ~> 0.3.0", [hex: :nimble_ownership, repo: "hexpm", optional: false]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "2ddd3d33f9ab714ced8d3c15fd03db40c14dbf129003c4a3eb80fac2cc0b1b08"},
   "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"},
 }
diff --git a/test/integration_test.exs b/test/integration_test.exs
@@ -37,7 +37,11 @@ defmodule Sanity.MutateIntegrationTest do
       )
       |> Sanity.request(config)
 
-    assert {:ok, %Response{body: %{"documents" => [%{"title" => "product x"}]}}} =
+    assert {:ok,
+            %Response{
+              body: %{"documents" => [%{"title" => "product x"}]},
+              headers: %{"content-type" => ["application/json; charset=utf-8"]}
+            }} =
              Sanity.doc(id) |> Sanity.request(config)
   end