Merge pull request #1094 from studzien/configurable-retriable-status-…

…codes Configurable max attempts for client errors
ex-aws · Oct 31, 2024 · 50af1b8 · 50af1b8
2 parents 1841c4d + 9edaa0a
commit 50af1b8
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 19 deletions.
diff --git a/lib/ex_aws/request.ex b/lib/ex_aws/request.ex
@@ -53,7 +53,7 @@ defmodule ExAws.Request do
                 config,
                 headers,
                 req_body,
-                attempt_again?(attempt, reason, config)
+                attempt_again?(attempt, reason, :client, config)
               )
 
             {:error, reason} ->
@@ -71,7 +71,7 @@ defmodule ExAws.Request do
             config,
             headers,
             req_body,
-            attempt_again?(attempt, reason, config)
+            attempt_again?(attempt, reason, :server, config)
           )
 
         {:error, reason_struct} ->
@@ -92,7 +92,7 @@ defmodule ExAws.Request do
             config,
             headers,
             req_body,
-            attempt_again?(attempt, reason, config)
+            attempt_again?(attempt, reason, :other, config)
           )
       end
     end
@@ -200,8 +200,14 @@ defmodule ExAws.Request do
     end
   end
 
-  def attempt_again?(attempt, reason, config) do
-    if attempt >= config[:retries][:max_attempts] do
+  def attempt_again?(attempt, reason, error_type, config) do
+    max_attempts =
+      case error_type do
+        :client -> config[:retries][:client_error_max_attempts] || config[:retries][:max_attempts]
+        _ -> config[:retries][:max_attempts]
+      end
+
+    if attempt >= max_attempts do
       {:error, reason}
     else
       attempt |> backoff(config)

diff --git a/test/ex_aws/request_test.exs b/test/ex_aws/request_test.exs
@@ -137,21 +137,8 @@ defmodule ExAws.RequestTest do
   end
 
   test "ProvisionedThroughputExceededException is retried", context do
-    exception =
-      "{\"__type\": \"ProvisionedThroughputExceededException\", \"message\": \"Rate exceeded for shard shardId-000000000005 in stream my_stream under account 1234567890.\"}"
-
-    success =
-      "{\"SequenceNumber\":\"49592207023850419758877078054930583111417627497740632066\",\"ShardId\":\"shardId-000000000000\"}"
-
     TelemetryHelper.attach_telemetry([:ex_aws, :request])
-
-    ExAws.Request.HttpMock
-    |> expect(:request, 2, fn _method, _url, _body, _headers, _opts ->
-      {:ok, %{status_code: 400, body: exception}}
-    end)
-    |> expect(:request, fn _method, _url, _body, _headers, _opts ->
-      {:ok, %{status_code: 200, body: success}}
-    end)
+    success = mock_provisioned_throughput_response(2)
 
     http_method = :post
     url = "https://kinesis.aws.com/"
@@ -177,6 +164,34 @@ defmodule ExAws.RequestTest do
     assert_receive {[:ex_aws, :request, :stop], %{duration: _}, %{attempt: 3, result: :ok}}
   end
 
+  test "ProvisionedThroughputExceededException is not retried if client error retries is set to 1",
+       context do
+    TelemetryHelper.attach_telemetry([:ex_aws, :request])
+    mock_provisioned_throughput_response(1)
+
+    http_method = :post
+    url = "https://kinesis.aws.com/"
+    service = :kinesis
+    request_body = ""
+
+    config = context[:config] |> put_in([:retries, :client_error_max_attempts], 1)
+
+    assert {:error, {"ProvisionedThroughputExceededException", _}} =
+             ExAws.Request.request_and_retry(
+               http_method,
+               url,
+               service,
+               config,
+               context[:headers],
+               request_body,
+               {:attempt, 1}
+             )
+
+    assert_receive {[:ex_aws, :request, :start], %{system_time: _}, %{attempt: 1}}
+    assert_receive {[:ex_aws, :request, :stop], %{duration: _}, %{attempt: 1, result: :error}}
+    refute_receive {[:ex_aws, :request, :start], %{system_time: _}, %{attempt: 2}}
+  end
+
   test "Expected sequence token is provided", context do
     exception =
       "{\"__type\": \"InvalidSequenceTokenException\", \"message\": \"The given sequenceToken is invalid. The next expected sequenceToken is: 49616449618992442982853194240983586320797062450229805234\", \"expectedSequenceToken\": \"49616449618992442982853194240983586320797062450229805234\"}"
@@ -250,4 +265,23 @@ defmodule ExAws.RequestTest do
                {:attempt, 5}
              )
   end
+
+  defp mock_provisioned_throughput_response(success_after_retries) do
+    exception =
+      "{\"__type\": \"ProvisionedThroughputExceededException\", \"message\": \"Rate exceeded for shard shardId-000000000005 in stream my_stream under account 1234567890.\"}"
+
+    success =
+      "{\"SequenceNumber\":\"49592207023850419758877078054930583111417627497740632066\",\"ShardId\":\"shardId-000000000000\"}"
+
+    ExAws.Request.HttpMock
+    |> expect(:request, success_after_retries, fn _method, _url, _body, _headers, _opts ->
+      {:ok, %{status_code: 400, body: exception}}
+    end)
+    |> expect(:request, fn _method, _url, _body, _headers, _opts ->
+      {:ok, %{status_code: 200, body: success}}
+    end)
+
+    success
+  end
+
 end