diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs index 5be1331..768d56f 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs @@ -93,7 +93,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 } }; } diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs index 3538bc8..deacdf5 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs @@ -22,7 +22,7 @@ public RetryState HandleResponse(RetryState state, ResponseInfo response) { if (IsLegacyMode) { - if (response.StatusCode >= 200 && response.StatusCode <= 299) + if (response.StatusCode >= 200 && response.StatusCode < 400) return state.RemoveBatch(response.BatchFile); if (response.StatusCode == 429 || (response.StatusCode >= 500 && response.StatusCode <= 599)) return state; // Keep @@ -31,7 +31,7 @@ public RetryState HandleResponse(RetryState state, ResponseInfo response) long currentTime = response.CurrentTime; - if (response.StatusCode >= 200 && response.StatusCode <= 299) + if (response.StatusCode >= 200 && response.StatusCode < 400) { return state.With( pipelineState: PipelineState.Ready, @@ -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,20 +141,36 @@ 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; - if (statusCode >= 200 && statusCode <= 299) + // Spec item 1: 2xx and 3xx are success. + if (statusCode >= 200 && statusCode < 400) return true; if (statusCode == 429) 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/Utilities/EventPipeline.cs b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs index 3056e8a..6cfeeb8 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs @@ -209,11 +209,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 +219,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/HTTPClient.cs b/Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs index 22b0168..b6946c0 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,7 +198,8 @@ public class Response /// /// A convenient method to check if the http request is successful /// - public bool IsSuccessStatusCode => StatusCode >= 200 && StatusCode < 300; + // Spec item 1: 2xx and 3xx are success. + public bool IsSuccessStatusCode => StatusCode >= 200 && StatusCode < 400; } } @@ -242,6 +253,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..e64a53b 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs @@ -234,11 +234,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 +244,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/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/RetryAfterDeleteBatchTest.cs b/Tests/Retry/RetryAfterDeleteBatchTest.cs new file mode 100644 index 0000000..fa51e16 --- /dev/null +++ b/Tests/Retry/RetryAfterDeleteBatchTest.cs @@ -0,0 +1,82 @@ +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(301)] + [InlineData(304)] + public void SuccessStatuses_AreDeleted(int status) + { + // Spec item 1: 2xx and 3xx are success, so the batch is done with. + Assert.True(RateLimitOnlyMachine().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" } }