Uh oh!
There was an error while loading. Please reload this page.
Expose HttpConfig so retry behaviour is user-configurable - #147
Open
MichaelGHSeg wants to merge 4 commits into
Open
Expose HttpConfig so retry behaviour is user-configurable#147MichaelGHSeg wants to merge 4 commits into
MichaelGHSeg wants to merge 4 commits into
Conversation
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.
This was referenced Sep 3, 2026
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The retry state machine added in #144 can only be configured from CDN settings.
RateLimitConfig,BackoffConfigandHttpConfigare allinternal, andConfigurationhas no entry point — so a C# consumer currently cannot set retry behaviour at all.Kotlin and Swift both expose this, as a single config object:
Configuration.httpConfig: HttpConfig? = null(publicdata class HttpConfig)public func httpConfig(_ config: HttpConfig?) -> ConfigurationThis brings C# in line with the two SDKs #144 was explicitly written to match.
What
RetryBehavior,RateLimitConfig,BackoffConfigandHttpConfigpublic.RetryConfigstaysinternal— it's plumbing built fromHttpConfig, never supplied by callers.Configuration.HttpConfig, as a trailing optional constructor argument so existing positional callers are unaffected. Defaults tonull, preserving today's CDN-only behaviour exactly.EventPipelineProvider/SyncEventPipelineProviderpass it through as the pipeline's starting retry config. CDN settings still override it later viaUpdateHttpConfig.HttpConfigpublic, so a customIEventPipelineProvidercan pass one on rather than only read it.Usage
Testing
216 tests pass, including 6 new cases in
Tests/Retry/ConfigurationHttpConfigTest.csasserting that a config set onConfigurationreaches both pipelines' retry state machines (and that omitting it still yields legacy mode).Notes
This supersedes the
Configurationportion of #143, which added individualMaxRetries/MaxTotalBackoffDuration/MaxRateLimitDurationknobs. That shape matches analytics-python but not Kotlin/Swift; #143 and #144 were independent branches off85f025eand never shared history, so the divergence was never reconciled.