From 7651e2f64a63c95a0066b9ca868566dc4f719340 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 4 Nov 2025 10:33:07 -0800 Subject: [PATCH 1/7] Add retry for original call on HTTP 307/308 --- .../UnitTests/HttpUtilWiremockRunnerTest.cs | 112 ++++++++++++++++++ .../wiremock/HttpUtil/http_307_retry.json | 52 ++++++++ .../wiremock/HttpUtil/http_308_retry.json | 51 ++++++++ Snowflake.Data/Core/HttpUtil.cs | 9 +- 4 files changed, 221 insertions(+), 3 deletions(-) create mode 100644 Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs create mode 100644 Snowflake.Data.Tests/wiremock/HttpUtil/http_307_retry.json create mode 100644 Snowflake.Data.Tests/wiremock/HttpUtil/http_308_retry.json diff --git a/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs b/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs new file mode 100644 index 000000000..85c33b404 --- /dev/null +++ b/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); + 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(3)); + request.Properties.Add(BaseRestRequest.REST_REQUEST_TIMEOUT_KEY, TimeSpan.FromSeconds(30)); +#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(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 SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) + { + var response = await base.SendAsync(request, cancellationToken); + return response; + } + } +} diff --git a/Snowflake.Data.Tests/wiremock/HttpUtil/http_307_retry.json b/Snowflake.Data.Tests/wiremock/HttpUtil/http_307_retry.json new file mode 100644 index 000000000..70de6536b --- /dev/null +++ b/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": 5000 + } + }, + { + "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 + } + } + } + ] +} + diff --git a/Snowflake.Data.Tests/wiremock/HttpUtil/http_308_retry.json b/Snowflake.Data.Tests/wiremock/HttpUtil/http_308_retry.json new file mode 100644 index 000000000..af0ae362d --- /dev/null +++ b/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": 5000 + } + }, + { + "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 + } + } + } + ] +} diff --git a/Snowflake.Data/Core/HttpUtil.cs b/Snowflake.Data/Core/HttpUtil.cs index 8cfe17baa..067d2b836 100755 --- a/Snowflake.Data/Core/HttpUtil.cs +++ b/Snowflake.Data/Core/HttpUtil.cs @@ -267,7 +267,8 @@ private HttpClientHandler CreateHttpClientHandler(HttpClientConfig config) { AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate, UseCookies = false, // Disable cookies - UseProxy = false + UseProxy = false, + AllowAutoRedirect = true }; } } @@ -281,7 +282,8 @@ private HttpClientHandler CreateHttpClientHandlerWithDotnetCrlCheck(HttpClientCo SslProtocols = config.GetRequestedTlsProtocolsRange(), AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate, UseCookies = false, // Disable cookies - UseProxy = false + UseProxy = false, + AllowAutoRedirect = true }; } @@ -302,7 +304,8 @@ private HttpClientHandler CreateHttpClientHandlerWithCustomizedCrlCheck(HttpClie SslProtocols = config.GetRequestedTlsProtocolsRange(), AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate, UseCookies = false, // Disable cookies - UseProxy = false + UseProxy = false, + AllowAutoRedirect = true }; } From e77133e07f584e96ab30dc8c2a6c5e02be6fd9b2 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 4 Nov 2025 10:59:30 -0800 Subject: [PATCH 2/7] Modify HttpClientConfig initialization --- Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs b/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs index 85c33b404..736e225c4 100644 --- a/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs @@ -66,7 +66,7 @@ public async Task TestHttp308Retry() private HttpClient CreateHttpClientForRetry() { - var config = new HttpClientConfig(null, null, null, null, null, false, false, 7); + var config = new HttpClientConfig(null, null, null, null, null, false, false, 7, 20); var handler = new CustomDelegatingHandler(new HttpClientHandler { ClientCertificateOptions = ClientCertificateOption.Manual, From 1236f740da47125b74176574183033c04f3f1cc5 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Tue, 4 Nov 2025 12:35:01 -0800 Subject: [PATCH 3/7] Add redirect logic for older .NET versions --- .../UnitTests/HttpUtilTest.cs | 2 ++ Snowflake.Data/Core/HttpUtil.cs | 28 +++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs b/Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs index 92ec7d8df..090f40a1c 100644 --- a/Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs +++ b/Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs @@ -64,6 +64,8 @@ public async Task TestNonRetryableHttpExceptionThrowsError() [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(); diff --git a/Snowflake.Data/Core/HttpUtil.cs b/Snowflake.Data/Core/HttpUtil.cs index ebc26a3b3..3a0a4557e 100644 --- a/Snowflake.Data/Core/HttpUtil.cs +++ b/Snowflake.Data/Core/HttpUtil.cs @@ -430,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) { @@ -453,6 +456,15 @@ internal Uri Update(int retryReason = 0) return uriBuilder.Uri; } + + private Uri GetRedirectedUri(Uri requestUri, Uri location) + { + if (location == null) + return uriBuilder.Uri; + if (requestUri?.AbsolutePath != location.ToString()) + return new Uri(uriBuilder.Uri, location); + return uriBuilder.Uri; + } } private class RetryHandler : DelegatingHandler { @@ -626,7 +638,7 @@ protected override async Task SendAsync(HttpRequestMessage // Disposing of the response if not null now that we don't need it anymore response?.Dispose(); - requestMessage.RequestUri = updater.Update(errorReason); + requestMessage.RequestUri = updater.Update(errorReason, requestMessage.RequestUri, response?.Headers?.Location); logger.Debug($"Sleep {backOffInSec} seconds and then retry the request, retryCount: {retryCount}"); @@ -677,7 +689,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); } /// From e32c71d3ba0a24a819a1795b593ada80df79c113 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 6 Nov 2025 11:01:32 -0800 Subject: [PATCH 4/7] Refactor uri update --- Snowflake.Data/Core/HttpUtil.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Snowflake.Data/Core/HttpUtil.cs b/Snowflake.Data/Core/HttpUtil.cs index 3a0a4557e..ee2e035d4 100644 --- a/Snowflake.Data/Core/HttpUtil.cs +++ b/Snowflake.Data/Core/HttpUtil.cs @@ -459,9 +459,7 @@ internal Uri Update(int retryReason = 0, Uri requestUri = null, Uri location = n private Uri GetRedirectedUri(Uri requestUri, Uri location) { - if (location == null) - return uriBuilder.Uri; - if (requestUri?.AbsolutePath != location.ToString()) + if (requestUri.AbsolutePath != location.ToString()) return new Uri(uriBuilder.Uri, location); return uriBuilder.Uri; } @@ -635,11 +633,11 @@ protected override async Task 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, requestMessage.RequestUri, response?.Headers?.Location); - logger.Debug($"Sleep {backOffInSec} seconds and then retry the request, retryCount: {retryCount}"); await Task.Delay(TimeSpan.FromSeconds(backOffInSec), cancellationToken).ConfigureAwait(false); From 0334b3d17e67c7630548cc5e976af4cc3e570e5a Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 6 Nov 2025 11:39:55 -0800 Subject: [PATCH 5/7] Revert whitespace --- Snowflake.Data/Core/HttpUtil.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Snowflake.Data/Core/HttpUtil.cs b/Snowflake.Data/Core/HttpUtil.cs index a5c0d1742..d0fc23495 100644 --- a/Snowflake.Data/Core/HttpUtil.cs +++ b/Snowflake.Data/Core/HttpUtil.cs @@ -456,6 +456,7 @@ internal Uri Update(int retryReason = 0, Uri requestUri = null, Uri location = n return uriBuilder.Uri; } + private Uri GetRedirectedUri(Uri requestUri, Uri location) { if (requestUri.AbsolutePath != location.ToString()) @@ -747,3 +748,5 @@ static internal bool IsOktaSSORequest(string host, string endpoint) } } } + + From 12c066d71b202632e392acca9e68e8e0ea9f0b20 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Thu, 6 Nov 2025 11:41:49 -0800 Subject: [PATCH 6/7] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75a1d1d24..b7da06c6b 100644 --- a/CHANGELOG.md +++ b/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. From 353819b6c9eb5d9408d8cd8c016b4f85419ce653 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf Date: Fri, 7 Nov 2025 11:38:12 -0800 Subject: [PATCH 7/7] Refactor timeout values --- .../UnitTests/HttpUtilWiremockRunnerTest.cs | 4 ++-- .../wiremock/HttpUtil/http_307_retry.json | 2 +- .../wiremock/HttpUtil/http_308_retry.json | 2 +- Snowflake.Data/Core/RestRequest.cs | 11 ++++++----- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs b/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs index 736e225c4..ac1d13a98 100644 --- a/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/HttpUtilWiremockRunnerTest.cs @@ -80,8 +80,8 @@ 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(3)); - request.Properties.Add(BaseRestRequest.REST_REQUEST_TIMEOUT_KEY, TimeSpan.FromSeconds(30)); + 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; } diff --git a/Snowflake.Data.Tests/wiremock/HttpUtil/http_307_retry.json b/Snowflake.Data.Tests/wiremock/HttpUtil/http_307_retry.json index 70de6536b..9e790b29e 100644 --- a/Snowflake.Data.Tests/wiremock/HttpUtil/http_307_retry.json +++ b/Snowflake.Data.Tests/wiremock/HttpUtil/http_307_retry.json @@ -24,7 +24,7 @@ "method": "POST" }, "response": { - "fixedDelayMilliseconds": 5000 + "fixedDelayMilliseconds": 20000 } }, { diff --git a/Snowflake.Data.Tests/wiremock/HttpUtil/http_308_retry.json b/Snowflake.Data.Tests/wiremock/HttpUtil/http_308_retry.json index af0ae362d..27c857dc2 100644 --- a/Snowflake.Data.Tests/wiremock/HttpUtil/http_308_retry.json +++ b/Snowflake.Data.Tests/wiremock/HttpUtil/http_308_retry.json @@ -24,7 +24,7 @@ "method": "POST" }, "response": { - "fixedDelayMilliseconds": 5000 + "fixedDelayMilliseconds": 20000 } }, { diff --git a/Snowflake.Data/Core/RestRequest.cs b/Snowflake.Data/Core/RestRequest.cs index d5325c96d..43d075f02 100644 --- a/Snowflake.Data/Core/RestRequest.cs +++ b/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; }