refactor: remove dependency for parsing HTTP-dates

Replaces the dependency on date_time_parser with an HTTP-date
parser implementation based on the one used in
https://github.com/wojtekmach/req

lib/tesla/middleware/retry.ex

@@ -124,8 +124,14 @@ defmodule Tesla.Middleware.Retry do
   # Calculate the min ms to retry after if the header is specified and enabled
   defp retry_after({_, %Tesla.Env{} = env}, %{use_retry_after_header: true}) do
     case Tesla.get_header(env, "retry-after") do
-      nil -> nil
-      header -> retry_after_delay(header)
+      nil ->
+        nil
+
+      header ->
+        case retry_delay_in_ms(header) do
+          {:ok, delay_ms} -> delay_ms
+          {:error, _} -> nil
+        end
     end
   end
 
@@ -134,20 +140,77 @@ defmodule Tesla.Middleware.Retry do
     nil
   end
 
-  # Attempt to interpret the retry-after header as integer number of seconds, then a DateTime
-  defp retry_after_delay(header) do
-    case Integer.parse(header) do
+  # Adapted from https://github.com/wojtekmach/req/blob/2a802826b1f3e65bb13a5a1da037ce2d8734e619/lib/req/response.ex#L265
+  # Copyright 2021 Wojtek Mach
+  # Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at
+  # http://www.apache.org/licenses/LICENSE-2.0
+  # Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+  # START
+
+  # changed to return :ok and :error tuples
+  defp retry_delay_in_ms(delay_value) do
+    case Integer.parse(delay_value) do
+      {seconds, ""} ->
+        {:ok, :timer.seconds(seconds)}
+
       :error ->
-        case DateTimeParser.parse_datetime(header) do
-          {:ok, date_time} -> max(0, DateTime.diff(date_time, DateTime.utc_now(), :millisecond))
-          {:error, _} -> nil
+        case parse_http_datetime(delay_value) do
+          {:ok, date_time} ->
+            {:ok,
+             date_time
+             |> DateTime.diff(DateTime.utc_now(), :millisecond)
+             |> max(0)}
+
+          {:error, _} = error ->
+            error
+        end
+    end
+  end
+
+  @month_numbers %{
+    "Jan" => "01",
+    "Feb" => "02",
+    "Mar" => "03",
+    "Apr" => "04",
+    "May" => "05",
+    "Jun" => "06",
+    "Jul" => "07",
+    "Aug" => "08",
+    "Sep" => "09",
+    "Oct" => "10",
+    "Nov" => "11",
+    "Dec" => "12"
+  }
+
+  # changed to return :ok and :error tuples rather than raise if the date cannot be parwsed
+  defp parse_http_datetime(datetime) do
+    case String.split(datetime, " ") do
+      [_day_of_week, day, month, year, time, "GMT"] ->
+        case @month_numbers[month] do
+          nil ->
+            {:error, "cannot parse \"retry-after\" header value #{inspect(datetime)} as datetime, reason: invalid month"}
+
+          month_number ->
+            date = year <> "-" <> month_number <> "-" <> day
+
+            case DateTime.from_iso8601(date <> " " <> time <> "Z") do
+              {:ok, valid_datetime, 0} ->
+                {:ok, valid_datetime}
+
+              {:error, reason} ->
+                {:error,
+                 "cannot parse \"retry-after\" header value #{inspect(datetime)} as datetime, reason: #{reason}"}
+            end
         end
 
-      {seconds, _} ->
-        seconds * 1000
+      _ ->
+        {:error,
+         "cannot parse \"retry-after\" header value #{inspect(datetime)} as datetime, reason: header is not in HTTP-date or integer format"}
     end
   end
 
+  # END
+
   defp do_retry(env, next, context) do
     case context.retry_after do
       nil ->

mix.exs

@@ -50,7 +50,6 @@ defmodule Tesla.Mixfile do
   defp deps do
     [
       {:mime, "~> 1.0 or ~> 2.0"},
-      {:date_time_parser, "~> 1.2.0"},
 
       # http clients
       {:ibrowse, "4.4.2", optional: true},

mix.lock

@@ -5,7 +5,6 @@
   "con_cache": {:hex, :con_cache, "1.1.0", "45c7c6cd6dc216e47636232e8c683734b7fe293221fccd9454fa1757bc685044", [:mix], [{:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "8655f2ae13a1e56c8aef304d250814c7ed929c12810f126fc423ecc8e871593b"},
   "cowboy": {:hex, :cowboy, "2.12.0", "f276d521a1ff88b2b9b4c54d0e753da6c66dd7be6c9fca3d9418b561828a3731", [:make, :rebar3], [{:cowlib, "2.13.0", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "1.8.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "8a7abe6d183372ceb21caa2709bec928ab2b72e18a3911aa1771639bef82651e"},
   "cowlib": {:hex, :cowlib, "2.13.0", "db8f7505d8332d98ef50a3ef34b34c1afddec7506e4ee4dd4a3a266285d282ca", [:make, :rebar3], [], "hexpm", "e1e1284dc3fc030a64b1ad0d8382ae7e99da46c3246b815318a4b848873800a4"},
-  "date_time_parser": {:hex, :date_time_parser, "1.2.0", "3d5a816b91967f51e0f94dcb16a34b2cb780f22cd48931779e81d72f7d3eadb1", [:mix], [{:kday, "~> 1.0", [hex: :kday, repo: "hexpm", optional: false]}], "hexpm", "0cf09ada9f42c0b3bfba02dc0ea2e4b4d2f543d9d2bf99b831a29e6b4a4160e5"},
   "dialyxir": {:hex, :dialyxir, "1.4.4", "fb3ce8741edeaea59c9ae84d5cec75da00fa89fe401c72d6e047d11a61f65f70", [:mix], [{:erlex, ">= 0.2.7", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "cd6111e8017ccd563e65621a4d9a4a1c5cd333df30cebc7face8029cacb4eff6"},
   "earmark": {:hex, :earmark, "1.4.3", "364ca2e9710f6bff494117dbbd53880d84bebb692dafc3a78eb50aa3183f2bfd", [:mix], [], "hexpm", "8cf8a291ebf1c7b9539e3cddb19e9cef066c2441b1640f13c34c1d3cfc825fec"},
   "earmark_parser": {:hex, :earmark_parser, "1.4.41", "ab34711c9dc6212dda44fcd20ecb87ac3f3fce6f0ca2f28d4a00e4154f8cd599", [:mix], [], "hexpm", "a81a04c7e34b6617c2792e291b5a2e57ab316365c2644ddc553bb9ed863ebefa"},
@@ -25,7 +24,6 @@
   "inch_ex": {:hex, :inch_ex, "2.0.0", "24268a9284a1751f2ceda569cd978e1fa394c977c45c331bb52a405de544f4de", [:mix], [{:bunt, "~> 0.2", [hex: :bunt, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "96d0ec5ecac8cf63142d02f16b7ab7152cf0f0f1a185a80161b758383c9399a8"},
   "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"},
   "jsx": {:hex, :jsx, "2.8.3", "a05252d381885240744d955fbe3cf810504eb2567164824e19303ea59eef62cf", [:mix, :rebar3], [], "hexpm", "fc3499fed7a726995aa659143a248534adc754ebd16ccd437cd93b649a95091f"},
-  "kday": {:hex, :kday, "1.0.2", "e96035c439323eeb8505268959122e9d30194a4e5a23a357dc75f1cae696e918", [:mix], [{:ex_doc, "~> 0.21", [hex: :ex_doc, repo: "hexpm", optional: true]}], "hexpm", "f040b9b6de21eea4a96dbf71753f4eeb0772a6ebd6c18cacd114694af7cabc9a"},
   "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"},
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
   "makeup_erlang": {:hex, :makeup_erlang, "1.0.1", "c7f58c120b2b5aa5fd80d540a89fdf866ed42f1f3994e4fe189abebeab610839", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "8a89a1eeccc2d798d6ea15496a6e4870b75e014d1af514b1b71fa33134f57814"},

test/tesla/middleware/retry_test.exs

@@ -62,10 +62,82 @@ defmodule Tesla.Middleware.RetryTest do
             "/retry_after_date" ->
               {:ok, %{env | status: 200}}
 
-            "/retry_after_invalid" when retries < 5 ->
+            "/retry_after_invalid_format" when retries < 5 ->
               {:ok, %{env | status: 429, headers: [{"retry-after", "foo"} | env.headers]}}
 
-            "/retry_after_invalid" ->
+            "/retry_after_invalid_format" ->
+              {:ok, %{env | status: 200}}
+
+            "/retry_after_invalid_month" when retries < 5 ->
+              {:ok,
+               %{
+                 env
+                 | status: 429,
+                   headers: [
+                     {"retry-after",
+                      Calendar.strftime(
+                        DateTime.add(start_time, 2, :second),
+                        "%a, %d Foo %Y %H:%M:%S GMT"
+                      )}
+                     | env.headers
+                   ]
+               }}
+
+            "/retry_after_invalid_month" ->
+              {:ok, %{env | status: 200}}
+
+            "/retry_after_invalid_day" when retries < 5 ->
+              {:ok,
+               %{
+                 env
+                 | status: 429,
+                   headers: [
+                     {"retry-after",
+                      Calendar.strftime(
+                        DateTime.add(start_time, 2, :second),
+                        "%a, Foo %b %Y %H:%M:%S GMT"
+                      )}
+                     | env.headers
+                   ]
+               }}
+
+            "/retry_after_invalid_day" ->
+              {:ok, %{env | status: 200}}
+
+            "/retry_after_invalid_year" when retries < 5 ->
+              {:ok,
+               %{
+                 env
+                 | status: 429,
+                   headers: [
+                     {"retry-after",
+                      Calendar.strftime(
+                        DateTime.add(start_time, 2, :second),
+                        "%a, %d %b Foo %H:%M:%S GMT"
+                      )}
+                     | env.headers
+                   ]
+               }}
+
+            "/retry_after_invalid_year" ->
+              {:ok, %{env | status: 200}}
+
+            "/retry_after_invalid_time" when retries < 5 ->
+              {:ok,
+               %{
+                 env
+                 | status: 429,
+                   headers: [
+                     {"retry-after",
+                      Calendar.strftime(
+                        DateTime.add(start_time, 2, :second),
+                        "%a, %d %b %Y Foo:Bar:%S GMT"
+                      )}
+                     | env.headers
+                   ]
+               }}
+
+            "/retry_after_invalid_time" ->
               {:ok, %{env | status: 200}}
           end
 
@@ -191,9 +263,37 @@ defmodule Tesla.Middleware.RetryTest do
     assert Agent.get(LaggyAdapter, fn %{retries: retries} -> retries end) == 2
   end
 
-  test "ingore Retry-After header if it is not in an expected format" do
-    assert {:ok, %Tesla.Env{url: "/retry_after_invalid", method: :get, status: 200}} =
-             ClientUsingRetryAfterHeader.get("/retry_after_invalid")
+  test "ignore Retry-After header if it is not in an expected format" do
+    assert {:ok, %Tesla.Env{url: "/retry_after_invalid_format", method: :get, status: 200}} =
+             ClientUsingRetryAfterHeader.get("/retry_after_invalid_format")
+
+    assert Agent.get(LaggyAdapter, fn %{retries: retries} -> retries end) == 6
+  end
+
+  test "ignore Retry-After header if it has an invalid month" do
+    assert {:ok, %Tesla.Env{url: "/retry_after_invalid_month", method: :get, status: 200}} =
+             ClientUsingRetryAfterHeader.get("/retry_after_invalid_month")
+
+    assert Agent.get(LaggyAdapter, fn %{retries: retries} -> retries end) == 6
+  end
+
+  test "ignore Retry-After header if it has an invalid day" do
+    assert {:ok, %Tesla.Env{url: "/retry_after_invalid_day", method: :get, status: 200}} =
+             ClientUsingRetryAfterHeader.get("/retry_after_invalid_day")
+
+    assert Agent.get(LaggyAdapter, fn %{retries: retries} -> retries end) == 6
+  end
+
+  test "ignore Retry-After header if it has an invalid year" do
+    assert {:ok, %Tesla.Env{url: "/retry_after_invalid_year", method: :get, status: 200}} =
+             ClientUsingRetryAfterHeader.get("/retry_after_invalid_year")
+
+    assert Agent.get(LaggyAdapter, fn %{retries: retries} -> retries end) == 6
+  end
+
+  test "ignore Retry-After header if it has an invalid time" do
+    assert {:ok, %Tesla.Env{url: "/retry_after_invalid_time", method: :get, status: 200}} =
+             ClientUsingRetryAfterHeader.get("/retry_after_invalid_time")
 
     assert Agent.get(LaggyAdapter, fn %{retries: retries} -> retries end) == 6
   end