Unified HTTP response handling and retry behavior, enabled by default

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,24 @@ private set
 
         public IEventPipelineProvider EventPipelineProvider { get; }
 
+        /// <summary>
+        /// HTTP retry configuration for rate limiting and exponential backoff. Defaults to
+        /// <c>null</c>, which runs rate limiting and backoff with their built-in defaults. Pass a
+        /// config to change them, or one with both subsystems disabled to opt out of retrying.
+        /// Set it before constructing <c>Analytics</c>, e.g.
+        /// <c>new Configuration("writeKey") { HttpConfig = new HttpConfig(...) }</c>.
+        /// Mirrors analytics-kotlin's mutable <c>Configuration.httpConfig</c>.
+        /// <para>
+        /// This sets the pipeline's starting configuration only. CDN settings take precedence:
+        /// any settings payload carrying an <c>httpConfig</c> key replaces the configuration the
+        /// pipeline is running with — this property keeps the value you set — and a CDN payload is
+        /// treated as enabling a subsystem unless it says <c>"enabled": "false"</c>. A payload with
+        /// no <c>httpConfig</c> key leaves this value in effect. This matches the behavior of
+        /// analytics-kotlin and analytics-swift.
+        /// </para>
+        /// </summary>
+        public HttpConfig HttpConfig { get; set; }
+
         /// <summary>
         /// Configuration that analytics can use
         /// </summary>

Analytics-CSharp/Segment/Analytics/Retry/HttpConfigParser.cs

@@ -46,20 +46,28 @@ private static RateLimitConfig ParseRateLimitConfig(JsonObject json, bool enable
             if (json == null)
                 return new RateLimitConfig(enabled: enabled);
 
-            int maxRetryCount = 100;
+            var defaults = new RateLimitConfig();
+
+            int maxRetryCount = defaults.MaxRetryCount;
             string maxRetriesStr = json.GetString("maxRetryCount");
             if (maxRetriesStr != null && int.TryParse(maxRetriesStr, out int parsedMaxRetries))
                 maxRetryCount = parsedMaxRetries;
 
-            int maxRetryInterval = 300;
+            int maxRetryInterval = defaults.MaxRetryInterval;
             string intervalStr = json.GetString("maxRetryInterval");
             if (intervalStr != null && int.TryParse(intervalStr, out int parsedInterval))
                 maxRetryInterval = parsedInterval;
 
+            long maxRateLimitDuration = defaults.MaxRateLimitDuration;
+            string durationStr = json.GetString("maxRateLimitDuration");
+            if (durationStr != null && long.TryParse(durationStr, out long parsedDuration))
+                maxRateLimitDuration = parsedDuration;
+
             return new RateLimitConfig(
                 enabled: enabled,
                 maxRetryCount: maxRetryCount,
-                maxRetryInterval: maxRetryInterval
+                maxRetryInterval: maxRetryInterval,
+                maxRateLimitDuration: maxRateLimitDuration
             );
         }
 
@@ -68,27 +76,29 @@ private static BackoffConfig ParseBackoffConfig(JsonObject json, bool enabled)
             if (json == null)
                 return new BackoffConfig(enabled: enabled);
 
-            int maxRetryCount = 100;
+            var defaults = new BackoffConfig();
+
+            int maxRetryCount = defaults.MaxRetryCount;
             string maxRetriesStr = json.GetString("maxRetryCount");
             if (maxRetriesStr != null && int.TryParse(maxRetriesStr, out int parsedMaxRetries))
                 maxRetryCount = parsedMaxRetries;
 
-            double baseBackoffInterval = 0.5;
+            double baseBackoffInterval = defaults.BaseBackoffInterval;
             string baseStr = json.GetString("baseBackoffInterval");
             if (baseStr != null && double.TryParse(baseStr, NumberStyles.Float, CultureInfo.InvariantCulture, out double parsedBase))
                 baseBackoffInterval = parsedBase;
 
-            int maxBackoffInterval = 300;
+            int maxBackoffInterval = defaults.MaxBackoffInterval;
             string maxStr = json.GetString("maxBackoffInterval");
             if (maxStr != null && int.TryParse(maxStr, out int parsedMax))
                 maxBackoffInterval = parsedMax;
 
-            long maxTotalBackoffDuration = 43200;
+            long maxTotalBackoffDuration = defaults.MaxTotalBackoffDuration;
             string durationStr = json.GetString("maxTotalBackoffDuration");
             if (durationStr != null && long.TryParse(durationStr, out long parsedDuration))
                 maxTotalBackoffDuration = parsedDuration;
 
-            int jitterPercent = 10;
+            int jitterPercent = defaults.JitterPercent;
             string jitterStr = json.GetString("jitterPercent");
             if (jitterStr != null && int.TryParse(jitterStr, out int parsedJitter))
                 jitterPercent = parsedJitter;

Analytics-CSharp/Segment/Analytics/Retry/RetryConfig.cs

@@ -3,27 +3,48 @@
 
 namespace Segment.Analytics.Retry
 {
-    internal class RateLimitConfig
+    public class RateLimitConfig
     {
+        /// <summary>Largest Retry-After the client will honour, in seconds. RFC 7231 allows
+        /// more, but the TAPI agreements cap it here and the other SDKs fix it at this value.</summary>
+        public const int MaxRetryIntervalCeiling = 300;
+
         public bool Enabled { get; }
         public int MaxRetryCount { get; }
         public int MaxRetryInterval { get; }
 
-        public RateLimitConfig(bool enabled = false, int maxRetryCount = 100, int maxRetryInterval = 300)
+        /// <summary>
+        /// Wall-clock ceiling, in seconds, on how long one rate-limit episode may keep a
+        /// batch alive. A last-ditch guard so a pathological Retry-After stream cannot hold
+        /// a batch forever; <see cref="MaxRetryCount"/> is what stops retrying in practice.
+        /// At the defaults the count is reached first by a wide margin, since
+        /// MaxRetryCount * MaxRetryIntervalCeiling is well under this.
+        /// </summary>
+        public long MaxRateLimitDuration { get; }
+
+        public RateLimitConfig(
+            bool enabled = true,
+            int maxRetryCount = 100,
+            int maxRetryInterval = 300,
+            long maxRateLimitDuration = 43200)
         {
             Enabled = enabled;
             MaxRetryCount = maxRetryCount;
             MaxRetryInterval = maxRetryInterval;
+            MaxRateLimitDuration = maxRateLimitDuration;
         }
 
         public RateLimitConfig Validated() => new RateLimitConfig(
             enabled: Enabled,
-            maxRetryCount: Math.Max(0, Math.Min(MaxRetryCount, 1000)),
-            maxRetryInterval: Math.Max(1, Math.Min(MaxRetryInterval, 3600))
+            // 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, MaxRetryIntervalCeiling)),
+            maxRateLimitDuration: Math.Max(0, Math.Min(MaxRateLimitDuration, 604800))
         );
     }
 
-    internal class BackoffConfig
+    public class BackoffConfig
     {
         public bool Enabled { get; }
         public int MaxRetryCount { get; }
@@ -37,10 +58,10 @@ internal class BackoffConfig
         public Dictionary<int, RetryBehavior> StatusCodeOverrides { get; }
 
         public BackoffConfig(
-            bool enabled = false,
-            int maxRetryCount = 100,
+            bool enabled = true,
+            int maxRetryCount = 10,
             double baseBackoffInterval = 0.5,
-            int maxBackoffInterval = 300,
+            int maxBackoffInterval = 60,
             long maxTotalBackoffDuration = 43200,
             int jitterPercent = 10,
             RetryBehavior default4xxBehavior = RetryBehavior.Drop,
@@ -57,15 +78,30 @@ public BackoffConfig(
             Default4xxBehavior = default4xxBehavior;
             Default5xxBehavior = default5xxBehavior;
             UnknownCodeBehavior = unknownCodeBehavior;
-            StatusCodeOverrides = statusCodeOverrides ?? DefaultStatusCodeOverrides;
+            // Merged over the defaults, not substituted for them. Replacing meant that
+            // overriding one status silently changed seven others: 408, 410, 429 and
+            // 460 stopped being retried, and 511 fell through to Default5xxBehavior
+            // and started being retried, which is the one thing it must never do.
+            // Copied rather than aliased because the property is public, so sharing
+            // the static default would let one caller's mutation corrupt every
+            // BackoffConfig built afterwards.
+            StatusCodeOverrides = new Dictionary<int, RetryBehavior>(DefaultStatusCodeOverrides);
+            if (statusCodeOverrides != null)
+            {
+                foreach (KeyValuePair<int, RetryBehavior> kvp in statusCodeOverrides)
+                    StatusCodeOverrides[kvp.Key] = kvp.Value;
+            }
         }
 
         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)),
+            // Floored at 1 for the same reason as maxRetryCount: ExceedsMaxDuration
+            // compares elapsed time against this, so 0 meant "no budget" — the batch
+            // was abandoned on its second attempt — rather than "no cap".
+            maxTotalBackoffDuration: Math.Max(1, Math.Min(MaxTotalBackoffDuration, 604800)),
             jitterPercent: Math.Max(0, Math.Min(JitterPercent, 50)),
             default4xxBehavior: Default4xxBehavior,
             default5xxBehavior: Default5xxBehavior,
@@ -93,7 +129,11 @@ private static Dictionary<int, RetryBehavior> 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 +149,7 @@ public RetryConfig(RateLimitConfig rateLimitConfig = null, BackoffConfig backoff
         }
     }
 
-    internal class HttpConfig
+    public class HttpConfig
     {
         public RateLimitConfig RateLimitConfig { get; }
         public BackoffConfig BackoffConfig { get; }

Analytics-CSharp/Segment/Analytics/Retry/RetryState.cs

@@ -34,6 +34,11 @@ internal class RetryState
         public PipelineState PipelineState { get; }
         public long? WaitUntilTime { get; }
         public int GlobalRetryCount { get; }
+
+        /// <summary>When the current rate-limit episode began, for MaxRateLimitDuration.
+        /// Null outside an episode; cleared on the first success.</summary>
+        public long? RateLimitStartTime { get; }
+
         public Dictionary<string, BatchMetadata> BatchMetadata { get; }
 
         private static readonly Dictionary<string, BatchMetadata> s_emptyMetadata =
@@ -43,11 +48,13 @@ public RetryState(
             PipelineState pipelineState = PipelineState.Ready,
             long? waitUntilTime = null,
             int globalRetryCount = 0,
-            Dictionary<string, BatchMetadata> batchMetadata = null)
+            Dictionary<string, BatchMetadata> batchMetadata = null,
+            long? rateLimitStartTime = null)
         {
             PipelineState = pipelineState;
             WaitUntilTime = waitUntilTime;
             GlobalRetryCount = globalRetryCount;
+            RateLimitStartTime = rateLimitStartTime;
             BatchMetadata = batchMetadata ?? s_emptyMetadata;
         }
 
@@ -63,13 +70,18 @@ public RetryState With(
             long? waitUntilTime = null,
             bool clearWaitUntilTime = false,
             int? globalRetryCount = null,
-            Dictionary<string, BatchMetadata> batchMetadata = null)
+            Dictionary<string, BatchMetadata> batchMetadata = null,
+            long? rateLimitStartTime = null,
+            bool clearRateLimitStartTime = false)
         {
             return new RetryState(
                 pipelineState: pipelineState ?? PipelineState,
                 waitUntilTime: clearWaitUntilTime ? null : (waitUntilTime ?? WaitUntilTime),
                 globalRetryCount: globalRetryCount ?? GlobalRetryCount,
-                batchMetadata: batchMetadata ?? BatchMetadata
+                batchMetadata: batchMetadata ?? BatchMetadata,
+                rateLimitStartTime: clearRateLimitStartTime
+                    ? null
+                    : (rateLimitStartTime ?? RateLimitStartTime)
             );
         }
 

Analytics-CSharp/Segment/Analytics/Retry/RetryStateMachine.cs

@@ -37,19 +37,33 @@ public RetryState HandleResponse(RetryState state, ResponseInfo response)
                     pipelineState: PipelineState.Ready,
                     clearWaitUntilTime: true,
                     globalRetryCount: 0,
-                    batchMetadata: RemoveFromMetadata(state, response.BatchFile)
+                    batchMetadata: RemoveFromMetadata(state, response.BatchFile),
+                    clearRateLimitStartTime: true
                 );
             }
 
+            // 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)
                     return HandleRateLimitResponse(state, response, currentTime);
+                // Dropped rather than handed to backoff: rateLimitConfig.enabled:false is a
+                // kill switch for 429 handling, symmetric with backoffConfig.enabled:false
+                // for 5xx. Asserted by the shared e2e suite's settings-enabled-flag tests.
                 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);
@@ -83,13 +97,29 @@ public Tuple<UploadDecision, RetryState> ShouldUploadBatch(RetryState state, str
                 && clearedState.GlobalRetryCount >= _config.RateLimitConfig.MaxRetryCount)
             {
                 RetryState resetState = clearedState
-                    .With(globalRetryCount: 0)
+                    .With(globalRetryCount: 0, clearRateLimitStartTime: true)
                     .RemoveBatch(batchFile);
                 return Tuple.Create(
                     UploadDecision.DropBatch(DropReason.MaxRetriesExceeded),
                     resetState);
             }
 
+            // Check 2b: how long this rate-limit episode has run. A last-ditch guard so a
+            // pathological Retry-After stream cannot hold a batch indefinitely; at the
+            // defaults Check 2 is reached long before this.
+            if (_config.RateLimitConfig.Enabled
+                && clearedState.RateLimitStartTime.HasValue
+                && currentTime - clearedState.RateLimitStartTime.Value
+                    >= _config.RateLimitConfig.MaxRateLimitDuration * 1000)
+            {
+                RetryState resetState = clearedState
+                    .With(globalRetryCount: 0, clearRateLimitStartTime: true)
+                    .RemoveBatch(batchFile);
+                return Tuple.Create(
+                    UploadDecision.DropBatch(DropReason.MaxDurationExceeded),
+                    resetState);
+            }
+
             // Check 3: Per-batch metadata
             BatchMetadata metadata;
             if (clearedState.BatchMetadata.TryGetValue(batchFile, out metadata))
@@ -131,20 +161,37 @@ 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);
+
+        /// <summary>
+        /// Whether the batch file should be removed. <paramref name="retryAfterSeconds"/> must be
+        /// the same value handed to <see cref="HandleResponse"/>, so that the two agree on whether
+        /// this response took the rate-limit path.
+        /// </summary>
+        public bool ShouldDeleteBatch(int statusCode, int? retryAfterSeconds)
         {
             if (IsLegacyMode)
                 return statusCode >= 400 && statusCode <= 499 && statusCode != 429;
 
             if (statusCode >= 200 && statusCode <= 299)
                 return true;
 
+            // Matches HandleResponse: with rate limiting off, a 429 is dropped rather than
+            // falling through to backoff.
             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;
         }
@@ -155,7 +202,10 @@ private RetryState HandleRateLimitResponse(RetryState state, ResponseInfo respon
             return state.With(
                 pipelineState: PipelineState.RateLimited,
                 waitUntilTime: waitUntilTimeMs,
-                globalRetryCount: state.GlobalRetryCount + 1
+                globalRetryCount: state.GlobalRetryCount + 1,
+                // Stamped on the first rate-limited response of an episode and left alone
+                // afterwards, so MaxRateLimitDuration measures the whole episode.
+                rateLimitStartTime: state.RateLimitStartTime ?? currentTime
             );
         }