From e30087ec45fc3281a4aa15306048bc971ec5b778 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Thu, 3 Sep 2026 11:03:13 -0400 Subject: [PATCH 1/9] Expose HttpConfig so retry behaviour is user-configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retry state machine added in #144 could only ever be configured from CDN settings: RateLimitConfig, BackoffConfig and HttpConfig were all internal and Configuration had no entry point, so a C# consumer could not set retry behaviour at all. Kotlin and Swift both expose this. Kotlin has `Configuration.httpConfig: HttpConfig?` with a public `data class HttpConfig`; Swift has `public func httpConfig(_ config: HttpConfig?) -> Configuration`. This brings C# in line with the SDKs #144 was written to match. - Make RetryBehavior, RateLimitConfig, BackoffConfig and HttpConfig public. RetryConfig stays internal — it is plumbing built from HttpConfig, never supplied by callers. - Add Configuration.HttpConfig, as a trailing optional constructor argument so existing positional callers are unaffected. Defaults to null, preserving today's CDN-only behaviour. - Have EventPipelineProvider and SyncEventPipelineProvider pass it through as the pipeline's starting retry config. CDN settings still override it later via UpdateHttpConfig. - Make the pipeline constructors that take an HttpConfig public, so a custom IEventPipelineProvider can pass one on rather than only read it. 216 tests pass, including 6 new ones covering that a config set on Configuration reaches both pipelines' retry state machines. --- .../Segment/Analytics/Configuration.cs | 12 ++- .../Segment/Analytics/Retry/RetryConfig.cs | 6 +- .../Segment/Analytics/Retry/RetryTypes.cs | 2 +- .../Analytics/Utilities/EventPipeline.cs | 2 +- .../Utilities/EventPipelineProvider.cs | 3 +- .../Analytics/Utilities/SyncEventPipeline.cs | 2 +- .../Utilities/SyncEventPipelineProvider.cs | 3 +- Tests/Retry/ConfigurationHttpConfigTest.cs | 99 +++++++++++++++++++ 8 files changed, 120 insertions(+), 9 deletions(-) create mode 100644 Tests/Retry/ConfigurationHttpConfigTest.cs diff --git a/Analytics-CSharp/Segment/Analytics/Configuration.cs b/Analytics-CSharp/Segment/Analytics/Configuration.cs index 79de436..b4ed621 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,12 @@ private set public IEventPipelineProvider EventPipelineProvider { get; } + /// + /// HTTP retry configuration for rate limiting and exponential backoff. + /// Defaults to null, meaning retry settings come from CDN settings alone. + /// + public HttpConfig HttpConfig { get; } + /// /// Configuration that analytics can use /// @@ -73,6 +80,7 @@ private set /// defaults to DefaultHTTPClientProvider /// /// set custom flush policies to tell analytics when and how to flush. If a value is given, it overwrites flushAt and flushInterval + /// retry configuration for rate limiting and exponential backoff. CDN settings, when present, take precedence public Configuration(string writeKey, int flushAt = 20, int flushInterval = 30, @@ -85,7 +93,8 @@ public Configuration(string writeKey, IStorageProvider storageProvider = default, IHTTPClientProvider httpClientProvider = default, IList flushPolicies = default, - IEventPipelineProvider eventPipelineProvider = default) + IEventPipelineProvider eventPipelineProvider = default, + HttpConfig httpConfig = null) { WriteKey = writeKey; FlushAt = flushAt; @@ -102,6 +111,7 @@ public Configuration(string writeKey, FlushPolicies.Add(new CountFlushPolicy(flushAt)); FlushPolicies.Add(new FrequencyFlushPolicy(flushInterval * 1000L)); EventPipelineProvider = eventPipelineProvider ?? new EventPipelineProvider(); + HttpConfig = httpConfig; } public Configuration(string writeKey, diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs index 5be1331..4e6463a 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; } @@ -23,7 +23,7 @@ public RateLimitConfig(bool enabled = false, int maxRetryCount = 100, int maxRet ); } - internal class BackoffConfig + public class BackoffConfig { public bool Enabled { get; } public int MaxRetryCount { get; } @@ -109,7 +109,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/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..f35cecf 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, 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/SyncEventPipeline.cs b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs index 4657be9..c17443e 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, 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/Tests/Retry/ConfigurationHttpConfigTest.cs b/Tests/Retry/ConfigurationHttpConfigTest.cs new file mode 100644 index 0000000..26ff94e --- /dev/null +++ b/Tests/Retry/ConfigurationHttpConfigTest.cs @@ -0,0 +1,99 @@ +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 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); + } + } +} From d5bf3882ad381e8d8145b144c72df6c46c10fe60 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Thu, 3 Sep 2026 13:07:11 -0400 Subject: [PATCH 2/9] Harden the newly public config surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems that only matter once these types are public: - BackoffConfig stored a reference to the shared static DefaultStatusCodeOverrides whenever no map was supplied. With StatusCodeOverrides exposed as a public property, a caller doing the natural thing — cfg.StatusCodeOverrides[500] = Drop — corrupted the defaults for every BackoffConfig constructed afterwards in the process, including ones parsed from CDN settings, with no way to reset. The constructor now copies the map. - A user-supplied HttpConfig reached the retry state machine unclamped, while the CDN path is validated by HttpConfigParser. Configuration.HttpConfig was therefore the only unvalidated route in, so out-of-range values such as maxRetryInterval: 0 or a negative jitterPercent took effect verbatim. Both pipelines now call Validated() on user-supplied config, matching the CDN path. 218 tests pass, including two new cases covering the copy and the clamping. --- .../Segment/Analytics/Retry/RetryConfig.cs | 5 +++- .../Analytics/Utilities/EventPipeline.cs | 6 ++-- .../Analytics/Utilities/SyncEventPipeline.cs | 6 ++-- Tests/Retry/ConfigurationHttpConfigTest.cs | 29 +++++++++++++++++++ 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs index 4e6463a..3f83581 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs @@ -57,7 +57,10 @@ public BackoffConfig( Default4xxBehavior = default4xxBehavior; Default5xxBehavior = default5xxBehavior; UnknownCodeBehavior = unknownCodeBehavior; - StatusCodeOverrides = statusCodeOverrides ?? DefaultStatusCodeOverrides; + // Copy: the property is public, and sharing the static default would let one + // caller's mutation corrupt every BackoffConfig built afterwards in the process. + StatusCodeOverrides = new Dictionary( + statusCodeOverrides ?? DefaultStatusCodeOverrides); } public BackoffConfig Validated() => new BackoffConfig( diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs index f35cecf..98c14bd 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs @@ -69,7 +69,9 @@ public EventPipeline( Running = false; var retryConfig = httpConfig != null - ? new RetryConfig(httpConfig.RateLimitConfig, httpConfig.BackoffConfig) + // Validated(): user-supplied config reaches us unclamped, unlike the + // CDN path which HttpConfigParser already validates. + ? new RetryConfig(httpConfig.RateLimitConfig.Validated(), httpConfig.BackoffConfig.Validated()) : new RetryConfig(); _retryStateMachine = new RetryStateMachine(retryConfig); _retryState = RetryStateStorage.LoadRetryState(_storage); @@ -78,7 +80,7 @@ public 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); } diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs index c17443e..9c283f3 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs @@ -86,7 +86,9 @@ public SyncEventPipeline( _flushCancellationToken = flushCancellationToken ?? CancellationToken.None; var retryConfig = httpConfig != null - ? new RetryConfig(httpConfig.RateLimitConfig, httpConfig.BackoffConfig) + // Validated(): user-supplied config reaches us unclamped, unlike the + // CDN path which HttpConfigParser already validates. + ? new RetryConfig(httpConfig.RateLimitConfig.Validated(), httpConfig.BackoffConfig.Validated()) : new RetryConfig(); _retryStateMachine = new RetryStateMachine(retryConfig); _retryState = RetryStateStorage.LoadRetryState(_storage); @@ -95,7 +97,7 @@ public 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); } diff --git a/Tests/Retry/ConfigurationHttpConfigTest.cs b/Tests/Retry/ConfigurationHttpConfigTest.cs index 26ff94e..7effcbe 100644 --- a/Tests/Retry/ConfigurationHttpConfigTest.cs +++ b/Tests/Retry/ConfigurationHttpConfigTest.cs @@ -85,6 +85,35 @@ public void SyncEventPipeline_WithoutHttpConfig_IsLegacyMode() Assert.True(pipeline._retryStateMachine.IsLegacyMode); } + [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() { From 192741b5701359f060efe62e4cd47a3ae259f11a Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Thu, 3 Sep 2026 13:14:22 -0400 Subject: [PATCH 3/9] Document that CDN settings replace Configuration.HttpConfig The property doc said retry settings come from CDN settings alone when this is null, which reads as 'non-null means yours is used'. It is not: SegmentDestination calls UpdateHttpConfig on every settings refresh carrying an httpConfig key, which replaces the whole config. A CDN payload also counts as enabling a subsystem unless it explicitly says enabled: false, so a payload tuning something unrelated can turn retries back on. Only a payload with no httpConfig key leaves this value in effect. This matches analytics-kotlin (SegmentDestination.kt:133) and analytics-swift (SegmentDestination.swift:83-91), which assign CDN config over the user's the same way and share the enabled-defaults-true rule, so the behaviour is left alone and only the documentation is corrected. --- Analytics-CSharp/Segment/Analytics/Configuration.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Analytics-CSharp/Segment/Analytics/Configuration.cs b/Analytics-CSharp/Segment/Analytics/Configuration.cs index b4ed621..c1f0495 100644 --- a/Analytics-CSharp/Segment/Analytics/Configuration.cs +++ b/Analytics-CSharp/Segment/Analytics/Configuration.cs @@ -49,8 +49,15 @@ private set public IEventPipelineProvider EventPipelineProvider { get; } /// - /// HTTP retry configuration for rate limiting and exponential backoff. - /// Defaults to null, meaning retry settings come from CDN settings alone. + /// HTTP retry configuration for rate limiting and exponential backoff. Defaults to + /// null. + /// + /// 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; } @@ -80,7 +87,7 @@ private set /// defaults to DefaultHTTPClientProvider /// /// set custom flush policies to tell analytics when and how to flush. If a value is given, it overwrites flushAt and flushInterval - /// retry configuration for rate limiting and exponential backoff. CDN settings, when present, take precedence + /// starting retry configuration for rate limiting and exponential backoff. CDN settings, when present, replace it — see public Configuration(string writeKey, int flushAt = 20, int flushInterval = 30, From 8e5dee2a4f8ed73f4a01d5c5febe52ddb7274c0e Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Thu, 3 Sep 2026 14:39:23 -0400 Subject: [PATCH 4/9] Expose HttpConfig as a settable property, not a ctor parameter Adding a trailing optional parameter to Configuration's constructor is source compatible but not binary compatible: the compiler bakes optional defaults into the call site, so the assembly loses the old 13-parameter .ctor and anything compiled against it fails with MissingMethodException. That is fine for NuGet consumers, who recompile, but this SDK also ships Unity and Xamarin samples where DLLs are dropped in. #144 never touched Configuration.cs, so the break would have been new here. Making HttpConfig a settable property is purely additive, leaves the existing constructor signature untouched, and is closer to analytics-kotlin, which uses a mutable 'var httpConfig' rather than a constructor argument. new Configuration("writeKey") { HttpConfig = new HttpConfig(...) } 218 tests pass. --- Analytics-CSharp/Segment/Analytics/Configuration.cs | 11 +++++------ Tests/Retry/ConfigurationHttpConfigTest.cs | 8 +++++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Analytics-CSharp/Segment/Analytics/Configuration.cs b/Analytics-CSharp/Segment/Analytics/Configuration.cs index c1f0495..c2aa395 100644 --- a/Analytics-CSharp/Segment/Analytics/Configuration.cs +++ b/Analytics-CSharp/Segment/Analytics/Configuration.cs @@ -50,7 +50,9 @@ private set /// /// HTTP retry configuration for rate limiting and exponential backoff. Defaults to - /// null. + /// 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 @@ -59,7 +61,7 @@ private set /// behaviour of analytics-kotlin and analytics-swift. /// /// - public HttpConfig HttpConfig { get; } + public HttpConfig HttpConfig { get; set; } /// /// Configuration that analytics can use @@ -87,7 +89,6 @@ private set /// defaults to DefaultHTTPClientProvider /// /// set custom flush policies to tell analytics when and how to flush. If a value is given, it overwrites flushAt and flushInterval - /// starting retry configuration for rate limiting and exponential backoff. CDN settings, when present, replace it — see public Configuration(string writeKey, int flushAt = 20, int flushInterval = 30, @@ -100,8 +101,7 @@ public Configuration(string writeKey, IStorageProvider storageProvider = default, IHTTPClientProvider httpClientProvider = default, IList flushPolicies = default, - IEventPipelineProvider eventPipelineProvider = default, - HttpConfig httpConfig = null) + IEventPipelineProvider eventPipelineProvider = default) { WriteKey = writeKey; FlushAt = flushAt; @@ -118,7 +118,6 @@ public Configuration(string writeKey, FlushPolicies.Add(new CountFlushPolicy(flushAt)); FlushPolicies.Add(new FrequencyFlushPolicy(flushInterval * 1000L)); EventPipelineProvider = eventPipelineProvider ?? new EventPipelineProvider(); - HttpConfig = httpConfig; } public Configuration(string writeKey, diff --git a/Tests/Retry/ConfigurationHttpConfigTest.cs b/Tests/Retry/ConfigurationHttpConfigTest.cs index 7effcbe..6e15dc5 100644 --- a/Tests/Retry/ConfigurationHttpConfigTest.cs +++ b/Tests/Retry/ConfigurationHttpConfigTest.cs @@ -31,9 +31,11 @@ private static Analytics CreateAnalytics(HttpConfig httpConfig) flushInterval: 0, flushAt: 2, httpClientProvider: new MockHttpClientProvider(mockHttpClient), - storageProvider: new MockStorageProvider(new Mock()), - httpConfig: httpConfig - ); + storageProvider: new MockStorageProvider(new Mock()) + ) + { + HttpConfig = httpConfig + }; return new Analytics(config); } From a294a85efc62f92563a4f68cb094c93450133db8 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Fri, 11 Sep 2026 11:11:23 -0400 Subject: [PATCH 5/9] Tighten retry comments Cut the before/after narration from the comments added with the HttpConfig work. The copy of StatusCodeOverrides and the Validated() calls now state why they are needed rather than what the code did without them. --- Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs | 4 ++-- Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs | 4 ++-- .../Segment/Analytics/Utilities/SyncEventPipeline.cs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs index 3f83581..35dbc42 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs @@ -57,8 +57,8 @@ public BackoffConfig( Default4xxBehavior = default4xxBehavior; Default5xxBehavior = default5xxBehavior; UnknownCodeBehavior = unknownCodeBehavior; - // Copy: the property is public, and sharing the static default would let one - // caller's mutation corrupt every BackoffConfig built afterwards in the process. + // 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); } diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs index 98c14bd..2342d8c 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs @@ -69,8 +69,8 @@ public EventPipeline( Running = false; var retryConfig = httpConfig != null - // Validated(): user-supplied config reaches us unclamped, unlike the - // CDN path which HttpConfigParser already validates. + // 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); diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs index 9c283f3..5678622 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs @@ -86,8 +86,8 @@ public SyncEventPipeline( _flushCancellationToken = flushCancellationToken ?? CancellationToken.None; var retryConfig = httpConfig != null - // Validated(): user-supplied config reaches us unclamped, unlike the - // CDN path which HttpConfigParser already validates. + // 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); From 983719f215ef4622753cc947f8dbe4b6febbb1f1 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Mon, 21 Sep 2026 15:20:57 -0400 Subject: [PATCH 6/9] Meet the remaining TAPI HTTP agreements: Authorization header, 511, generic Retry-After (529) (#148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Handle Retry-After on every retryable status, including 529 Route any retryable response carrying a valid Retry-After header through the rate-limit path (no retry-budget cost) instead of special-casing 429. Retryable statuses without Retry-After continue to use counted exponential backoff. Adds 529 to the retryable set and covers both paths with tests. Matches the behaviour already shipped in analytics-java 3.5.5 and the generic-retry-after conformance suite in sdk-e2e-tests. * Keep the batch when Retry-After routes it to the rate-limit path Routing any retryable status with Retry-After to the rate-limit path left ShouldDeleteBatch inconsistent with HandleResponse. With rate limiting on and backoff off, a 503 or 529 carrying Retry-After would rate-limit the pipeline (WaitUntilTime set, uploads blocked) while ShouldDeleteBatch still reported true, so the batch file was deleted and the pipeline then stalled waiting to retry events that no longer existed. That configuration is reachable from CDN settings and directly from Configuration.HttpConfig — it is the config ConfigurationHttpConfigTest builds. ShouldDeleteBatch now keeps a retryable batch whenever rate limiting is enabled, matching swift's shouldDropBatch ("Rate limit config handles retryable codes that carry Retry-After — don't drop"). Non-retryable statuses are still dropped, and a retryable status with neither rate limiting nor backoff enabled is still dropped since nothing would retry it. * Base the keep-or-delete decision on Retry-After, not just config The previous commit kept a retryable batch whenever rate limiting was enabled, which was too broad: a 500 with no Retry-After and backoff disabled was also kept, so the file was re-uploaded even though nothing had scheduled a retry. The sdk-e2e-tests "backoffConfig.enabled: false" case caught this — it expects exactly one request and saw two. ShouldDeleteBatch now takes the same retryAfterSeconds value handed to HandleResponse, so the two agree on whether the response actually took the rate-limit path. A retryable status keeps its batch only when it carries a usable Retry-After and rate limiting is on; otherwise only backoff can retry it, and with backoff off the batch is dropped as before. The single-argument overload is retained. 232 tests pass. * Treat 3xx as success, per spec item 1 Analytics-CSharp-plan.md states 'Spec item 1: 2xx and 3xx are success', but IsSuccessStatusCode and the two status checks in RetryStateMachine were 2xx-only, so a 3xx fell through to the retry classifier. analytics-go, analytics-python and analytics-php already follow the spec here; this brings C# into line with them and with its own plan. 236 tests pass, including new cases covering 200, 201, 301 and 304. * Send the Authorization header, and drop 511 Two Key Agreements from the HTTP response design doc that this SDK did not meet. The doc requires every SDK to send the write key in the Authorization header, and TAPI authenticates and routes on it instead of parsing the payload — which is the performance reason the header exists. This SDK sent no Authorization at all; it relied solely on the writeKey embedded in the batch body by Storage. Upload requests now carry Basic credentials built from the write key with an empty password, matching analytics-python, -go, -ruby, -php and -java, all of which send base64(":"). The value is exposed as a protected BasicAuthorization on HTTPClient rather than by widening _apiKey, so a custom IHTTPClientProvider can send the same header; the Unity sample, which overrides DoPost, now does. The writeKey stays in the payload, so nothing depends on the header alone yet. Separately, 511 Network Authentication Required was retryable here. The doc makes it conditional — "Authenticate, then retry if library supports OAuth" — and this SDK has no OAuth, so a 511 could never be satisfied and retrying only spent the budget. It joins 501 and 505 as an explicit Drop. analytics-python, the one SDK with OAuth, correctly retries 511 only when an OauthManager is configured; go, ruby, php and java exclude it as this now does. 240 tests pass, including new coverage of the header value, and all 79 e2e tests still pass. * Opt in to the e2e Authorization check The header assertion in sdk-e2e-tests is opt-in per SDK, since analytics-kotlin and analytics-swift do not send it yet. This SDK does, so it runs the check. --- .../Segment/Analytics/Retry/RetryConfig.cs | 6 +- .../Analytics/Retry/RetryStateMachine.cs | 42 ++++++++-- .../Analytics/Utilities/EventPipeline.cs | 8 +- .../Segment/Analytics/Utilities/HTTPClient.cs | 14 +++- .../Analytics/Utilities/RetryAfterParser.cs | 38 +++++++++ .../Analytics/Utilities/SyncEventPipeline.cs | 8 +- Samples/UnitySample/UnityHTTPClient.cs | 1 + Tests/Retry/RetryAfterDeleteBatchTest.cs | 82 +++++++++++++++++++ Tests/Retry/RetryAfterParserTest.cs | 81 ++++++++++++++++++ Tests/Retry/RetryStateMachineTest.cs | 66 +++++++++++++++ Tests/Utilities/HttpClientAuthTest.cs | 42 ++++++++++ e2e-cli/e2e-config.json | 3 +- 12 files changed, 368 insertions(+), 23 deletions(-) create mode 100644 Analytics-CSharp/Segment/Analytics/Utilities/RetryAfterParser.cs create mode 100644 Tests/Retry/RetryAfterDeleteBatchTest.cs create mode 100644 Tests/Retry/RetryAfterParserTest.cs create mode 100644 Tests/Utilities/HttpClientAuthTest.cs diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs index 35dbc42..fa36226 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs @@ -96,7 +96,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 2342d8c..1eac511 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/EventPipeline.cs @@ -211,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) { @@ -225,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/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 5678622..25ad712 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs @@ -236,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) { @@ -250,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/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" } } From 7593a634748f732e26b6689bca68cbdc48dd4476 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Tue, 22 Sep 2026 12:04:20 -0400 Subject: [PATCH 7/9] Treat only 2xx as a successful upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every one of these SDKs treated a 3xx as a failure before this work, and the change to 200-399 came from the design doc's "Spec item 1: 2xx and 3xx are success". That line is wrong, and the doc is what needs correcting. Measured against a local server, with the same HTTP clients these SDKs use: 307/308 + Location -> followed as POST with the body, arrives as 200 301/302/303 + Loc. -> followed as GET with no body, arrives as 200 302 without Location-> surfaces raw as 302 300 Multiple Choices-> surfaces raw as 300 304 Not Modified -> surfaces raw as 304 So a raw 3xx only reaches the classifier when the client has already declined to follow it, meaning nothing was uploaded. The one redirect that genuinely works, 307/308, never produces a 3xx here at all — it produces 200 — so narrowing the bound cannot break it. Nothing was gained by the wider range; a 300, 304, or Location-less 302 from a proxy was being logged as a delivered batch and dropped with no error callback. The narrower bound also needs no new branches: a 3xx is neither 5xx nor in the retryable 4xx set, so it already falls through to the non-retryable path and reports a failure. TAPI does not emit 3xx and has no plans to. This matters because host is customer-configurable and proxies in front of it are common. HttpClient follows what it can. Three sites narrowed, and the tests now assert 300/301/304 are not success rather than that they are. --- .../Analytics/Retry/RetryStateMachine.cs | 7 +++--- .../Segment/Analytics/Utilities/HTTPClient.cs | 5 +++-- Tests/Retry/RetryAfterDeleteBatchTest.cs | 22 ++++++++++++++++--- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs index deacdf5..853d9ed 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 < 400) + if (response.StatusCode >= 200 && response.StatusCode <= 299) 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 < 400) + if (response.StatusCode >= 200 && response.StatusCode <= 299) { return state.With( pipelineState: PipelineState.Ready, @@ -153,8 +153,7 @@ public bool ShouldDeleteBatch(int statusCode, int? retryAfterSeconds) if (IsLegacyMode) return statusCode >= 400 && statusCode <= 499 && statusCode != 429; - // Spec item 1: 2xx and 3xx are success. - if (statusCode >= 200 && statusCode < 400) + if (statusCode >= 200 && statusCode <= 299) return true; if (statusCode == 429) diff --git a/Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs b/Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs index b6946c0..fd5cd0e 100644 --- a/Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs +++ b/Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs @@ -198,8 +198,9 @@ public class Response /// /// A convenient method to check if the http request is successful /// - // Spec item 1: 2xx and 3xx are success. - public bool IsSuccessStatusCode => StatusCode >= 200 && StatusCode < 400; + // 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; } } diff --git a/Tests/Retry/RetryAfterDeleteBatchTest.cs b/Tests/Retry/RetryAfterDeleteBatchTest.cs index fa51e16..8a3d12a 100644 --- a/Tests/Retry/RetryAfterDeleteBatchTest.cs +++ b/Tests/Retry/RetryAfterDeleteBatchTest.cs @@ -56,14 +56,30 @@ public void RetryAfter_RateLimitsPipelineAndKeepsBatch() [Theory] [InlineData(200)] [InlineData(201)] - [InlineData(301)] - [InlineData(304)] + [InlineData(204)] public void SuccessStatuses_AreDeleted(int status) { - // Spec item 1: 2xx and 3xx are success, so the batch is done with. + // 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() { From caff264b12883ba89482701cccf12e5cad8541e4 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Tue, 22 Sep 2026 18:47:14 -0400 Subject: [PATCH 8/9] Floor maxRetryCount at 1 so zero does not drop everything unsent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ShouldUploadBatch compares a fresh state's counts against MaxRetryCount before the first attempt: RetryStateMachine.cs:92 checks GlobalRetryCount, which starts at 0. Validated() clamped with Math.Max(0, ...), so maxRetryCount: 0 — a plausible way to say "do not retry" — made that 0 >= 0 and dropped every batch before it was ever sent, not merely after a failure. Exposing HttpConfig publicly made that reachable from user code as well as from CDN settings, so both clamps now floor at 1. analytics-kotlin and analytics-swift clamp the same way and have the same hole. 251 tests pass, including one that asserts a fresh batch proceeds under maxRetryCount: 0; it fails against the old clamp. All 82 e2e tests pass. --- .../Segment/Analytics/Retry/RetryConfig.cs | 6 ++++-- Tests/Retry/ConfigurationHttpConfigTest.cs | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs index fa36226..e371e75 100644 --- a/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs +++ b/Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs @@ -18,7 +18,9 @@ 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)) ); } @@ -65,7 +67,7 @@ public BackoffConfig( 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)), diff --git a/Tests/Retry/ConfigurationHttpConfigTest.cs b/Tests/Retry/ConfigurationHttpConfigTest.cs index 6e15dc5..55add83 100644 --- a/Tests/Retry/ConfigurationHttpConfigTest.cs +++ b/Tests/Retry/ConfigurationHttpConfigTest.cs @@ -1,3 +1,4 @@ +using System; using Moq; using Segment.Analytics; using Segment.Analytics.Retry; @@ -87,6 +88,21 @@ public void SyncEventPipeline_WithoutHttpConfig_IsLegacyMode() 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() { From 385b47e48f562fcd124fd76d123c0f13f9c3ce02 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Tue, 22 Sep 2026 18:57:14 -0400 Subject: [PATCH 9/9] Add release notes for the HTTP response and retry work Records the retry/Retry-After work and, for the SDKs where a header is newly on the wire, an upgrade note: customers whose proxies allowlist request headers had uploads rejected by the already-released analytics-next change, and the same trap applies here. --- CHANGELOG.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 CHANGELOG.md 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.