SNOW-1997074: Add retry for original call on HTTP 307/308

CHANGELOG.md

@@ -7,6 +7,7 @@
     - Added `CRLDOWNLOADMAXSIZE` connection parameter to limit the maximum size of CRL files downloaded during certificate revocation checks.
     - AWS WIF will now also check the application config and AWS profile credential store when determining the current AWS region
     - Allow users to configure the maximum amount of connections via `SERVICE_POINT_CONNECTION_LIMIT` property.
+    - Add retry for HTTP 307/308 status codes
 - v5.0.0
     - Disabled CRL checks by default.
     - Added support for alternative, memory efficient and thread safe CRL (Certificate Revocation List) checks.

Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs

@@ -64,6 +64,8 @@
         [TestCase((HttpStatusCode)429, false, true)] // HttpStatusCode.TooManyRequests is not available on .NET Framework
         [TestCase(HttpStatusCode.InternalServerError, false, true)]
         [TestCase(HttpStatusCode.ServiceUnavailable, false, true)]
+        [TestCase(HttpStatusCode.TemporaryRedirect, false, true)]
+        [TestCase((HttpStatusCode)308, false, true)]  // HttpStatusCode.PermanentRedirect is not available on .NET Framework
         public async Task TestIsRetryableHTTPCode(HttpStatusCode statusCode, bool forceRetryOn404, bool expectedIsRetryable)
         {
             var mockHttp = new MockHttpMessageHandler();
@@ -192,8 +194,8 @@
         {
             // arrange
             var expectedLimit = 51;
             var originalLimit = ServicePointManager.DefaultConnectionLimit;
             ServicePointManager.DefaultConnectionLimit = expectedLimit;
 
             try
             {

Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs

@@ -0,0 +1,112 @@
+using System.Net.Http;
+using System.Threading.Tasks;
+using Newtonsoft.Json;
+using NUnit.Framework;
+using Snowflake.Data.Core;
+using Snowflake.Data.Tests.Util;
+using System;
+using System.Threading;
+
+namespace Snowflake.Data.Tests.UnitTests
+{
+    [TestFixture, NonParallelizable]
+    public class HttpUtilWiremockRunnerTest
+    {
+        private WiremockRunner _runner;
+
+        [OneTimeSetUp]
+        public void BeforeAll()
+        {
+            _runner = WiremockRunner.NewWiremock();
+        }
+
+        [SetUp]
+        public void BeforeEach()
+        {
+            _runner.ResetMapping();
+        }
+
+        [OneTimeTearDown]
+        public void AfterAll()
+        {
+            _runner.Stop();
+        }
+
+        [Test]
+        public async Task TestHttp307Retry()
+        {
+            // arrange
+            _runner.AddMappings("wiremock/HttpUtil/http_307_retry.json");
+            var expectedQueryId = "http_307_retry_queryId";
+            var httpClient = CreateHttpClientForRetry();
+            var request = CreateQueryRequest();
+
+            //act
+            var response = await httpClient.SendAsync(request);
+
+            // assert
+            await AssertResponseId(response, expectedQueryId);
+        }
+
+        [Test]
+        public async Task TestHttp308Retry()
+        {
+            // arrange
+            _runner.AddMappings("wiremock/HttpUtil/http_308_retry.json");
+            var expectedQueryId = "http_308_retry_queryId";
+            var httpClient = CreateHttpClientForRetry();
+            var request = CreateQueryRequest();
+
+            //act
+            var response = await httpClient.SendAsync(request);
+
+            // assert
+            await AssertResponseId(response, expectedQueryId);
+        }
+
+        private HttpClient CreateHttpClientForRetry()
+        {
+            var config = new HttpClientConfig(null, null, null, null, null, false, false, 7, 20);
+            var handler = new CustomDelegatingHandler(new HttpClientHandler
+            {
+                ClientCertificateOptions = ClientCertificateOption.Manual,
+                ServerCertificateCustomValidationCallback = (_, _, _, _) => true,
+                AllowAutoRedirect = true,
+            });
+            return HttpUtil.Instance.CreateNewHttpClient(config, handler);
+        }
+
+        private HttpRequestMessage CreateQueryRequest()
+        {
+            var request = new HttpRequestMessage(HttpMethod.Post, _runner.WiremockBaseHttpsUrl + "/queries/v1/query-request?requestId=abc");
+#pragma warning disable CS0618 // Type or member is obsolete
+            request.Properties.Add(BaseRestRequest.HTTP_REQUEST_TIMEOUT_KEY, TimeSpan.FromSeconds(BaseRestRequest.s_defaultHttpSecondsTimeout));
+            request.Properties.Add(BaseRestRequest.REST_REQUEST_TIMEOUT_KEY, TimeSpan.FromSeconds(BaseRestRequest.s_defaultRestRetrySecondsTimeout));
+#pragma warning restore CS0618 // Type or member is obsolete
+            return request;
+        }
+
+        private async Task AssertResponseId(HttpResponseMessage response, string expectedQueryId)
+        {
+            var json = await response.Content.ReadAsStringAsync();
+            var result = JsonConvert.DeserializeObject<QueryExecResponse>(json, JsonUtils.JsonSettings);
+
+            // assert
+            Assert.True(response.IsSuccessStatusCode);
+            Assert.AreEqual(expectedQueryId, result.data.queryId);
+        }
+    }
+
+    public class CustomDelegatingHandler : DelegatingHandler
+    {
+        public CustomDelegatingHandler(HttpMessageHandler httpMessageHandler) : base(httpMessageHandler) { }
+
+        protected override async Task<HttpResponseMessage> SendAsync(
+            HttpRequestMessage request,
+            CancellationToken cancellationToken)
+        {
+            var response = await base.SendAsync(request, cancellationToken);
+            return response;
+        }
+    }
+}

Snowflake.Data.Tests/wiremock/HttpUtil/http_307_retry.json

@@ -0,0 +1,52 @@
+{
+    "mappings": [
+        {
+            "scenarioName": "HTTP 307 retry",
+            "requiredScenarioState": "Started",
+            "newScenarioState": "Query attempt with HTTP 307 response",
+            "request": {
+                "urlPathPattern": "/queries/v1/query-request.*",
+                "method": "POST"
+            },
+            "response": {
+                "status": 307,
+                "headers": {
+                    "Location": "/temp-redirect-1"
+                }
+            }
+        },
+        {
+            "scenarioName": "HTTP 307 retry",
+            "requiredScenarioState": "Query attempt with HTTP 307 response",
+            "newScenarioState": "307 redirect followed and times out",
+            "request": {
+                "urlPathPattern": "/temp-redirect-1",
+                "method": "POST"
+            },
+            "response": {
+                "fixedDelayMilliseconds": 20000
+            }
+        },
+        {
+            "scenarioName": "HTTP 307 retry",
+            "requiredScenarioState": "307 redirect followed and times out",
+            "newScenarioState": "Retry attempt successful",
+            "request": {
+                "urlPathPattern": "/queries/v1/query-request.*",
+                "method": "POST"
+            },
+            "response": {
+                "status": 200,
+                "jsonBody": {
+                    "data": {
+                        "queryId": "http_307_retry_queryId"
+                    },
+                    "code": null,
+                    "message": null,
+                    "success": true
+                }
+            }
+        }
+    ]
+}
+

Snowflake.Data.Tests/wiremock/HttpUtil/http_308_retry.json

@@ -0,0 +1,51 @@
+{
+    "mappings": [
+        {
+            "scenarioName": "HTTP 308 retry",
+            "requiredScenarioState": "Started",
+            "newScenarioState": "Query attempt with HTTP 308 response",
+            "request": {
+                "urlPathPattern": "/queries/v1/query-request.*",
+                "method": "POST"
+            },
+            "response": {
+                "status": 308,
+                "headers": {
+                    "Location": "/temp-redirect-1"
+                }
+            }
+        },
+        {
+            "scenarioName": "HTTP 308 retry",
+            "requiredScenarioState": "Query attempt with HTTP 308 response",
+            "newScenarioState": "308 redirect followed and times out",
+            "request": {
+                "urlPathPattern": "/temp-redirect-1",
+                "method": "POST"
+            },
+            "response": {
+                "fixedDelayMilliseconds": 20000
+            }
+        },
+        {
+            "scenarioName": "HTTP 308 retry",
+            "requiredScenarioState": "308 redirect followed and times out",
+            "newScenarioState": "Retry attempt successful",
+            "request": {
+                "urlPathPattern": "/queries/v1/query-request.*",
+                "method": "POST"
+            },
+            "response": {
+                "status": 200,
+                "jsonBody": {
+                    "data": {
+                        "queryId": "http_308_retry_queryId"
+                    },
+                    "code": null,
+                    "message": null,
+                    "success": true
+                }
+            }
+        }
+    ]
+}

Snowflake.Data/Core/HttpUtil.cs

@@ -292,7 +292,8 @@ private HttpClientHandler CreateHttpClientHandler(HttpClientConfig config)
                 {
                     AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate,
                     UseCookies = false, // Disable cookies
-                    UseProxy = false
+                    UseProxy = false,
+                    AllowAutoRedirect = true
                 };
             }
         }
@@ -306,7 +307,8 @@ private HttpClientHandler CreateHttpClientHandlerWithDotnetCrlCheck(HttpClientCo
                 SslProtocols = config.GetRequestedTlsProtocolsRange(),
                 AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate,
                 UseCookies = false, // Disable cookies
-                UseProxy = false
+                UseProxy = false,
+                AllowAutoRedirect = true
             };
         }
 
@@ -327,7 +329,8 @@ private HttpClientHandler CreateHttpClientHandlerWithCustomizedCrlCheck(HttpClie
                 SslProtocols = config.GetRequestedTlsProtocolsRange(),
                 AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate,
                 UseCookies = false, // Disable cookies
-                UseProxy = false
+                UseProxy = false,
+                AllowAutoRedirect = true
             };
         }
 
@@ -427,8 +430,11 @@ internal UriUpdater(Uri uri, bool includeRetryReason = true)
                 }
             }
 
-            internal Uri Update(int retryReason = 0)
+            internal Uri Update(int retryReason = 0, Uri requestUri = null, Uri location = null)
             {
+                if (IsRedirectHTTPCode(retryReason))
+                    return GetRedirectedUri(requestUri, location);
+
                 // Optimization to bypass parsing if there is no rules at all.
                 if (rules.Count == 0)
                 {
@@ -450,6 +456,13 @@ internal Uri Update(int retryReason = 0)
 
                 return uriBuilder.Uri;
             }
+
+            private Uri GetRedirectedUri(Uri requestUri, Uri location)
+            {
+                if (requestUri.AbsolutePath != location.ToString())
+                    return new Uri(uriBuilder.Uri, location);
+                return uriBuilder.Uri;
+            }
         }
         private class RetryHandler : DelegatingHandler
         {
@@ -620,10 +633,11 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
                         throw new OperationCanceledException(errorMessage);
                     }
 
+                    requestMessage.RequestUri = updater.Update(errorReason, requestMessage.RequestUri, response?.Headers?.Location);
+
                     // Disposing of the response if not null now that we don't need it anymore
                     response?.Dispose();
 
-                    requestMessage.RequestUri = updater.Update(errorReason);
 
                     logger.Debug($"Sleep {backOffInSec} seconds and then retry the request, retryCount: {retryCount}");
 
@@ -674,7 +688,17 @@ static public bool isRetryableHTTPCode(int statusCode, bool forceRetryOn404)
             // Request timeout
             (statusCode == 408) ||
             // Too many requests
-            (statusCode == 429);
+            (statusCode == 429) ||
+            IsRedirectHTTPCode(statusCode);
+        }
+
+        static public bool IsRedirectHTTPCode(int statusCode)
+        {
+            return
+            // Temporary redirect
+            (statusCode == 307) ||
+            // Permanent redirect
+            (statusCode == 308);
         }
 
         /// <summary>

Snowflake.Data/Core/RestRequest.cs

@@ -24,7 +24,10 @@ internal abstract class BaseRestRequest : IRestRequest
         internal static string REST_REQUEST_TIMEOUT_KEY = "TIMEOUT_PER_REST_REQUEST";
 
         // The default Rest timeout. Set to 120 seconds.
-        public static int DEFAULT_REST_RETRY_SECONDS_TIMEOUT = 120;
+        public static readonly int s_defaultRestRetrySecondsTimeout = 120;
+
+        // Default each http request timeout to 16 seconds
+        public static readonly int s_defaultHttpSecondsTimeout = 16;
 
         internal Uri Url { get; set; }
 
@@ -110,10 +113,8 @@ internal class SFRestRequest : BaseRestRequest, IRestRequest
 
         internal SFRestRequest() : base()
         {
-            RestTimeout = TimeSpan.FromSeconds(DEFAULT_REST_RETRY_SECONDS_TIMEOUT);
-
-            // default each http request timeout to 16 seconds
-            HttpTimeout = TimeSpan.FromSeconds(16);
+            RestTimeout = TimeSpan.FromSeconds(s_defaultRestRetrySecondsTimeout);
+            HttpTimeout = TimeSpan.FromSeconds(s_defaultHttpSecondsTimeout);
         }
 
         internal Object jsonBody { get; set; }