diff --git a/Analytics-CSharp/Segment/Analytics/Configuration.cs b/Analytics-CSharp/Segment/Analytics/Configuration.cs index 79de436..c2aa395 100644 --- a/Analytics-CSharp/Segment/Analytics/Configuration.cs +++ b/Analytics-CSharp/Segment/Analytics/Configuration.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using Segment.Analytics.Policies; +using Segment.Analytics.Retry; using Segment.Analytics.Utilities; using Segment.Concurrent; using Segment.Serialization; @@ -47,6 +48,21 @@ private set public IEventPipelineProvider EventPipelineProvider { get; } + /// + /// HTTP retry configuration for rate limiting and exponential backoff. Defaults to + /// null. Set it before constructing Analytics, e.g. + /// new Configuration("writeKey") { HttpConfig = new HttpConfig(...) }. + /// Mirrors analytics-kotlin's mutable Configuration.httpConfig. + /// + /// This sets the pipeline's starting configuration only. CDN settings take precedence: + /// any settings payload carrying an httpConfig key replaces this value, and a CDN + /// payload is treated as enabling a subsystem unless it says "enabled": "false". + /// A payload with no httpConfig key leaves this value in effect. This matches the + /// behaviour of analytics-kotlin and analytics-swift. + /// + /// + public HttpConfig HttpConfig { get; set; } + /// /// Configuration that analytics can use /// diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs index 5be1331..e371e75 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs @@ -3,7 +3,7 @@ namespace Segment.Analytics.Retry { - internal class RateLimitConfig + public class RateLimitConfig { public bool Enabled { get; } public int MaxRetryCount { get; } @@ -18,12 +18,14 @@ public RateLimitConfig(bool enabled = false, int maxRetryCount = 100, int maxRet public RateLimitConfig Validated() => new RateLimitConfig( enabled: Enabled, - maxRetryCount: Math.Max(0, Math.Min(MaxRetryCount, 1000)), + // Floored at 1: the count is compared against a fresh state's retry + // count, so 0 would drop every batch before it was ever sent. + maxRetryCount: Math.Max(1, Math.Min(MaxRetryCount, 1000)), maxRetryInterval: Math.Max(1, Math.Min(MaxRetryInterval, 3600)) ); } - internal class BackoffConfig + public class BackoffConfig { public bool Enabled { get; } public int MaxRetryCount { get; } @@ -57,12 +59,15 @@ public BackoffConfig( Default4xxBehavior = default4xxBehavior; Default5xxBehavior = default5xxBehavior; UnknownCodeBehavior = unknownCodeBehavior; - StatusCodeOverrides = statusCodeOverrides ?? DefaultStatusCodeOverrides; + // Copied because the property is public: sharing the static default would let + // one caller's mutation corrupt every BackoffConfig built afterwards. + StatusCodeOverrides = new Dictionary( + statusCodeOverrides ?? DefaultStatusCodeOverrides); } public BackoffConfig Validated() => new BackoffConfig( enabled: Enabled, - maxRetryCount: Math.Max(0, Math.Min(MaxRetryCount, 1000)), + maxRetryCount: Math.Max(1, Math.Min(MaxRetryCount, 1000)), baseBackoffInterval: Math.Max(0.1, Math.Min(BaseBackoffInterval, 60.0)), maxBackoffInterval: Math.Max(1, Math.Min(MaxBackoffInterval, 3600)), maxTotalBackoffDuration: Math.Max(0, Math.Min(MaxTotalBackoffDuration, 604800)), @@ -93,7 +98,11 @@ private static Dictionary ValidateOverrides( { 429, RetryBehavior.Retry }, { 460, RetryBehavior.Retry }, { 501, RetryBehavior.Drop }, - { 505, RetryBehavior.Drop } + { 505, RetryBehavior.Drop }, + // 511 is only retryable for an SDK that can re-authenticate via OAuth. + // This one cannot, so retrying would spend the budget on a request that + // can never succeed. + { 511, RetryBehavior.Drop } }; } @@ -109,7 +118,7 @@ public RetryConfig(RateLimitConfig rateLimitConfig = null, BackoffConfig backoff } } - internal class HttpConfig + public class HttpConfig { public RateLimitConfig RateLimitConfig { get; } public BackoffConfig BackoffConfig { get; } diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs index 3538bc8..853d9ed 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs @@ -41,6 +41,16 @@ public RetryState HandleResponse(RetryState state, ResponseInfo response) ); } + // Any retryable status with Retry-After → rate-limit path + if (response.RetryAfterSeconds.HasValue && response.RetryAfterSeconds.Value > 0) + { + RetryBehavior behavior = response.StatusCode == 429 + ? RetryBehavior.Retry // 429 is always retryable + : ResolveStatusCodeBehavior(response.StatusCode); + if (behavior == RetryBehavior.Retry && _config.RateLimitConfig.Enabled) + return HandleRateLimitResponse(state, response, currentTime); + } + if (response.StatusCode == 429) { if (_config.RateLimitConfig.Enabled) @@ -48,8 +58,8 @@ public RetryState HandleResponse(RetryState state, ResponseInfo response) return state.RemoveBatch(response.BatchFile); } - RetryBehavior behavior = ResolveStatusCodeBehavior(response.StatusCode); - if (behavior == RetryBehavior.Retry && _config.BackoffConfig.Enabled) + RetryBehavior statusBehavior = ResolveStatusCodeBehavior(response.StatusCode); + if (statusBehavior == RetryBehavior.Retry && _config.BackoffConfig.Enabled) return HandleRetryableError(state, response, currentTime); return state.RemoveBatch(response.BatchFile); @@ -131,7 +141,14 @@ public int GetRetryCount(RetryState state, string batchFile) return Math.Max(batchRetryCount, state.GlobalRetryCount); } - public bool ShouldDeleteBatch(int statusCode) + public bool ShouldDeleteBatch(int statusCode) => ShouldDeleteBatch(statusCode, null); + + /// + /// Whether the batch file should be removed. must be + /// the same value handed to , so that the two agree on whether + /// this response took the rate-limit path. + /// + public bool ShouldDeleteBatch(int statusCode, int? retryAfterSeconds) { if (IsLegacyMode) return statusCode >= 400 && statusCode <= 499 && statusCode != 429; @@ -143,8 +160,16 @@ public bool ShouldDeleteBatch(int statusCode) return !_config.RateLimitConfig.Enabled; RetryBehavior behavior = ResolveStatusCodeBehavior(statusCode); - if (behavior == RetryBehavior.Retry && !_config.BackoffConfig.Enabled) - return true; + if (behavior == RetryBehavior.Retry) + { + // A usable Retry-After sends this response down the rate-limit path, which has + // just scheduled the retry — keep the batch that retry will re-upload. + if (retryAfterSeconds.HasValue && retryAfterSeconds.Value > 0 && _config.RateLimitConfig.Enabled) + return false; + + // Otherwise only backoff can retry it; with backoff off, nothing will. + return !_config.BackoffConfig.Enabled; + } return behavior == RetryBehavior.Drop; } diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryTypes.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryTypes.cs index 7a87348..5884cb3 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryTypes.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryTypes.cs @@ -6,7 +6,7 @@ internal enum PipelineState RateLimited } - internal enum RetryBehavior + public enum RetryBehavior { Retry, Drop diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs index 3056e8a..1eac511 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs @@ -48,7 +48,7 @@ public EventPipeline( string apiHost = HTTPClient.DefaultAPIHost) : this(analytics, logTag, apiKey, flushPolicies, apiHost, (HttpConfig)null) { } - internal EventPipeline( + public EventPipeline( Analytics analytics, string logTag, string apiKey, @@ -69,7 +69,9 @@ internal EventPipeline( Running = false; var retryConfig = httpConfig != null - ? new RetryConfig(httpConfig.RateLimitConfig, httpConfig.BackoffConfig) + // User-supplied config arrives unclamped; the CDN path is already + // validated by HttpConfigParser. + ? new RetryConfig(httpConfig.RateLimitConfig.Validated(), httpConfig.BackoffConfig.Validated()) : new RetryConfig(); _retryStateMachine = new RetryStateMachine(retryConfig); _retryState = RetryStateStorage.LoadRetryState(_storage); @@ -78,7 +80,7 @@ internal EventPipeline( internal void UpdateHttpConfig(HttpConfig config) { var retryConfig = config != null - ? new RetryConfig(config.RateLimitConfig, config.BackoffConfig) + ? new RetryConfig(config.RateLimitConfig.Validated(), config.BackoffConfig.Validated()) : new RetryConfig(); _retryStateMachine = new RetryStateMachine(retryConfig); } @@ -209,11 +211,7 @@ await Scope.WithContext(_analytics.FileIODispatcher, () => HTTPClient.Response response = await _httpClient.UploadWithResponse(data, retryCount); statusCode = response.StatusCode; - if (!string.IsNullOrEmpty(response.RetryAfterHeader) - && int.TryParse(response.RetryAfterHeader.Trim(), out int parsedRetryAfter)) - { - retryAfterSeconds = parsedRetryAfter; - } + retryAfterSeconds = RetryAfterParser.Parse(response.RetryAfterHeader); if (response.IsSuccessStatusCode) { @@ -223,7 +221,7 @@ await Scope.WithContext(_analytics.FileIODispatcher, () => else { Analytics.Logger.Log(LogLevel.Error, message: "Error " + statusCode + " uploading " + url); - shouldCleanup = _retryStateMachine.ShouldDeleteBatch(statusCode); + shouldCleanup = _retryStateMachine.ShouldDeleteBatch(statusCode, retryAfterSeconds); if (shouldCleanup) { _analytics.ReportInternalError(AnalyticsErrorType.NetworkServerRejected, diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipelineProvider.cs b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipelineProvider.cs index abd376c..137780c 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipelineProvider.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipelineProvider.cs @@ -11,7 +11,8 @@ public IEventPipeline Create(Analytics analytics, string key) return new EventPipeline(analytics, key, analytics.Configuration.WriteKey, analytics.Configuration.FlushPolicies, - analytics.Configuration.ApiHost); + analytics.Configuration.ApiHost, + analytics.Configuration.HttpConfig); } } } \ No newline at end of file diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs b/Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs index 22b0168..fd5cd0e 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs @@ -5,6 +5,7 @@ using System.Net; using System.Net.Http; using System.Net.Http.Headers; +using System.Text; using System.Threading.Tasks; using Segment.Analytics.Retry; using Segment.Serialization; @@ -25,6 +26,15 @@ public abstract class HTTPClient private readonly string _apiKey; + /// + /// Value for the Authorization header: the write key as HTTP Basic credentials with an + /// empty password, matching the other Segment SDKs. TAPI authenticates and routes on this + /// header rather than parsing the payload, so custom + /// implementations should send it on upload requests. + /// + protected string BasicAuthorization => + "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes(_apiKey + ":")); + protected readonly string _apiHost; protected readonly string _cdnHost; @@ -188,6 +198,8 @@ public class Response /// /// A convenient method to check if the http request is successful /// + // Only 2xx. HttpClient follows any redirect it can, so a 3xx here means it + // declined to (no Location, a 300, or a 304) and nothing was uploaded. public bool IsSuccessStatusCode => StatusCode >= 200 && StatusCode < 300; } } @@ -242,6 +254,7 @@ public override async Task DoPost(string url, byte[] data, int retryCo var request = new HttpRequestMessage(HttpMethod.Post, url); request.Headers.Add("Connection", "close"); + request.Headers.Add("Authorization", BasicAuthorization); request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("text/plain")); if (retryCount > 0) request.Headers.Add("X-Retry-Count", retryCount.ToString()); diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/RetryAfterParser.cs b/Analytics-CSharp/Segment/Analytics/Utilities/RetryAfterParser.cs new file mode 100644 index 0000000..7a5f249 --- /dev/null +++ b/Analytics-CSharp/Segment/Analytics/Utilities/RetryAfterParser.cs @@ -0,0 +1,38 @@ +using System; +using System.Globalization; + +namespace Segment.Analytics.Utilities +{ + internal static class RetryAfterParser + { + /// + /// Parses a Retry-After header value. Supports both integer seconds and HTTP-date (RFC 1123) format. + /// Returns the number of seconds to wait, or null if the header is empty/unparseable/in the past. + /// + internal static int? Parse(string headerValue, DateTimeOffset? now = null) + { + if (string.IsNullOrEmpty(headerValue)) + return null; + + string trimmed = headerValue.Trim(); + + if (int.TryParse(trimmed, out int parsedInt)) + { + return parsedInt; + } + + if (DateTimeOffset.TryParseExact(trimmed, + new[] { "r", "ddd, dd MMM yyyy HH:mm:ss 'GMT'" }, + CultureInfo.InvariantCulture, + DateTimeStyles.AssumeUniversal, + out DateTimeOffset targetDate)) + { + DateTimeOffset reference = now ?? DateTimeOffset.UtcNow; + int seconds = (int)(targetDate - reference).TotalSeconds; + return seconds > 0 ? seconds : (int?)null; + } + + return null; + } + } +} diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs index 4657be9..25ad712 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs @@ -61,7 +61,7 @@ public SyncEventPipeline( CancellationToken? flushCancellationToken = null) : this(analytics, logTag, apiKey, flushPolicies, apiHost, flushTimeout, flushCancellationToken, null) { } - internal SyncEventPipeline( + public SyncEventPipeline( Analytics analytics, string logTag, string apiKey, @@ -86,7 +86,9 @@ internal SyncEventPipeline( _flushCancellationToken = flushCancellationToken ?? CancellationToken.None; var retryConfig = httpConfig != null - ? new RetryConfig(httpConfig.RateLimitConfig, httpConfig.BackoffConfig) + // User-supplied config arrives unclamped; the CDN path is already + // validated by HttpConfigParser. + ? new RetryConfig(httpConfig.RateLimitConfig.Validated(), httpConfig.BackoffConfig.Validated()) : new RetryConfig(); _retryStateMachine = new RetryStateMachine(retryConfig); _retryState = RetryStateStorage.LoadRetryState(_storage); @@ -95,7 +97,7 @@ internal SyncEventPipeline( internal void UpdateHttpConfig(HttpConfig config) { var retryConfig = config != null - ? new RetryConfig(config.RateLimitConfig, config.BackoffConfig) + ? new RetryConfig(config.RateLimitConfig.Validated(), config.BackoffConfig.Validated()) : new RetryConfig(); _retryStateMachine = new RetryStateMachine(retryConfig); } @@ -234,11 +236,7 @@ await Scope.WithContext(_analytics.FileIODispatcher, () => HTTPClient.Response response = await _httpClient.UploadWithResponse(data, retryCount); statusCode = response.StatusCode; - if (!string.IsNullOrEmpty(response.RetryAfterHeader) - && int.TryParse(response.RetryAfterHeader.Trim(), out int parsedRetryAfter)) - { - retryAfterSeconds = parsedRetryAfter; - } + retryAfterSeconds = RetryAfterParser.Parse(response.RetryAfterHeader); if (response.IsSuccessStatusCode) { @@ -248,7 +246,7 @@ await Scope.WithContext(_analytics.FileIODispatcher, () => else { Analytics.Logger.Log(LogLevel.Error, message: "Error " + statusCode + " uploading " + url); - shouldCleanup = _retryStateMachine.ShouldDeleteBatch(statusCode); + shouldCleanup = _retryStateMachine.ShouldDeleteBatch(statusCode, retryAfterSeconds); if (shouldCleanup) { _analytics.ReportInternalError(AnalyticsErrorType.NetworkServerRejected, diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipelineProvider.cs b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipelineProvider.cs index 5794677..931a10b 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipelineProvider.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipelineProvider.cs @@ -22,7 +22,8 @@ public IEventPipeline Create(Analytics analytics, string key) analytics.Configuration.FlushPolicies, analytics.Configuration.ApiHost, _flushTimeout, - _flushCancellationToken); + _flushCancellationToken, + analytics.Configuration.HttpConfig); } } } \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..06c52cd --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,25 @@ +# Changelog + +Release notes for published versions are generated on the +[Releases page](https://github.com/segmentio/Analytics-CSharp/releases). +This file carries the notes that need more than a pull-request title. + +## Unreleased + +### Upgrade note: new request headers and proxy allowlists + +This release sends two request headers that 2.6.0 did not: `Authorization` +(HTTP Basic, carrying your write key) and `X-Retry-Count` (on retries only). +If your traffic to Segment goes through a proxy, gateway or WAF that +allowlists request headers, add both before upgrading or uploads will be +rejected. Unity WebGL builds must also add them to the CORS +`Access-Control-Allow-Headers` allowlist on any proxy they point at. + +- Send the write key as an `Authorization: Basic` header. It is still included in the request body, so no server-side change is required. +- Send `X-Retry-Count` on retries, so the server can distinguish a retry from a first attempt. +- `HttpConfig` is now a settable property on `Configuration` rather than a constructor parameter, so retry behaviour can be configured after construction. For mobile targets, CDN settings replace `Configuration.HttpConfig` when they are present. +- `Retry-After` is honoured on every retryable status rather than 429 alone, which brings 529 in through the generic 5xx rule. Numeric seconds and the RFC 7231 HTTP-date formats are both accepted, capped at `MaxRetryInterval`. +- 511 is dropped rather than retried: it asks the client to authenticate, which this library cannot do. +- Only 2xx responses count as a successful upload. A 3xx is now retried rather than silently treated as delivered; the Segment endpoint does not redirect, so this only affects custom host values. +- `RateLimitConfig.MaxRetryCount` and `BackoffConfig.MaxRetryCount` are floored at 1. A configured 0 previously dropped every batch before it was ever sent. +- `BackoffConfig.StatusCodeOverrides` is copied rather than held by reference, so mutating the caller's dictionary no longer changes a live config. diff --git a/Samples/UnitySample/UnityHTTPClient.cs b/Samples/UnitySample/UnityHTTPClient.cs index 60cb8e0..0b87d2d 100644 --- a/Samples/UnitySample/UnityHTTPClient.cs +++ b/Samples/UnitySample/UnityHTTPClient.cs @@ -61,6 +61,7 @@ IEnumerator PostRequest(NetworkRequest networkRequest) using (var request = UnityWebRequest.Put(networkRequest.URL, networkRequest.Data)) { request.SetRequestHeader("Content-Type", "text/plain"); + request.SetRequestHeader("Authorization", BasicAuthorization); yield return request.SendWebRequest(); networkRequest.Response.StatusCode = (int)request.responseCode; diff --git a/Tests/Retry/ConfigurationHttpConfigTest.cs b/Tests/Retry/ConfigurationHttpConfigTest.cs new file mode 100644 index 0000000..55add83 --- /dev/null +++ b/Tests/Retry/ConfigurationHttpConfigTest.cs @@ -0,0 +1,146 @@ +using System; +using Moq; +using Segment.Analytics; +using Segment.Analytics.Retry; +using Segment.Analytics.Utilities; +using Segment.Serialization; +using Tests.Utils; +using Xunit; + +namespace Tests.Retry +{ + /// + /// Configuration.HttpConfig is the user-facing entry point for retry settings, + /// mirroring Kotlin's Configuration.httpConfig and Swift's .httpConfig(_:). + /// These cover that a config supplied there actually reaches the pipeline's + /// retry state machine; CDN settings still override it later via UpdateHttpConfig. + /// + public class ConfigurationHttpConfigTest + { + private static Analytics CreateAnalytics(HttpConfig httpConfig) + { + Settings? settings = JsonUtility.FromJson( + "{\"integrations\":{\"Segment.io\":{\"apiKey\":\"k\"}},\"plan\":{},\"edgeFunction\":{}}"); + + var mockHttpClient = new Mock(null, null, null); + mockHttpClient.Setup(c => c.Settings()).ReturnsAsync(settings); + + var config = new Configuration( + writeKey: "123", + autoAddSegmentDestination: false, + useSynchronizeDispatcher: true, + flushInterval: 0, + flushAt: 2, + httpClientProvider: new MockHttpClientProvider(mockHttpClient), + storageProvider: new MockStorageProvider(new Mock()) + ) + { + HttpConfig = httpConfig + }; + return new Analytics(config); + } + + [Fact] + public void Configuration_ExposesHttpConfig() + { + var httpConfig = new HttpConfig(backoffConfig: new BackoffConfig(enabled: true, maxRetryCount: 7)); + Analytics analytics = CreateAnalytics(httpConfig); + + Assert.Same(httpConfig, analytics.Configuration.HttpConfig); + } + + [Fact] + public void Configuration_HttpConfigDefaultsToNull() + { + Analytics analytics = CreateAnalytics(null); + + Assert.Null(analytics.Configuration.HttpConfig); + } + + [Fact] + public void EventPipeline_WithoutHttpConfig_IsLegacyMode() + { + Analytics analytics = CreateAnalytics(null); + + var pipeline = (EventPipeline)new EventPipelineProvider().Create(analytics, "key"); + + Assert.True(pipeline._retryStateMachine.IsLegacyMode); + } + + [Fact] + public void EventPipeline_WithHttpConfig_LeavesLegacyMode() + { + Analytics analytics = CreateAnalytics( + new HttpConfig(backoffConfig: new BackoffConfig(enabled: true))); + + var pipeline = (EventPipeline)new EventPipelineProvider().Create(analytics, "key"); + + Assert.False(pipeline._retryStateMachine.IsLegacyMode); + } + + [Fact] + public void SyncEventPipeline_WithoutHttpConfig_IsLegacyMode() + { + Analytics analytics = CreateAnalytics(null); + + var pipeline = (SyncEventPipeline)new SyncEventPipelineProvider().Create(analytics, "key"); + + Assert.True(pipeline._retryStateMachine.IsLegacyMode); + } + + [Fact] + public void MaxRetryCountOfZero_DoesNotDropBeforeTheFirstAttempt() + { + // ShouldUploadBatch compares a fresh state's counts against MaxRetryCount, + // so an unfloored 0 dropped every batch without ever sending it. + var machine = new RetryStateMachine(new RetryConfig( + new RateLimitConfig(enabled: true, maxRetryCount: 0).Validated(), + new BackoffConfig(enabled: true, maxRetryCount: 0).Validated())); + + Tuple decision = + machine.ShouldUploadBatch(new RetryState(), "b.json"); + + Assert.IsType(decision.Item1); + } + + [Fact] + public void BackoffConfig_DoesNotShareTheDefaultOverrideMap() + { + var first = new BackoffConfig(enabled: true); + first.StatusCodeOverrides[500] = RetryBehavior.Drop; + + var second = new BackoffConfig(enabled: true); + + Assert.False(second.StatusCodeOverrides.ContainsKey(500)); + Assert.NotSame(first.StatusCodeOverrides, second.StatusCodeOverrides); + } + + [Fact] + public void UserSuppliedHttpConfig_IsValidatedOnTheWayIn() + { + // maxRetryInterval: 0 is out of range and must clamp to 1 second, exactly as the + // CDN path does via HttpConfigParser. Unvalidated it would schedule the retry at + // currentTime, i.e. no wait at all. + Analytics analytics = CreateAnalytics( + new HttpConfig(rateLimitConfig: new RateLimitConfig(enabled: true, maxRetryInterval: 0))); + + var pipeline = (EventPipeline)new EventPipelineProvider().Create(analytics, "key"); + RetryState state = pipeline._retryStateMachine.HandleResponse( + new RetryState(), + new ResponseInfo(429, retryAfterSeconds: null, batchFile: "b.json", currentTime: 1000)); + + Assert.Equal(2000, state.WaitUntilTime); + } + + [Fact] + public void SyncEventPipeline_WithHttpConfig_LeavesLegacyMode() + { + Analytics analytics = CreateAnalytics( + new HttpConfig(rateLimitConfig: new RateLimitConfig(enabled: true))); + + var pipeline = (SyncEventPipeline)new SyncEventPipelineProvider().Create(analytics, "key"); + + Assert.False(pipeline._retryStateMachine.IsLegacyMode); + } + } +} diff --git a/Tests/Retry/RetryAfterDeleteBatchTest.cs b/Tests/Retry/RetryAfterDeleteBatchTest.cs new file mode 100644 index 0000000..8a3d12a --- /dev/null +++ b/Tests/Retry/RetryAfterDeleteBatchTest.cs @@ -0,0 +1,98 @@ +using Segment.Analytics.Retry; +using Xunit; + +namespace Tests.Retry +{ + /// + /// ShouldDeleteBatch must agree with HandleResponse about whether a response took the + /// rate-limit path. A retryable status carrying Retry-After schedules a retry, so its + /// batch must be kept; without Retry-After only backoff can retry it. + /// + public class RetryAfterDeleteBatchTest + { + private static RetryStateMachine RateLimitOnlyMachine() => + new RetryStateMachine(new RetryConfig( + new RateLimitConfig(enabled: true), + new BackoffConfig(enabled: false))); + + [Theory] + [InlineData(503)] + [InlineData(529)] + [InlineData(408)] + [InlineData(410)] + public void RetryableStatus_WithRetryAfter_IsKept(int status) + { + Assert.False(RateLimitOnlyMachine().ShouldDeleteBatch(status, 30)); + } + + [Theory] + [InlineData(503)] + [InlineData(529)] + public void RetryableStatus_WithoutRetryAfter_AndBackoffDisabled_IsDeleted(int status) + { + // Nothing would retry it, so holding the file would leak storage. + Assert.True(RateLimitOnlyMachine().ShouldDeleteBatch(status, null)); + } + + [Fact] + public void RetryAfterZero_DoesNotCountAsRateLimited() + { + Assert.True(RateLimitOnlyMachine().ShouldDeleteBatch(503, 0)); + } + + [Fact] + public void RetryAfter_RateLimitsPipelineAndKeepsBatch() + { + var machine = RateLimitOnlyMachine(); + var response = new ResponseInfo(503, retryAfterSeconds: 30, batchFile: "b.json", currentTime: 1000); + + RetryState state = machine.HandleResponse(new RetryState(), response); + + Assert.Equal(PipelineState.RateLimited, state.PipelineState); + Assert.Equal(31000, state.WaitUntilTime); + Assert.False(machine.ShouldDeleteBatch(503, 30)); + } + + [Theory] + [InlineData(200)] + [InlineData(201)] + [InlineData(204)] + public void SuccessStatuses_AreDeleted(int status) + { + // 2xx is success, so the batch is done with. + Assert.True(RateLimitOnlyMachine().ShouldDeleteBatch(status, null)); + } + + [Theory] + [InlineData(300)] + [InlineData(301)] + [InlineData(304)] + public void Redirects_AreNotSuccess(int status) + { + // HttpClient follows what it can; a 3xx arriving here means nothing was + // uploaded, so the batch must not be treated as delivered. + var machine = RateLimitOnlyMachine(); + RetryState state = machine.HandleResponse( + new RetryState(), + new ResponseInfo(status, retryAfterSeconds: null, batchFile: "b.json", currentTime: 1000)); + + Assert.Equal(PipelineState.Ready, state.PipelineState); + Assert.True(machine.ShouldDeleteBatch(status, null)); + } + + [Fact] + public void NonRetryableStatus_IsDeletedEvenWithRetryAfter() + { + Assert.True(RateLimitOnlyMachine().ShouldDeleteBatch(400, 30)); + Assert.True(RateLimitOnlyMachine().ShouldDeleteBatch(501, 30)); + } + + [Fact] + public void NetworkAuthenticationRequired_IsDropped() + { + // 511 is retryable only for an SDK that can re-authenticate; this one cannot. + Assert.True(RateLimitOnlyMachine().ShouldDeleteBatch(511, null)); + Assert.True(RateLimitOnlyMachine().ShouldDeleteBatch(511, 30)); + } + } +} diff --git a/Tests/Retry/RetryAfterParserTest.cs b/Tests/Retry/RetryAfterParserTest.cs new file mode 100644 index 0000000..4880a83 --- /dev/null +++ b/Tests/Retry/RetryAfterParserTest.cs @@ -0,0 +1,81 @@ +using System; +using Segment.Analytics.Utilities; +using Xunit; + +namespace Tests.Retry +{ + public class RetryAfterParserTest + { + [Fact] + public void Parse_IntegerSeconds_ReturnsParsedValue() + { + Assert.Equal(60, RetryAfterParser.Parse("60")); + } + + [Fact] + public void Parse_IntegerWithWhitespace_ReturnsParsedValue() + { + Assert.Equal(120, RetryAfterParser.Parse(" 120 ")); + } + + [Fact] + public void Parse_Null_ReturnsNull() + { + Assert.Null(RetryAfterParser.Parse(null)); + } + + [Fact] + public void Parse_Empty_ReturnsNull() + { + Assert.Null(RetryAfterParser.Parse("")); + } + + [Fact] + public void Parse_HttpDate_InFuture_ReturnsSeconds() + { + var now = new DateTimeOffset(2026, 6, 16, 12, 0, 0, TimeSpan.Zero); + // 2 seconds in the future + string httpDate = "Tue, 16 Jun 2026 12:00:02 GMT"; + + int? result = RetryAfterParser.Parse(httpDate, now); + + Assert.Equal(2, result); + } + + [Fact] + public void Parse_HttpDate_InPast_ReturnsNull() + { + var now = new DateTimeOffset(2026, 6, 16, 12, 0, 0, TimeSpan.Zero); + // 10 seconds in the past + string httpDate = "Tue, 16 Jun 2026 11:59:50 GMT"; + + int? result = RetryAfterParser.Parse(httpDate, now); + + Assert.Null(result); + } + + [Fact] + public void Parse_HttpDate_Rfc1123Format_ParsesCorrectly() + { + var now = new DateTimeOffset(2026, 6, 16, 10, 0, 0, TimeSpan.Zero); + // 300 seconds (5 minutes) in the future + string httpDate = "Tue, 16 Jun 2026 10:05:00 GMT"; + + int? result = RetryAfterParser.Parse(httpDate, now); + + Assert.Equal(300, result); + } + + [Fact] + public void Parse_InvalidString_ReturnsNull() + { + Assert.Null(RetryAfterParser.Parse("not-a-date-or-number")); + } + + [Fact] + public void Parse_Zero_ReturnsZero() + { + Assert.Equal(0, RetryAfterParser.Parse("0")); + } + } +} diff --git a/Tests/Retry/RetryStateMachineTest.cs b/Tests/Retry/RetryStateMachineTest.cs index 69b0250..1e27178 100644 --- a/Tests/Retry/RetryStateMachineTest.cs +++ b/Tests/Retry/RetryStateMachineTest.cs @@ -371,6 +371,72 @@ public void ShouldDeleteBatch_SmartMode_408_False() Assert.False(machine.ShouldDeleteBatch(408)); } + // --- RetryAfterSeconds on retryable errors --- + + [Fact] + public void HandleResponse_503_WithRetryAfter_RoutesToRateLimitPath() + { + var machine = CreateMachine(maxRetryInterval: 300); + var state = new RetryState(); + var response = new ResponseInfo(503, retryAfterSeconds: 2, batchFile: "batch1.json", currentTime: 1000); + + RetryState newState = machine.HandleResponse(state, response); + + Assert.Equal(PipelineState.RateLimited, newState.PipelineState); + Assert.Equal(1, newState.GlobalRetryCount); + Assert.Equal(1000L + 2000L, newState.WaitUntilTime); + } + + [Fact] + public void HandleResponse_529_WithRetryAfter_RoutesToRateLimitPath() + { + var machine = CreateMachine(maxRetryInterval: 300); + var state = new RetryState(); + var response = new ResponseInfo(529, retryAfterSeconds: 3, batchFile: "batch1.json", currentTime: 1000); + + RetryState newState = machine.HandleResponse(state, response); + + Assert.Equal(PipelineState.RateLimited, newState.PipelineState); + Assert.Equal(1, newState.GlobalRetryCount); + Assert.Equal(1000L + 3000L, newState.WaitUntilTime); + } + + [Fact] + public void HandleResponse_503_WithoutRetryAfter_UsesExponentialBackoff() + { + var machine = CreateMachine(); + var state = new RetryState(); + var response = new ResponseInfo(503, retryAfterSeconds: null, batchFile: "batch1.json", currentTime: 1000); + + RetryState newState = machine.HandleResponse(state, response); + + // Still goes through backoff path (failureCount incremented, not rate-limited) + Assert.True(newState.BatchMetadata.ContainsKey("batch1.json")); + Assert.Equal(1, newState.BatchMetadata["batch1.json"].FailureCount); + Assert.True(newState.BatchMetadata["batch1.json"].NextRetryTime > 1000L); + Assert.Equal(PipelineState.Ready, newState.PipelineState); + Assert.Equal(0, newState.GlobalRetryCount); + } + + [Fact] + public void HandleResponse_503_WithRetryAfter_ClampsToMaxRetryInterval() + { + var config = new RetryConfig( + new RateLimitConfig(enabled: true, maxRetryCount: 100, maxRetryInterval: 10), + new BackoffConfig(enabled: true, maxRetryCount: 100, maxBackoffInterval: 300) + ); + var machine = new RetryStateMachine(config, new FakeTimeProvider(), new Random(42)); + var state = new RetryState(); + var response = new ResponseInfo(503, retryAfterSeconds: 999, batchFile: "batch1.json", currentTime: 1000); + + RetryState newState = machine.HandleResponse(state, response); + + // Now routes through rate-limit path, clamped to maxRetryInterval=10 + Assert.Equal(PipelineState.RateLimited, newState.PipelineState); + Assert.Equal(1000L + 10 * 1000L, newState.WaitUntilTime); + Assert.Equal(1, newState.GlobalRetryCount); + } + // --- GetRetryCount tests --- [Fact] diff --git a/Tests/Utilities/HttpClientAuthTest.cs b/Tests/Utilities/HttpClientAuthTest.cs new file mode 100644 index 0000000..d4fd9b8 --- /dev/null +++ b/Tests/Utilities/HttpClientAuthTest.cs @@ -0,0 +1,42 @@ +using System; +using System.Text; +using Segment.Analytics.Utilities; +using Xunit; + +namespace Tests.Utilities +{ + /// + /// TAPI authenticates and routes on the Authorization header rather than parsing the + /// payload, so the value has to match what the other Segment SDKs send: the write key + /// as Basic credentials with an empty password. + /// + public class HttpClientAuthTest + { + private class AuthProbe : DefaultHTTPClient + { + public AuthProbe(string apiKey) : base(apiKey) { } + + public string Authorization => BasicAuthorization; + } + + [Theory] + [InlineData("writekey123")] + [InlineData("aBc-123_XYZ")] + public void UsesWriteKeyAsBasicCredentialsWithEmptyPassword(string writeKey) + { + var expected = "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes(writeKey + ":")); + + Assert.Equal(expected, new AuthProbe(writeKey).Authorization); + } + + [Fact] + public void EncodesTheTrailingColonSeparator() + { + // Decoding must yield ":" — an empty password, not a missing one. + var header = new AuthProbe("k").Authorization; + var decoded = Encoding.UTF8.GetString(Convert.FromBase64String(header.Substring("Basic ".Length))); + + Assert.Equal("k:", decoded); + } + } +} diff --git a/e2e-cli/e2e-config.json b/e2e-cli/e2e-config.json index 91e0a5f..7237fc7 100644 --- a/e2e-cli/e2e-config.json +++ b/e2e-cli/e2e-config.json @@ -4,6 +4,7 @@ "auto_settings": true, "patch": null, "env": { - "HTTP_CONFIG_SETTINGS": "true" + "HTTP_CONFIG_SETTINGS": "true", + "AUTH_HEADER": "true" } }