Retry HttpClient requests on 502/503/504 too

Previously, HttpClient retries requests (when 'tries' was large enough)
in two conditions: on 429 (obeying the retry-after header) and on 500
(retrying after 1 second).

The 502 (Bad Gateway), 503 (Service Unavailable), and 504 (Gateway
Timeout) statuses are all _infrastructure_ prompted conditions, but from
the caller's perspective they are all equivalent to a 500: "the service
failed to handle my request". Allowing retries to apply to these
statuses is natural and appropriate.

Add a small section to the docs describing the behavior of the 'tries'
parameter in more detail, including the updated behavior.

Author: nevinera

docs/usage/rest.md

@@ -214,6 +214,20 @@ Each method can take the parameters outlined in the table below.
 
 **Note:** _These parameters can still be used in all methods regardless of if they are required._
 
+#### Automatic Retries
+
+When `tries` is greater than `1`, the client will automatically retry requests that receive one of the following HTTP status codes:
+
+- `429 Too Many Requests`
+- `500 Internal Server Error`
+- `502 Bad Gateway`
+- `503 Service Unavailable`
+- `504 Gateway Timeout`
+
+For `429` responses, the sleep duration between retries is taken from the `Retry-After` response header if present. For all other retryable status codes, or if that header is not present, the client waits 1 second between attempts.
+
+If all retries are exhausted, a `ShopifyAPI::Errors::MaxHttpRetriesExceededError` is raised.
+
 #### Output
 ##### Success
 If the request is successful these methods will all return a [`ShopifyAPI::Clients::HttpResponse`](https://github.com/Shopify/shopify-api-ruby/blob/main/lib/shopify_api/clients/http_response.rb) object, which has the following methods:

lib/shopify_api/clients/http_client.rb

@@ -80,7 +80,7 @@ def request(request, response_as_struct: false)
 
           error_message = serialized_error(response)
 
-          unless [429, 500].include?(response.code)
+          unless [429, 500, 502, 503, 504].include?(response.code)
             raise ShopifyAPI::Errors::HttpResponseError.new(response: response), error_message
           end
 
@@ -91,7 +91,7 @@ def request(request, response_as_struct: false)
               "Exceeded maximum retry count of #{request.tries}. Last message: #{error_message}"
           end
 
-          if response.code == 500 || response.headers["retry-after"].nil?
+          if [500, 502, 503, 504].include?(response.code) || response.headers["retry-after"].nil?
             sleep(RETRY_WAIT_TIME)
           else
             sleep(T.must(response.headers["retry-after"])[0].to_i)

test/clients/http_client_test.rb

@@ -240,6 +240,48 @@ def test_retry_internal_error
         verify_http_request
       end
 
+      def test_retry_bad_gateway
+        @request.tries = 2
+
+        @client.expects(:sleep).with(1).times(1)
+
+        stub_request(@request.http_method, "https://#{@shop}#{@base_path}/#{@request.path}")
+          .with(body: @request.body.to_json, query: @request.query, headers: @expected_headers)
+          .to_return(body: { errors: "Bad gateway" }.to_json, headers: @response_headers, status: 502)
+          .times(1)
+          .then.to_return(body: @success_body.to_json, headers: @response_headers)
+
+        verify_http_request
+      end
+
+      def test_retry_service_unavailable
+        @request.tries = 2
+
+        @client.expects(:sleep).with(1).times(1)
+
+        stub_request(@request.http_method, "https://#{@shop}#{@base_path}/#{@request.path}")
+          .with(body: @request.body.to_json, query: @request.query, headers: @expected_headers)
+          .to_return(body: { errors: "Service unavailable" }.to_json, headers: @response_headers, status: 503)
+          .times(1)
+          .then.to_return(body: @success_body.to_json, headers: @response_headers)
+
+        verify_http_request
+      end
+
+      def test_retry_gateway_timeout
+        @request.tries = 2
+
+        @client.expects(:sleep).with(1).times(1)
+
+        stub_request(@request.http_method, "https://#{@shop}#{@base_path}/#{@request.path}")
+          .with(body: @request.body.to_json, query: @request.query, headers: @expected_headers)
+          .to_return(body: { errors: "Gateway timeout" }.to_json, headers: @response_headers, status: 504)
+          .times(1)
+          .then.to_return(body: @success_body.to_json, headers: @response_headers)
+
+        verify_http_request
+      end
+
       def test_retries_exceeded
         @request.tries = 3