From ab15699d6299b3212a17574f194033aea849a66a Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 7 Aug 2026 16:26:16 -0400 Subject: [PATCH 01/12] feat: conform FDv1 streaming and polling to RETRY spec (SDK-2788) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bring the FDv1 streaming and polling data sources into conformance with the RETRY specification: no HTTP response and no transport-level failure causes the data source to permanently stop. Previously-terminal 4xx statuses (401, 403, 405, other 4xx that aren't 400/408/429) and TLS/cert validation failures now trigger an extended-regime backoff and continue retrying indefinitely. Streaming: classifies each failure via classifyHTTPFailure / classifyTransportFailure into Normal or Unexpected. Unexpected failures activate an extended retry curve on the underlying eventsource stream (base 5 min, max 1 hour, doubling with jitter) instead of stopping. Sixty seconds of continuous healthy operation returns the source to the normal-regime curve. Polling: introduces pollingStrategy, an encapsulated RETRY §1.4 state machine (attempts / regime / rng) with a wait floor at PollInterval per RETRY §1.4.4. Unexpected classifications engage the extended regime with initialDelay = max(configured, PollInterval); two consecutive successful polls reset back to the normal regime per RETRY §1.8. The poll loop uses a dynamically-resettable timer to serve the state machine. Configuration surface (public builders): unchanged. New test-only knobs are exposed via .Internal() escape hatches on both builders, with matching free-function wrappers in the new testhelpers/datasourcetest package. Production code must not import that package; it exists so that contract tests can compress the extended regime into an observable window. Testservice: adds ExtendedInitialDelayMS / ResetThresholdMS knobs on the streaming servicedef, ExtendedInitialDelayMS on polling, and declares the retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling capabilities so the sdk-test-harness can drive the new conformance suite. go.mod / testservice/go.mod carry a temporary local replace directive pointing to ../eventsource so this branch can build against the unreleased RetryCurve API in eventsource PR #68. The TODO comment on the replace notes it must be removed once the eventsource release tagging that API ships. --- go.mod | 5 + go.sum | 2 - internal/datasource/helpers.go | 112 +++++++++++--- internal/datasource/helpers_test.go | 42 ++++- internal/datasource/polling_data_source.go | 77 ++++------ .../datasource/polling_data_source_test.go | 101 +++++++++++-- internal/datasource/polling_strategy.go | 101 +++++++++++++ internal/datasource/polling_strategy_test.go | 143 ++++++++++++++++++ internal/datasource/streaming_data_source.go | 101 ++++++++----- .../datasource/streaming_data_source_test.go | 55 +++++-- ldclient_end_to_end_test.go | 93 +++++++++--- ldcomponents/polling_data_source_builder.go | 45 +++++- ldcomponents/streaming_data_source_builder.go | 67 +++++++- testhelpers/datasourcetest/datasourcetest.go | 50 ++++++ testservice/go.mod | 9 +- testservice/go.sum | 6 +- testservice/sdk_client_entity.go | 13 ++ testservice/service.go | 2 + testservice/servicedef/sdk_config.go | 15 +- testservice/servicedef/service_params.go | 52 ++++--- 20 files changed, 882 insertions(+), 209 deletions(-) create mode 100644 internal/datasource/polling_strategy.go create mode 100644 internal/datasource/polling_strategy_test.go create mode 100644 testhelpers/datasourcetest/datasourcetest.go diff --git a/go.mod b/go.mod index 9a345af1..8bf2c2fc 100644 --- a/go.mod +++ b/go.mod @@ -35,3 +35,8 @@ require ( ) retract [v7.15.1, v7.15.2] // Introduced unintentional breaking changes; use version v7.15.3 or later. + +// TODO(SDK-2788): temporary local replace so this branch can build against the +// unreleased RetryCurve API in eventsource PR #68. Remove after the eventsource +// PR merges and a release with the API is tagged (>=1.12.0 expected). +replace github.com/launchdarkly/eventsource => ../eventsource diff --git a/go.sum b/go.sum index 3f81bb71..418259bd 100644 --- a/go.sum +++ b/go.sum @@ -17,8 +17,6 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/launchdarkly/ccache v1.1.0 h1:voD1M+ZJXR3MREOKtBwgTF9hYHl1jg+vFKS/+VAkR2k= github.com/launchdarkly/ccache v1.1.0/go.mod h1:TlxzrlnzvYeXiLHmesMuvoZetu4Z97cV1SsdqqBJi1Q= -github.com/launchdarkly/eventsource v1.10.0 h1:H9Tp6AfGu/G2qzBJC26iperrvwhzdbiA/gx7qE2nDFI= -github.com/launchdarkly/eventsource v1.10.0/go.mod h1:J3oa50bPvJesZqNAJtb5btSIo5N6roDWhiAS3IpsKck= github.com/launchdarkly/go-jsonstream/v3 v3.1.1 h1:ugupp2eNtwVbr69KCdeUrm1vUf1/3ju4Wdliaob95uY= github.com/launchdarkly/go-jsonstream/v3 v3.1.1/go.mod h1:ZBjhKq8mhArCtqotGRGnteY6eXpNm1GaOdUSZHh+ZjM= github.com/launchdarkly/go-ntlm-proxy-auth v1.0.3 h1:i3V0N+R0Fd2nXfGEVKCBIZ8kyttZ+SRKvBG8cdcphO4= diff --git a/internal/datasource/helpers.go b/internal/datasource/helpers.go index 5da570e0..b304fd4e 100644 --- a/internal/datasource/helpers.go +++ b/internal/datasource/helpers.go @@ -1,6 +1,9 @@ package datasource import ( + "crypto/tls" + "crypto/x509" + "errors" "fmt" "net/http" @@ -20,53 +23,114 @@ func (e httpStatusError) Error() string { return e.Message } -// Tests whether an HTTP error status represents a condition that might resolve on its own if we retry, -// or at least should not make us permanently stop sending requests. -func isHTTPErrorRecoverable(statusCode int) bool { +// FailureClass categorizes a data source failure per RETRY §1.5–§1.7. Under the +// RETRY spec no failure is permanently terminal: every failure is either "normal" +// (regular backoff and retry) or "unexpected" (extended backoff via a longer +// retry curve or wait interval, still retrying indefinitely). +type FailureClass int + +const ( + // FailureClassNormal indicates a failure that a caller should treat as an + // ordinary transient error. + FailureClassNormal FailureClass = iota + // FailureClassUnexpected indicates a failure that the caller should treat as + // signalling a durable, non-transient upstream problem. + FailureClassUnexpected +) + +// classifyHTTPFailure returns the failure classification for an HTTP status code +// received during a data source request, per RETRY §1.6. Called only when the +// status indicates failure (non-2xx). +func classifyHTTPFailure(statusCode int) FailureClass { if statusCode >= 400 && statusCode < 500 { switch statusCode { - case 400: // bad request - return true - case 408: // request timeout - return true - case 429: // too many requests - return true + case 400, 408, 429: + return FailureClassNormal default: - return false // all other 4xx errors are unrecoverable + return FailureClassUnexpected } } - return true + return FailureClassNormal +} + +// classifyTransportFailure returns the failure classification for a transport-layer +// error (i.e., not an HTTP response, but a lower-level network or TLS failure) +// per RETRY §1.7. TLS/certificate validation failures are treated as unexpected; +// everything else is treated as normal. +func classifyTransportFailure(err error) FailureClass { + if err == nil { + return FailureClassNormal + } + var certErr *tls.CertificateVerificationError + if errors.As(err, &certErr) { + return FailureClassUnexpected + } + var x509UnknownAuthorityErr x509.UnknownAuthorityError + if errors.As(err, &x509UnknownAuthorityErr) { + return FailureClassUnexpected + } + var x509HostnameErr x509.HostnameError + if errors.As(err, &x509HostnameErr) { + return FailureClassUnexpected + } + var x509InvalidErr x509.CertificateInvalidError + if errors.As(err, &x509InvalidErr) { + return FailureClassUnexpected + } + return FailureClassNormal } func httpErrorDescription(statusCode int) string { message := "" if statusCode == 401 || statusCode == 403 { - message = " (invalid SDK key)" + message = " (authentication failed)" } return fmt.Sprintf("HTTP error %d%s", statusCode, message) } -// Logs an HTTP error or network error at the appropriate level and determines whether it is recoverable -// (as defined by isHTTPErrorRecoverable). -func checkIfErrorIsRecoverableAndLog( +// classifyAndLogHTTPFailure classifies an HTTP failure per RETRY §1.6, logs it +// at the appropriate level, and returns the classification for the caller to act +// on. Never signals "give up permanently" — under the RETRY spec the caller must +// continue retrying, though possibly with an extended backoff regime. +func classifyAndLogHTTPFailure( loggers ldlog.Loggers, errorDesc, errorContext string, statusCode int, - recoverableMessage string, -) bool { - if statusCode > 0 && !isHTTPErrorRecoverable(statusCode) { - loggers.Errorf("Error %s (giving up permanently): %s", errorContext, errorDesc) - return false + willRetryMessage string, +) FailureClass { + class := classifyHTTPFailure(statusCode) + if class == FailureClassUnexpected { + loggers.Errorf("Error %s (%s; will continue retrying with extended backoff): %s", + errorContext, willRetryMessage, errorDesc) + } else { + loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, errorDesc) } - loggers.Warnf("Error %s (%s): %s", errorContext, recoverableMessage, errorDesc) - return true + return class +} + +// classifyAndLogTransportFailure classifies a transport-layer failure per RETRY +// §1.7, logs it, and returns the classification. +func classifyAndLogTransportFailure( + loggers ldlog.Loggers, + err error, + errorContext, willRetryMessage string, +) FailureClass { + class := classifyTransportFailure(err) + if class == FailureClassUnexpected { + loggers.Errorf("Error %s (%s; will continue retrying with extended backoff): %s", + errorContext, willRetryMessage, err.Error()) + } else { + loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, err.Error()) + } + return class } func checkForHTTPError(statusCode int, url string) error { if statusCode == http.StatusUnauthorized { return httpStatusError{ - Message: fmt.Sprintf("Invalid SDK key when accessing URL: %s. Verify that your SDK key is correct.", url), - Code: statusCode} + Message: fmt.Sprintf("Authentication failed for URL: %s. If this persists, verify that your SDK key is correct.", + url), + Code: statusCode} } if statusCode == http.StatusNotFound { diff --git a/internal/datasource/helpers_test.go b/internal/datasource/helpers_test.go index baca96c8..80f2fc29 100644 --- a/internal/datasource/helpers_test.go +++ b/internal/datasource/helpers_test.go @@ -1,6 +1,10 @@ package datasource import ( + "crypto/tls" + "crypto/x509" + "errors" + "fmt" "strconv" "testing" @@ -12,19 +16,47 @@ func TestHTTPStatusError(t *testing.T) { assert.Equal(t, "message", error.Error()) } -func TestIsHTTPErrorRecoverable(t *testing.T) { +// classifyHTTPFailure per RETRY §1.6: 400, 408, 429 are normal (transient +// server-side conditions); all other 4xx are unexpected (durable client-side +// misconfiguration); 5xx and everything else are normal. +func TestClassifyHTTPFailure(t *testing.T) { for i := 400; i < 500; i++ { - assert.Equal(t, i == 400 || i == 408 || i == 429, isHTTPErrorRecoverable(i), strconv.Itoa(i)) + expected := FailureClassNormal + if !(i == 400 || i == 408 || i == 429) { + expected = FailureClassUnexpected + } + assert.Equal(t, expected, classifyHTTPFailure(i), strconv.Itoa(i)) } for i := 500; i < 600; i++ { - assert.True(t, isHTTPErrorRecoverable(i)) + assert.Equal(t, FailureClassNormal, classifyHTTPFailure(i), strconv.Itoa(i)) } } +// classifyTransportFailure per RETRY §1.7: TLS/certificate validation failures +// are unexpected; other transport-layer errors are normal. +func TestClassifyTransportFailure(t *testing.T) { + assert.Equal(t, FailureClassNormal, classifyTransportFailure(nil)) + assert.Equal(t, FailureClassNormal, classifyTransportFailure(errors.New("boom"))) + + // TLS certificate errors. + assert.Equal(t, FailureClassUnexpected, + classifyTransportFailure(&tls.CertificateVerificationError{})) + assert.Equal(t, FailureClassUnexpected, + classifyTransportFailure(x509.UnknownAuthorityError{})) + assert.Equal(t, FailureClassUnexpected, + classifyTransportFailure(x509.HostnameError{Host: "example.invalid"})) + assert.Equal(t, FailureClassUnexpected, + classifyTransportFailure(x509.CertificateInvalidError{Reason: x509.Expired})) + + // Wrapped errors are still classified correctly. + wrapped := fmt.Errorf("some wrapper: %w", x509.UnknownAuthorityError{}) + assert.Equal(t, FailureClassUnexpected, classifyTransportFailure(wrapped)) +} + func TestHTTPErrorDescription(t *testing.T) { assert.Equal(t, "HTTP error 400", httpErrorDescription(400)) - assert.Equal(t, "HTTP error 401 (invalid SDK key)", httpErrorDescription(401)) - assert.Equal(t, "HTTP error 403 (invalid SDK key)", httpErrorDescription(403)) + assert.Equal(t, "HTTP error 401 (authentication failed)", httpErrorDescription(401)) + assert.Equal(t, "HTTP error 403 (authentication failed)", httpErrorDescription(403)) assert.Equal(t, "HTTP error 500", httpErrorDescription(500)) } diff --git a/internal/datasource/polling_data_source.go b/internal/datasource/polling_data_source.go index e5685f53..fce7d340 100644 --- a/internal/datasource/polling_data_source.go +++ b/internal/datasource/polling_data_source.go @@ -21,9 +21,10 @@ const ( // PollingConfig describes the configuration for a polling data source. It is exported so that // it can be used in the PollingDataSourceBuilder. type PollingConfig struct { - BaseURI string - PollInterval time.Duration - FilterKey string + BaseURI string + PollInterval time.Duration + FilterKey string + ExtendedInitialPollInterval time.Duration } // Requester allows PollingProcessor to delegate fetching data to another component. @@ -43,6 +44,7 @@ type PollingProcessor struct { dataSourceUpdates subsystems.DataSourceUpdateSink requester Requester pollInterval time.Duration + strategy *pollingStrategy loggers ldlog.Loggers setInitializedOnce sync.Once isInitialized internal.AtomicBoolean @@ -57,7 +59,10 @@ func NewPollingProcessor( cfg PollingConfig, ) *PollingProcessor { httpRequester := NewPollingRequester(context, context.GetHTTP().CreateHTTPClient(), cfg.BaseURI, cfg.FilterKey) - return newPollingProcessor(context, dataSourceUpdates, httpRequester, cfg.PollInterval) + return newPollingProcessor( + context, dataSourceUpdates, httpRequester, + cfg.PollInterval, cfg.ExtendedInitialPollInterval, + ) } func newPollingProcessor( @@ -65,11 +70,13 @@ func newPollingProcessor( dataSourceUpdates subsystems.DataSourceUpdateSink, requester Requester, pollInterval time.Duration, + extendedInitialPollInterval time.Duration, ) *PollingProcessor { pp := &PollingProcessor{ dataSourceUpdates: dataSourceUpdates, requester: requester, pollInterval: pollInterval, + strategy: newPollingStrategy(pollInterval, extendedInitialPollInterval), loggers: context.GetLogging().Loggers, quit: make(chan struct{}), } @@ -80,10 +87,13 @@ func newPollingProcessor( func (pp *PollingProcessor) Start(closeWhenReady chan<- struct{}) { pp.loggers.Infof("Starting LaunchDarkly polling with interval: %+v", pp.pollInterval) - ticker := newTickerWithInitialTick(pp.pollInterval) + // Fires immediately for the first poll; Reset after each iteration to schedule + // the next. Under RETRY (SDK-2775), the interval between polls is dynamic per + // the pollingStrategy state machine, so we can't use a fixed-period Ticker. + timer := time.NewTimer(0) go func() { - defer ticker.Stop() + defer timer.Stop() var readyOnce sync.Once notifyReady := func() { @@ -98,28 +108,23 @@ func (pp *PollingProcessor) Start(closeWhenReady chan<- struct{}) { select { case <-pp.quit: return - case <-ticker.C: + case <-timer.C: if err := pp.poll(); err != nil { + var class FailureClass if hse, ok := err.(httpStatusError); ok { errorInfo := interfaces.DataSourceErrorInfo{ Kind: interfaces.DataSourceErrorKindErrorResponse, StatusCode: hse.Code, Time: time.Now(), } - recoverable := checkIfErrorIsRecoverableAndLog( + class = classifyAndLogHTTPFailure( pp.loggers, httpErrorDescription(hse.Code), pollingErrorContext, hse.Code, pollingWillRetryMessage, ) - if recoverable { - pp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateInterrupted, errorInfo) - } else { - pp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateOff, errorInfo) - notifyReady() - return - } + pp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateInterrupted, errorInfo) } else { errorInfo := interfaces.DataSourceErrorInfo{ Kind: interfaces.DataSourceErrorKindNetworkError, @@ -129,17 +134,22 @@ func (pp *PollingProcessor) Start(closeWhenReady chan<- struct{}) { if _, ok := err.(malformedJSONError); ok { errorInfo.Kind = interfaces.DataSourceErrorKindInvalidData } - checkIfErrorIsRecoverableAndLog(pp.loggers, err.Error(), pollingErrorContext, 0, pollingWillRetryMessage) + class = classifyAndLogTransportFailure( + pp.loggers, err, pollingErrorContext, pollingWillRetryMessage, + ) pp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateInterrupted, errorInfo) } - continue + pp.strategy.OnFailure(class) + } else { + pp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateValid, interfaces.DataSourceErrorInfo{}) + pp.setInitializedOnce.Do(func() { + pp.isInitialized.Set(true) + pp.loggers.Info("First polling request successful") + notifyReady() + }) + pp.strategy.OnSuccess() } - pp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateValid, interfaces.DataSourceErrorInfo{}) - pp.setInitializedOnce.Do(func() { - pp.isInitialized.Set(true) - pp.loggers.Info("First polling request successful") - notifyReady() - }) + timer.Reset(pp.strategy.NextWait()) } } }() @@ -190,24 +200,3 @@ func (pp *PollingProcessor) GetPollInterval() time.Duration { func (pp *PollingProcessor) GetFilterKey() string { return pp.requester.FilterKey() } - -type tickerWithInitialTick struct { - *time.Ticker - C <-chan time.Time -} - -func newTickerWithInitialTick(interval time.Duration) *tickerWithInitialTick { - c := make(chan time.Time) - ticker := time.NewTicker(interval) - t := &tickerWithInitialTick{ - C: c, - Ticker: ticker, - } - go func() { - c <- time.Now() // Ensure we do an initial poll immediately - for tt := range ticker.C { - c <- tt - } - }() - return t -} diff --git a/internal/datasource/polling_data_source_test.go b/internal/datasource/polling_data_source_test.go index 2e31f1f8..4d8b8acb 100644 --- a/internal/datasource/polling_data_source_test.go +++ b/internal/datasource/polling_data_source_test.go @@ -27,7 +27,7 @@ func TestPollingProcessorClosingItShouldNotBlock(t *testing.T) { r.RequestAllRespCh <- mocks.RequestAllResponse{} withMockDataSourceUpdates(func(dataSourceUpdates *mocks.MockDataSourceUpdates) { - p := newPollingProcessor(sharedtest.BasicClientContext(), dataSourceUpdates, r, time.Minute) + p := newPollingProcessor(sharedtest.BasicClientContext(), dataSourceUpdates, r, time.Minute, 0) p.Close() @@ -49,7 +49,7 @@ func TestPollingProcessorInitialization(t *testing.T) { r.RequestAllRespCh <- resp withMockDataSourceUpdates(func(dataSourceUpdates *mocks.MockDataSourceUpdates) { - p := newPollingProcessor(sharedtest.BasicClientContext(), dataSourceUpdates, r, time.Millisecond*10) + p := newPollingProcessor(sharedtest.BasicClientContext(), dataSourceUpdates, r, time.Millisecond*10, 0) defer p.Close() closeWhenReady := make(chan struct{}) @@ -116,7 +116,7 @@ func testPollingProcessorRecoverableError(t *testing.T, err error, verifyError f req.RequestAllRespCh <- mocks.RequestAllResponse{Err: err} withMockDataSourceUpdates(func(dataSourceUpdates *mocks.MockDataSourceUpdates) { - p := newPollingProcessor(sharedtest.BasicClientContext(), dataSourceUpdates, req, time.Millisecond*10) + p := newPollingProcessor(sharedtest.BasicClientContext(), dataSourceUpdates, req, time.Millisecond*10, 0) defer p.Close() closeWhenReady := make(chan struct{}) p.Start(closeWhenReady) @@ -141,10 +141,15 @@ func testPollingProcessorRecoverableError(t *testing.T, err error, verifyError f }) } -func TestPollingProcessorUnrecoverableErrors(t *testing.T) { +// Under the RETRY spec (SDK-2775), previously-terminal 4xx errors (401, 403, 404, +// 405) engage an extended-regime backoff but keep polling indefinitely. The +// processor transitions to Interrupted (not Off) and continues to poll. Replaces +// the pre-RETRY TestPollingProcessorUnrecoverableErrors, which asserted the old +// permanent-stop behavior. +func TestPollingProcessorUnexpectedErrorsEngageExtendedRegimeAndKeepRetrying(t *testing.T) { for _, statusCode := range []int{401, 403, 404, 405} { t.Run(fmt.Sprintf("HTTP %d", statusCode), func(t *testing.T) { - testPollingProcessorUnrecoverableError( + testPollingProcessorUnexpectedError( t, httpStatusError{Code: statusCode}, func(errorInfo interfaces.DataSourceErrorInfo) { @@ -156,7 +161,7 @@ func TestPollingProcessorUnrecoverableErrors(t *testing.T) { } } -func testPollingProcessorUnrecoverableError( +func testPollingProcessorUnexpectedError( t *testing.T, err error, verifyError func(interfaces.DataSourceErrorInfo), @@ -164,23 +169,93 @@ func testPollingProcessorUnrecoverableError( req := mocks.NewPollingRequester() defer req.Close() - req.RequestAllRespCh <- mocks.RequestAllResponse{Err: err} - req.RequestAllRespCh <- mocks.RequestAllResponse{} // we shouldn't get a second request, but just in case + // Feed several consecutive failures so we can observe multiple retry attempts. + for i := 0; i < 5; i++ { + req.RequestAllRespCh <- mocks.RequestAllResponse{Err: err} + } withMockDataSourceUpdates(func(dataSourceUpdates *mocks.MockDataSourceUpdates) { - p := newPollingProcessor(sharedtest.BasicClientContext(), dataSourceUpdates, req, time.Millisecond*10) + // Both PollInterval and ExtendedInitialPollInterval are dialed down so we don't + // wait the extended-regime 5-minute default between the first failure and + // observing the second attempt. The extended regime engages internally; here + // its wait floor is dominated by these two knobs. + p := newPollingProcessor( + sharedtest.BasicClientContext(), dataSourceUpdates, req, + 10*time.Millisecond, // PollInterval + 20*time.Millisecond, // ExtendedInitialPollInterval + ) defer p.Close() closeWhenReady := make(chan struct{}) p.Start(closeWhenReady) - // wait for first poll + // Wait for the first poll to fire. <-req.PollsCh - waitForReadyWithTimeout(t, closeWhenReady, time.Second) + // Initialization must not complete: no permanent stop, no successful poll. + select { + case <-closeWhenReady: + t.Fatal("closeWhenReady should not be closed — RETRY §1.2.1 forbids permanent stops on 4xx") + case <-time.After(500 * time.Millisecond): + } - status := dataSourceUpdates.RequireStatusOf(t, interfaces.DataSourceStateOff) + // Status reports the failure as Interrupted, not Off. + status := dataSourceUpdates.RequireStatusOf(t, interfaces.DataSourceStateInterrupted) verifyError(status.LastError) - assert.Len(t, req.PollsCh, 0) + + // Confirm the polling processor kept polling — at least one additional poll + // attempt landed at the mock requester. + select { + case <-req.PollsCh: + case <-time.After(500 * time.Millisecond): + t.Fatal("expected polling processor to retry after unexpected-classified error") + } + }) +} + +// After two consecutive successful polls, the processor must reset to the normal +// regime: attempts=0, and subsequent waits equal PollInterval (not the extended +// initial delay). This exercises the RETRY §1.8 polling binding, where the reset +// condition is a fixed count of successful polls rather than a time threshold. +func TestPollingResetsToNormalAfterTwoConsecutiveSuccesses(t *testing.T) { + req := mocks.NewPollingRequester() + defer req.Close() + + // Sequence: fail (engages extended), succeed, succeed (triggers reset), then + // stay in normal regime. Followed by many placeholder successes so the loop + // doesn't block. + req.RequestAllRespCh <- mocks.RequestAllResponse{Err: httpStatusError{Code: 401}} + for i := 0; i < 10; i++ { + req.RequestAllRespCh <- mocks.RequestAllResponse{} + } + + withMockDataSourceUpdates(func(dataSourceUpdates *mocks.MockDataSourceUpdates) { + p := newPollingProcessor( + sharedtest.BasicClientContext(), dataSourceUpdates, req, + 10*time.Millisecond, // PollInterval + 20*time.Millisecond, // ExtendedInitialPollInterval + ) + defer p.Close() + closeWhenReady := make(chan struct{}) + p.Start(closeWhenReady) + + // Poll #1: failure. Extended regime engages internally. + <-req.PollsCh + // Poll #2: success. priorPollWasSuccessful flips true; regime not yet reset. + <-req.PollsCh + // Poll #3: second consecutive success — reset should fire (attempts=0, back to normal). + <-req.PollsCh + // Poll #4: normal regime. The wait between polls should be ~PollInterval (10ms), not + // the extended base. Bounding at 200ms keeps a big margin against goroutine + // scheduling variance while still catching a regression that would produce + // a many-second or many-minute wait. + startPoll4 := time.Now() + <-req.PollsCh + elapsed := time.Since(startPoll4) + assert.Less(t, elapsed, 200*time.Millisecond, + "after 2 consecutive successes the polling processor should be back at PollInterval cadence") + + // Init completes on the first success. + waitForReadyWithTimeout(t, closeWhenReady, time.Second) }) } diff --git a/internal/datasource/polling_strategy.go b/internal/datasource/polling_strategy.go new file mode 100644 index 00000000..ac99189b --- /dev/null +++ b/internal/datasource/polling_strategy.go @@ -0,0 +1,101 @@ +package datasource + +import ( + "math" + "math/rand" + "time" +) + +// extendedPollMaxDelay is the RETRY-spec extended-regime ceiling on the polling +// backoff. Effective ceiling is max(extendedPollMaxDelay, PollInterval); see +// pollingStrategy.OnFailure. +const extendedPollMaxDelay = 1 * time.Hour + +// pollingStrategy implements the RETRY §1.4 timing mechanics for the polling +// data source. It owns: +// +// - the attempts counter n (RETRY §1.4); +// - the current regime's (initialDelay, maxDelay), toggled by classification +// (RETRY §1.5–§1.7 via the caller's FailureClass); +// - the two-consecutive-success reset gate (RETRY §1.8 polling binding); +// - jitter (RETRY §1.4.3); +// - the PollInterval wait floor (RETRY §1.4.4 polling override). +// +// All state is owned and mutated by the polling run() goroutine only — no +// locking required. +type pollingStrategy struct { + normalInterval time.Duration + extendedInitialPollInterval time.Duration + rng *rand.Rand + attempts int + priorPollWasSuccessful bool + initialDelay time.Duration + maxDelay time.Duration +} + +func newPollingStrategy(pollInterval, extendedInitialPollInterval time.Duration) *pollingStrategy { + return &pollingStrategy{ + normalInterval: pollInterval, + extendedInitialPollInterval: extendedInitialPollInterval, + //nolint:gosec // not a cryptographic use-case, weak RNG is acceptable for jitter + rng: rand.New(rand.NewSource(time.Now().UnixNano())), + initialDelay: pollInterval, + maxDelay: pollInterval, + } +} + +// OnFailure updates the strategy state after a failed poll. An Unexpected +// classification engages the extended regime for subsequent waits; +// initialDelay is clamped to at least PollInterval so the extended regime +// is never faster than the customer-configured normal cadence. +func (s *pollingStrategy) OnFailure(class FailureClass) { + s.priorPollWasSuccessful = false + s.attempts++ + if class == FailureClassUnexpected { + s.initialDelay = s.extendedInitialPollInterval + if s.initialDelay < s.normalInterval { + s.initialDelay = s.normalInterval + } + s.maxDelay = extendedPollMaxDelay + if s.maxDelay < s.normalInterval { + s.maxDelay = s.normalInterval + } + } +} + +// OnSuccess updates the strategy state after a successful poll. Two consecutive +// successes reset attempts and return the SDK to the normal regime (RETRY §1.8 +// polling binding). A single success is a necessary precondition but not +// sufficient — any failure between the first and second success clears it. +func (s *pollingStrategy) OnSuccess() { + if s.priorPollWasSuccessful { + s.attempts = 0 + s.initialDelay = s.normalInterval + s.maxDelay = s.normalInterval + } + s.priorPollWasSuccessful = true +} + +// NextWait returns the delay before the next poll attempt per RETRY §1.4. +// Formula: T = initialDelay * 2^(attempts-1), clamped to maxDelay. +// Jitter J is a uniform random in [0, T/2]. The final wait = max(PollInterval, +// T − J) ensures the interval never drops below the caller's configured +// PollInterval. +func (s *pollingStrategy) NextWait() time.Duration { + if s.attempts <= 0 { + return s.normalInterval + } + t := time.Duration(math.Min( + float64(s.initialDelay)*math.Pow(2, float64(s.attempts-1)), + float64(s.maxDelay), + )) + var jitter time.Duration + if halfT := int64(t / 2); halfT > 0 { + jitter = time.Duration(s.rng.Int63n(halfT)) + } + wait := t - jitter + if wait < s.normalInterval { + wait = s.normalInterval + } + return wait +} diff --git a/internal/datasource/polling_strategy_test.go b/internal/datasource/polling_strategy_test.go new file mode 100644 index 00000000..60fce21f --- /dev/null +++ b/internal/datasource/polling_strategy_test.go @@ -0,0 +1,143 @@ +package datasource + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +// In the normal regime (no failures observed), NextWait returns exactly PollInterval. +func TestPollingStrategy_NormalRegimeReturnsPollInterval(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + assert.Equal(t, 30*time.Second, s.NextWait()) +} + +// A normal-classified failure advances the attempts counter but does not engage +// the extended regime. Wait is still pinned to PollInterval by the wait floor. +func TestPollingStrategy_NormalFailureDoesNotEngageExtended(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + s.OnFailure(FailureClassNormal) + + assert.Equal(t, 1, s.attempts) + assert.Equal(t, 30*time.Second, s.initialDelay) + assert.Equal(t, 30*time.Second, s.maxDelay) + assert.Equal(t, 30*time.Second, s.NextWait()) +} + +// An unexpected-classified failure engages the extended regime: initialDelay +// becomes the configured extended base and maxDelay becomes the RETRY-spec cap. +func TestPollingStrategy_UnexpectedFailureEngagesExtended(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + s.OnFailure(FailureClassUnexpected) + + assert.Equal(t, 5*time.Minute, s.initialDelay) + assert.Equal(t, time.Hour, s.maxDelay) +} + +// Polling spec: initialDelay = max(configured, PollInterval). When PollInterval +// is larger than the configured extended base (mobile-background case), the +// extended regime uses PollInterval as its base. Result: no observable +// differentiation between regimes. +func TestPollingStrategy_ExtendedInitialClampedToPollInterval(t *testing.T) { + s := newPollingStrategy(time.Hour, 5*time.Minute) + s.OnFailure(FailureClassUnexpected) + + assert.Equal(t, time.Hour, s.initialDelay) + assert.Equal(t, time.Hour, s.maxDelay) + // All subsequent waits collapse to PollInterval. + assert.Equal(t, time.Hour, s.NextWait()) +} + +// Extended regime doubles per attempt per RETRY §1.4, capped at +// extendedPollMaxDelay (1 hour). Jitter subtracts up to T/2, so each wait falls +// in [T/2, T] before the (in these cases irrelevant) wait floor. +func TestPollingStrategy_ExtendedDoubling(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + + tests := []struct { + n int + lowerBound, upperBound time.Duration + }{ + {1, 2*time.Minute + 30*time.Second, 5 * time.Minute}, // T = 5m + {2, 5 * time.Minute, 10 * time.Minute}, // T = 10m + {3, 10 * time.Minute, 20 * time.Minute}, // T = 20m + {4, 20 * time.Minute, 40 * time.Minute}, // T = 40m + {5, 30 * time.Minute, time.Hour}, // T capped at 60m + {6, 30 * time.Minute, time.Hour}, // still capped + } + for _, tc := range tests { + s.OnFailure(FailureClassUnexpected) + w := s.NextWait() + assert.GreaterOrEqual(t, w, tc.lowerBound, "n=%d", tc.n) + assert.LessOrEqual(t, w, tc.upperBound, "n=%d", tc.n) + } +} + +// RETRY §1.4.4 polling B1 override: NextWait never returns less than PollInterval, +// even when the exponential math (or jitter) would drive it below. +func TestPollingStrategy_WaitFloorAtPollInterval(t *testing.T) { + // Extended base is much smaller than PollInterval. Every wait should be + // clamped up to PollInterval until doubling grows T past it. + s := newPollingStrategy(30*time.Second, 1*time.Millisecond) + for n := 1; n <= 10; n++ { + s.OnFailure(FailureClassUnexpected) + assert.GreaterOrEqual(t, s.NextWait(), 30*time.Second, "n=%d", n) + } +} + +// The 2-consecutive-success reset gate: first success flips the gate flag but +// does not clear attempts or exit the extended regime. +func TestPollingStrategy_FirstSuccessDoesNotReset(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + s.OnFailure(FailureClassUnexpected) + s.OnSuccess() + + assert.True(t, s.priorPollWasSuccessful, "reset gate should be armed") + assert.Equal(t, 1, s.attempts, "one success must not reset attempts") + assert.Equal(t, 5*time.Minute, s.initialDelay, "one success must not exit extended regime") +} + +// Two consecutive successes clear attempts and return the strategy to the +// normal regime (RETRY §1.8 polling binding). +func TestPollingStrategy_TwoConsecutiveSuccessesReset(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + s.OnFailure(FailureClassUnexpected) + s.OnSuccess() + s.OnSuccess() + + assert.Equal(t, 0, s.attempts) + assert.Equal(t, 30*time.Second, s.initialDelay) + assert.Equal(t, 30*time.Second, s.maxDelay) + assert.Equal(t, 30*time.Second, s.NextWait()) +} + +// A failure between two successes clears the reset gate — reset requires +// STRICTLY consecutive successes, so a first-then-fail-then-first pattern does +// not fire the reset. +func TestPollingStrategy_FailureClearsResetGate(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + s.OnFailure(FailureClassUnexpected) + s.OnSuccess() // gate armed + s.OnFailure(FailureClassNormal) // gate cleared, attempts=2 + s.OnSuccess() // gate armed again but does not fire reset + + assert.True(t, s.priorPollWasSuccessful) + assert.Equal(t, 2, s.attempts, "reset must not have fired") +} + +// A single normal failure after a reset does not re-engage extended-regime +// parameters — extended engagement requires an Unexpected classification. +func TestPollingStrategy_NormalFailureAfterResetStaysNormal(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + // Engage extended, then reset back to normal. + s.OnFailure(FailureClassUnexpected) + s.OnSuccess() + s.OnSuccess() + // A subsequent normal failure must not re-engage extended. + s.OnFailure(FailureClassNormal) + + assert.Equal(t, 1, s.attempts) + assert.Equal(t, 30*time.Second, s.initialDelay, "normal failure must not engage extended regime") + assert.Equal(t, 30*time.Second, s.maxDelay) +} diff --git a/internal/datasource/streaming_data_source.go b/internal/datasource/streaming_data_source.go index 0068465d..56352736 100644 --- a/internal/datasource/streaming_data_source.go +++ b/internal/datasource/streaming_data_source.go @@ -44,14 +44,17 @@ import ( // if we succeed then the client can detect that we're initialized now by calling our Initialized method. const ( - putEvent = "put" - patchEvent = "patch" - deleteEvent = "delete" - streamReadTimeout = 5 * time.Minute // the LaunchDarkly stream should send a heartbeat comment every 3 minutes - streamMaxRetryDelay = 30 * time.Second - streamRetryResetInterval = 60 * time.Second - streamJitterRatio = 0.5 - defaultStreamRetryDelay = 1 * time.Second + putEvent = "put" + patchEvent = "patch" + deleteEvent = "delete" + // The LaunchDarkly stream should send a heartbeat comment every 3 minutes. + streamReadTimeout = 5 * time.Minute + streamJitterRatio = 0.5 + streamRetryResetInterval = 60 * time.Second + defaultStreamRetryDelay = 1 * time.Second + streamMaxRetryDelay = 30 * time.Second + defaultStreamExtendedRetryDelay = 5 * time.Minute + streamExtendedMaxRetryDelay = 1 * time.Hour streamingErrorContext = "in stream connection" streamingWillRetryMessage = "will retry" @@ -60,9 +63,11 @@ const ( // StreamConfig describes the configuration for a streaming data source. It is exported so that // it can be used in the StreamingDataSourceBuilder. type StreamConfig struct { - URI string - FilterKey string - InitialReconnectDelay time.Duration + URI string + FilterKey string + InitialReconnectDelay time.Duration + ExtendedInitialReconnectDelay time.Duration + RetryResetInterval time.Duration } // StreamProcessor is the internal implementation of the streaming data source. @@ -298,56 +303,80 @@ func (sp *StreamProcessor) subscribe(closeWhenReady chan<- struct{}) { if initialRetryDelay <= 0 { // COVERAGE: can't cause this condition in unit tests initialRetryDelay = defaultStreamRetryDelay } + extendedInitialDelay := sp.cfg.ExtendedInitialReconnectDelay + if extendedInitialDelay <= 0 { + extendedInitialDelay = defaultStreamExtendedRetryDelay + } + retryResetInterval := sp.cfg.RetryResetInterval + if retryResetInterval <= 0 { + retryResetInterval = streamRetryResetInterval + } + + defaultCurve := es.NewRetryCurve( + es.RetryCurveBaseDelay(initialRetryDelay), + es.RetryCurveMaxDelay(streamMaxRetryDelay), + es.RetryCurveJitter(streamJitterRatio), + ) + + extendedCurve := es.NewRetryCurve( + es.RetryCurveBaseDelay(extendedInitialDelay), + es.RetryCurveMaxDelay(streamExtendedMaxRetryDelay), + es.RetryCurveJitter(streamJitterRatio), + ) errorHandler := func(err error) es.StreamErrorHandlerResult { sp.logConnectionResult(false) + var class FailureClass + var errorInfo interfaces.DataSourceErrorInfo + if se, ok := err.(es.SubscriptionError); ok { - errorInfo := interfaces.DataSourceErrorInfo{ + errorInfo = interfaces.DataSourceErrorInfo{ Kind: interfaces.DataSourceErrorKindErrorResponse, StatusCode: se.Code, Time: time.Now(), } - recoverable := checkIfErrorIsRecoverableAndLog( + class = classifyAndLogHTTPFailure( sp.loggers, httpErrorDescription(se.Code), streamingErrorContext, se.Code, streamingWillRetryMessage, ) - if recoverable { - sp.logConnectionStarted() - sp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateInterrupted, errorInfo) - return es.StreamErrorHandlerResult{CloseNow: false} + } else { + errorInfo = interfaces.DataSourceErrorInfo{ + Kind: interfaces.DataSourceErrorKindNetworkError, + Message: err.Error(), + Time: time.Now(), } - sp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateOff, errorInfo) - return es.StreamErrorHandlerResult{CloseNow: true} + class = classifyAndLogTransportFailure( + sp.loggers, + err, + streamingErrorContext, + streamingWillRetryMessage, + ) } - checkIfErrorIsRecoverableAndLog( - sp.loggers, - err.Error(), - streamingErrorContext, - 0, - streamingWillRetryMessage, - ) - errorInfo := interfaces.DataSourceErrorInfo{ - Kind: interfaces.DataSourceErrorKindNetworkError, - Message: err.Error(), - Time: time.Now(), - } sp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateInterrupted, errorInfo) sp.logConnectionStarted() - return es.StreamErrorHandlerResult{CloseNow: false} + + // Per RETRY §1.2.1: no failure is permanently terminal. Unexpected failures + // engage the extended-regime curve; the library keeps retrying at extended + // cadence until a healthy-op reset (retryResetInterval of continuous + // connection) reverts. + result := es.StreamErrorHandlerResult{CloseNow: false} + if class == FailureClassUnexpected { + result.ActivateCurve = extendedCurve + } + return result } stream, err := es.SubscribeWithRequestAndOptions(req, es.StreamOptionHTTPClient(sp.client), es.StreamOptionReadTimeout(streamReadTimeout), - es.StreamOptionInitialRetry(initialRetryDelay), - es.StreamOptionUseBackoff(streamMaxRetryDelay), - es.StreamOptionUseJitter(streamJitterRatio), - es.StreamOptionRetryResetInterval(streamRetryResetInterval), + es.StreamOptionDefaultRetryCurve(defaultCurve), + es.StreamOptionRegisterRetryCurve(extendedCurve), + es.StreamOptionRetryResetInterval(retryResetInterval), es.StreamOptionErrorHandler(errorHandler), es.StreamOptionCanRetryFirstConnection(-1), es.StreamOptionLogger(sp.loggers.ForLevel(ldlog.Info)), diff --git a/internal/datasource/streaming_data_source_test.go b/internal/datasource/streaming_data_source_test.go index 4277e996..b7f09adc 100644 --- a/internal/datasource/streaming_data_source_test.go +++ b/internal/datasource/streaming_data_source_test.go @@ -26,6 +26,7 @@ import ( "github.com/launchdarkly/go-test-helpers/v3/httphelpers" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -248,10 +249,15 @@ func TestStreamProcessorRecoverableErrorsCauseStreamRestart(t *testing.T) { }) } -func TestStreamProcessorUnrecoverableErrorsCauseStreamShutdown(t *testing.T) { +// Under the RETRY spec (SDK-2775), 401 / 403 / other 4xx are no longer terminal — +// they engage an extended-regime backoff but keep retrying indefinitely. The SDK +// transitions to Interrupted (not Off) and does not close the initialization +// channel. This test replaces the pre-RETRY TestStreamProcessorUnrecoverableErrors +// CauseStreamShutdown, which asserted the old (permanent-stop) behavior. +func TestStreamProcessorUnexpectedErrorsEngageExtendedRegimeAndKeepRetrying(t *testing.T) { for _, status := range []int{401, 403, 404} { t.Run(fmt.Sprintf("HTTP status %d", status), func(t *testing.T) { - testStreamProcessorUnrecoverableHTTPError(t, status) + testStreamProcessorUnexpectedHTTPError(t, status) }) } } @@ -378,10 +384,12 @@ func TestStreamProcessorStoreUpdateFailureWithoutStatusTracking(t *testing.T) { } -func testStreamProcessorUnrecoverableHTTPError(t *testing.T, statusCode int) { +func testStreamProcessorUnexpectedHTTPError(t *testing.T, statusCode int) { mockLog := ldlogtest.NewMockLog() defer mockLog.DumpIfTestFailed(t) - httphelpers.WithServer(httphelpers.HandlerWithStatus(statusCode), func(ts *httptest.Server) { + // Record the request stream so we can verify the SDK keeps retrying after the failure. + handler, requestsCh := httphelpers.RecordingHandler(httphelpers.HandlerWithStatus(statusCode)) + httphelpers.WithServer(handler, func(ts *httptest.Server) { withMockDataSourceUpdates(func(dataSourceUpdates *mocks.MockDataSourceUpdates) { id := ldevents.NewDiagnosticID(sharedtest.TestSDKKey) diagnosticsManager := ldevents.NewDiagnosticsManager(id, ldvalue.Null(), ldvalue.Null(), time.Now(), nil) @@ -393,22 +401,45 @@ func testStreamProcessorUnrecoverableHTTPError(t *testing.T, statusCode int) { DiagnosticsManager: diagnosticsManager, } - sp := NewStreamProcessor(context, dataSourceUpdates, StreamConfig{URI: ts.URL, InitialReconnectDelay: time.Second}) + // Short retry delays so we can observe at least two attempts within the + // assertion window. The extended-regime curve activates immediately on the + // first failure (per the RETRY spec — no grace period for initial-connection + // unexpected classifications), so we need to shorten ExtendedInitialReconnectDelay + // too, not just the normal InitialReconnectDelay. + sp := NewStreamProcessor(context, dataSourceUpdates, StreamConfig{ + URI: ts.URL, + InitialReconnectDelay: 10 * time.Millisecond, + ExtendedInitialReconnectDelay: 20 * time.Millisecond, + }) defer sp.Close() closeWhenReady := make(chan struct{}) - sp.Start(closeWhenReady) - th.AssertChannelClosed(t, closeWhenReady, time.Second*3, "Initialization shouldn't block after this error") - - event := diagnosticsManager.CreateStatsEventAndReset(0, 0, 0) - assert.Equal(t, 1, event.GetByKey("streamInits").Count()) - assert.Equal(t, ldvalue.Bool(true), event.GetByKey("streamInits").GetByIndex(0).GetByKey("failed")) + // Initialization must not complete: no permanent stop, no successful put. + // The client's Start() would time out via StartWaitTimeMS in production; here + // we just assert the channel stays open. + select { + case <-closeWhenReady: + t.Fatal("closeWhenReady should not be closed — RETRY §1.2.1 forbids permanent stops on 4xx") + case <-time.After(time.Second): + } - status := dataSourceUpdates.RequireStatusOf(t, interfaces.DataSourceStateOff) + // The mock data source updates records the raw UpdateStatus calls made + // by the processor — so we see the Interrupted call the processor tried + // to make. (The real DataSourceUpdateSinkImpl would clamp it to + // Initializing since we never reached Valid; that's tested at the + // LDClient-level in the RETRY end-to-end tests. Here we just verify the + // processor emitted the correct call.) + status := dataSourceUpdates.RequireStatusOf(t, interfaces.DataSourceStateInterrupted) assert.Equal(t, interfaces.DataSourceErrorKindErrorResponse, status.LastError.Kind) assert.Equal(t, statusCode, status.LastError.StatusCode) + + // Confirm the SDK actually retried (at least a second request landed at the mock server). + // The initial-regime retry delay is 10ms above, so 500ms is plenty of budget. + require.Eventually(t, func() bool { return len(requestsCh) >= 2 }, + 500*time.Millisecond, 10*time.Millisecond, + "expected the SDK to retry after the unexpected-classified error") }) }) } diff --git a/ldclient_end_to_end_test.go b/ldclient_end_to_end_test.go index 1ab39e4a..77fd0656 100644 --- a/ldclient_end_to_end_test.go +++ b/ldclient_end_to_end_test.go @@ -17,6 +17,7 @@ import ( "github.com/launchdarkly/go-server-sdk/v7/interfaces" "github.com/launchdarkly/go-server-sdk/v7/internal/sharedtest" "github.com/launchdarkly/go-server-sdk/v7/ldcomponents" + "github.com/launchdarkly/go-server-sdk/v7/testhelpers/datasourcetest" "github.com/launchdarkly/go-server-sdk/v7/testhelpers/ldservices" "github.com/launchdarkly/go-test-helpers/v3/httphelpers" @@ -102,36 +103,62 @@ func TestClientStartsInStreamingMode(t *testing.T) { }) } -func TestClientFailsToStartInStreamingModeWith401Error(t *testing.T) { +// Under the RETRY spec (SDK-2775), a 401 is no longer terminal: the client +// times out waiting for initial data but the data source keeps retrying +// indefinitely. Init returns the usual timeout error; the data source state is +// Interrupted (not Off); the stream keeps hitting the server. Replaces the +// pre-RETRY TestClientFailsToStartInStreamingModeWith401Error, which asserted +// the old permanent-stop behavior. +func TestClientInStreamingModeWith401KeepsRetrying(t *testing.T) { handler, requestsCh := httphelpers.RecordingHandler(httphelpers.HandlerWithStatus(401)) httphelpers.WithServer(handler, func(streamServer *httptest.Server) { logCapture := ldlogtest.NewMockLog() + // Short reconnect delays so multiple attempts fit within the init wait window. + // Extended-regime curve activates immediately on 401 (unexpected), so we need + // to shorten its base too, not just the normal-regime InitialReconnectDelay. + streamingBuilder := ldcomponents.StreamingDataSource().InitialReconnectDelay(10 * time.Millisecond) + datasourcetest.WithStreamingExtendedInitialReconnectDelay(streamingBuilder, 20*time.Millisecond) + config := Config{ Events: ldcomponents.NoEvents(), Logging: ldcomponents.Logging().Loggers(logCapture.Loggers), ServiceEndpoints: interfaces.ServiceEndpoints{Streaming: streamServer.URL}, + DataSource: streamingBuilder, } - client, err := MakeCustomClient(testSdkKey, config, time.Second*5) - require.Error(t, err) + client, err := MakeCustomClient(testSdkKey, config, 500*time.Millisecond) + require.Error(t, err) // init timed out — no permanent stop, no successful put either require.NotNil(t, client) defer client.Close() - assert.Equal(t, initializationFailedErrorMessage, err.Error()) - - assert.Equal(t, string(interfaces.DataSourceStateOff), string(client.GetDataSourceStatusProvider().GetStatus().State)) - + assert.Equal(t, ErrInitializationTimeout, err) + + // Under RETRY the SDK does not permanently stop on 401. Since we never + // reached a Valid state, the SinkImpl keeps state as Initializing rather + // than transitioning to Interrupted (see maybeUpdateStatus's Initializing + // clamp), but LastError records the failure. The key assertion is + // "state is NOT Off" — no permanent stop. + require.Eventually(t, func() bool { + s := client.GetDataSourceStatusProvider().GetStatus() + return s.LastError.Kind == interfaces.DataSourceErrorKindErrorResponse && + s.LastError.StatusCode == 401 + }, 2*time.Second, 10*time.Millisecond, + "data source should record the 401 as LastError") + assert.NotEqual(t, string(interfaces.DataSourceStateOff), + string(client.GetDataSourceStatusProvider().GetStatus().State), + "RETRY §1.2.1: no permanent stop on 401") + + // Flag evaluation still works and returns defaults. value, _ := client.BoolVariation(alwaysTrueFlag.Key, testUser, false) assert.False(t, value) + // Confirm the client kept retrying — at least one additional request landed at the mock server. r := <-requestsCh assert.Equal(t, testSdkKey, r.Request.Header.Get("Authorization")) - assertNoMoreRequests(t, requestsCh) - - expectedError := "Error in stream connection (giving up permanently): HTTP error 401 (invalid SDK key)" - assert.Equal(t, []string{expectedError}, logCapture.GetOutput(ldlog.Error)) - assert.Equal(t, []string{initializationFailedErrorMessage}, logCapture.GetOutput(ldlog.Warn)) + require.Eventually(t, func() bool { return len(requestsCh) >= 1 }, + 500*time.Millisecond, 10*time.Millisecond, + "expected the client to keep retrying after 401") }) } @@ -258,37 +285,55 @@ func TestInstanceIDIsDifferentBetweenClients(t *testing.T) { }) } -func TestClientFailsToStartInPollingModeWith401Error(t *testing.T) { +// Under the RETRY spec (SDK-2775), a 401 is no longer terminal for polling +// either: the goroutine keeps polling on the extended-regime cadence. Init +// times out; state is Interrupted; the client kept polling. Replaces the +// pre-RETRY TestClientFailsToStartInPollingModeWith401Error. +func TestClientInPollingModeWith401KeepsRetrying(t *testing.T) { handler, requestsCh := httphelpers.RecordingHandler(httphelpers.HandlerWithStatus(401)) httphelpers.WithServer(handler, func(pollServer *httptest.Server) { logCapture := ldlogtest.NewMockLog() + pollingBuilder := ldcomponents.PollingDataSource() + datasourcetest.WithPollingExtendedInitialPollInterval(pollingBuilder, 20*time.Millisecond) + config := Config{ - DataSource: ldcomponents.PollingDataSource(), + DataSource: pollingBuilder, Events: ldcomponents.NoEvents(), Logging: ldcomponents.Logging().Loggers(logCapture.Loggers), ServiceEndpoints: interfaces.ServiceEndpoints{Polling: pollServer.URL}, } - client, err := MakeCustomClient(testSdkKey, config, time.Second*5) - require.Error(t, err) + client, err := MakeCustomClient(testSdkKey, config, 500*time.Millisecond) + require.Error(t, err) // init timed out require.NotNil(t, client) defer client.Close() - assert.Equal(t, initializationFailedErrorMessage, err.Error()) - - assert.Equal(t, string(interfaces.DataSourceStateOff), string(client.GetDataSourceStatusProvider().GetStatus().State)) + assert.Equal(t, ErrInitializationTimeout, err) + + // Under RETRY the SDK does not permanently stop on 401. Since we never + // reached a Valid state, the SinkImpl keeps state as Initializing rather + // than transitioning to Interrupted. The key assertion is "state is NOT + // Off" — no permanent stop — and LastError records the failure. + require.Eventually(t, func() bool { + s := client.GetDataSourceStatusProvider().GetStatus() + return s.LastError.Kind == interfaces.DataSourceErrorKindErrorResponse && + s.LastError.StatusCode == 401 + }, 2*time.Second, 10*time.Millisecond, + "data source should record the 401 as LastError") + assert.NotEqual(t, string(interfaces.DataSourceStateOff), + string(client.GetDataSourceStatusProvider().GetStatus().State), + "RETRY §1.2.1: no permanent stop on 401") value, _ := client.BoolVariation(alwaysTrueFlag.Key, testUser, false) assert.False(t, value) + // Confirm the polling goroutine hit the server. Not asserting a second + // poll here — the polling B1 wait floor is PollInterval (30s default), + // so observing multiple polls at unit-test timescales requires the + // internal-constructor pathway used by polling_data_source_test.go. r := <-requestsCh assert.Equal(t, testSdkKey, r.Request.Header.Get("Authorization")) - assertNoMoreRequests(t, requestsCh) - - expectedError := "Error on polling request (giving up permanently): HTTP error 401 (invalid SDK key)" - assert.Equal(t, []string{expectedError}, logCapture.GetOutput(ldlog.Error)) - assert.Equal(t, []string{pollingModeWarningMessage, initializationFailedErrorMessage}, logCapture.GetOutput(ldlog.Warn)) }) } diff --git a/ldcomponents/polling_data_source_builder.go b/ldcomponents/polling_data_source_builder.go index 72f4cf3e..775fe8fe 100644 --- a/ldcomponents/polling_data_source_builder.go +++ b/ldcomponents/polling_data_source_builder.go @@ -16,12 +16,17 @@ const DefaultPollingBaseURI = "https://app.launchdarkly.com" // DefaultPollInterval is the default value for [PollingDataSourceBuilder.PollInterval]. This is also the minimum value. const DefaultPollInterval = 30 * time.Second +// DefaultExtendedInitialPollInterval is the default value for +// [PollingDataSourceBuilder.ExtendedInitialPollInterval]. +const DefaultExtendedInitialPollInterval = 5 * time.Minute + // PollingDataSourceBuilder provides methods for configuring the polling data source. // // See [PollingDataSource] for usage. type PollingDataSourceBuilder struct { - pollInterval time.Duration - filterKey ldvalue.OptionalString + pollInterval time.Duration + extendedInitialPollInterval time.Duration + filterKey ldvalue.OptionalString } // PollingDataSource returns a configurable factory for using polling mode to get feature flag data. @@ -40,7 +45,8 @@ type PollingDataSourceBuilder struct { // } func PollingDataSource() *PollingDataSourceBuilder { return &PollingDataSourceBuilder{ - pollInterval: DefaultPollInterval, + pollInterval: DefaultPollInterval, + extendedInitialPollInterval: DefaultExtendedInitialPollInterval, } } @@ -66,6 +72,32 @@ func (b *PollingDataSourceBuilder) forcePollInterval( return b } +// PollingDataSourceBuilderInternal is an internal test-only accessor for a +// PollingDataSourceBuilder. It exposes knobs that are not part of the SDK's +// stable public API and must not be used in production code. +type PollingDataSourceBuilderInternal struct{ builder *PollingDataSourceBuilder } + +// Internal returns a test-only accessor for setting fields not exposed on the +// public builder surface. This is not part of the SDK's stable public API and +// must not be used in production code. +func (b *PollingDataSourceBuilder) Internal() PollingDataSourceBuilderInternal { + return PollingDataSourceBuilderInternal{builder: b} +} + +// ExtendedInitialPollInterval sets the base delay for the extended-regime backoff +// that engages after RETRY-classified unexpected failures (401, 403, TLS/cert). +// Values ≤ 0 are clamped to [DefaultExtendedInitialPollInterval]. +func (i PollingDataSourceBuilderInternal) ExtendedInitialPollInterval( + delay time.Duration, +) PollingDataSourceBuilderInternal { + if delay <= 0 { + i.builder.extendedInitialPollInterval = DefaultExtendedInitialPollInterval + } else { + i.builder.extendedInitialPollInterval = delay + } + return i +} + // PayloadFilter sets the filter key for the polling connection. // // By default, the SDK is able to evaluate all flags in an environment. If this is undesirable - @@ -92,9 +124,10 @@ func (b *PollingDataSourceBuilder) Build(context subsystems.ClientContext) (subs context.GetLogging().Loggers, ) cfg := datasource.PollingConfig{ - BaseURI: configuredBaseURI, - PollInterval: b.pollInterval, - FilterKey: filterKey, + BaseURI: configuredBaseURI, + PollInterval: b.pollInterval, + ExtendedInitialPollInterval: b.extendedInitialPollInterval, + FilterKey: filterKey, } pp := datasource.NewPollingProcessor(context, context.GetDataSourceUpdateSink(), cfg) return pp, nil diff --git a/ldcomponents/streaming_data_source_builder.go b/ldcomponents/streaming_data_source_builder.go index ade67b04..3ab6e385 100644 --- a/ldcomponents/streaming_data_source_builder.go +++ b/ldcomponents/streaming_data_source_builder.go @@ -16,12 +16,22 @@ const DefaultStreamingBaseURI = endpoints.DefaultStreamingBaseURI // DefaultInitialReconnectDelay is the default value for [StreamingDataSourceBuilder.InitialReconnectDelay]. const DefaultInitialReconnectDelay = time.Second +// DefaultExtendedInitialReconnectDelay is the default value for +// [StreamingDataSourceBuilder.ExtendedInitialReconnectDelay]. +const DefaultExtendedInitialReconnectDelay = 5 * time.Minute + +// DefaultRetryResetInterval is the default value for +// [StreamingDataSourceBuilder.RetryResetInterval]. +const DefaultRetryResetInterval = 60 * time.Second + // StreamingDataSourceBuilder provides methods for configuring the streaming data source. // // See StreamingDataSource for usage. type StreamingDataSourceBuilder struct { - initialReconnectDelay time.Duration - filterKey ldvalue.OptionalString + initialReconnectDelay time.Duration + extendedInitialReconnectDelay time.Duration + retryResetInterval time.Duration + filterKey ldvalue.OptionalString } // StreamingDataSource returns a configurable factory for using streaming mode to get feature flag data. @@ -36,7 +46,9 @@ type StreamingDataSourceBuilder struct { // } func StreamingDataSource() *StreamingDataSourceBuilder { return &StreamingDataSourceBuilder{ - initialReconnectDelay: DefaultInitialReconnectDelay, + initialReconnectDelay: DefaultInitialReconnectDelay, + extendedInitialReconnectDelay: DefaultExtendedInitialReconnectDelay, + retryResetInterval: DefaultRetryResetInterval, } } @@ -58,6 +70,47 @@ func (b *StreamingDataSourceBuilder) InitialReconnectDelay( return b } +// StreamingDataSourceBuilderInternal is an internal test-only accessor for a +// StreamingDataSourceBuilder. It exposes knobs that are not part of the SDK's +// stable public API and must not be used in production code — the LaunchDarkly +// RETRY conformance test suite is the only intended caller. +type StreamingDataSourceBuilderInternal struct{ builder *StreamingDataSourceBuilder } + +// Internal returns a test-only accessor for setting fields not exposed on the +// public builder surface. This is not part of the SDK's stable public API and +// must not be used in production code. +func (b *StreamingDataSourceBuilder) Internal() StreamingDataSourceBuilderInternal { + return StreamingDataSourceBuilderInternal{builder: b} +} + +// ExtendedInitialReconnectDelay sets the base delay for the extended-regime retry +// curve that engages after RETRY-classified unexpected failures (401, 403, TLS/cert). +// Values ≤ 0 are clamped to [DefaultExtendedInitialReconnectDelay]. +func (i StreamingDataSourceBuilderInternal) ExtendedInitialReconnectDelay( + delay time.Duration, +) StreamingDataSourceBuilderInternal { + if delay <= 0 { + i.builder.extendedInitialReconnectDelay = DefaultExtendedInitialReconnectDelay + } else { + i.builder.extendedInitialReconnectDelay = delay + } + return i +} + +// RetryResetInterval sets the threshold of continuous healthy stream operation +// before the SDK resets its retry backoff to the normal regime. +// Values ≤ 0 are clamped to [DefaultRetryResetInterval]. +func (i StreamingDataSourceBuilderInternal) RetryResetInterval( + interval time.Duration, +) StreamingDataSourceBuilderInternal { + if interval <= 0 { + i.builder.retryResetInterval = DefaultRetryResetInterval + } else { + i.builder.retryResetInterval = interval + } + return i +} + // PayloadFilter sets the payload filter key for this streaming connection. The filter key // cannot be an empty string. // @@ -83,9 +136,11 @@ func (b *StreamingDataSourceBuilder) Build(context subsystems.ClientContext) (su context.GetLogging().Loggers, ) cfg := datasource.StreamConfig{ - URI: configuredBaseURI, - InitialReconnectDelay: b.initialReconnectDelay, - FilterKey: filterKey, + URI: configuredBaseURI, + InitialReconnectDelay: b.initialReconnectDelay, + ExtendedInitialReconnectDelay: b.extendedInitialReconnectDelay, + RetryResetInterval: b.retryResetInterval, + FilterKey: filterKey, } return datasource.NewStreamProcessor( context, diff --git a/testhelpers/datasourcetest/datasourcetest.go b/testhelpers/datasourcetest/datasourcetest.go new file mode 100644 index 00000000..3b929fa7 --- /dev/null +++ b/testhelpers/datasourcetest/datasourcetest.go @@ -0,0 +1,50 @@ +// Package datasourcetest provides test-only helpers for configuring knobs on the +// SDK's streaming and polling data source builders that are not part of the +// SDK's stable public API. It is intended for LaunchDarkly's own contract-test +// tooling and for SDK integration tests that need to observe extended-regime +// behavior within a test-relevant time budget. +// +// Production code must not import this package. +// +// This package is a companion to the Internal() escape hatches on the builders +// in ldcomponents. Adding a new test-only knob to a builder means: +// 1. Add the field and the setter to the Internal type in ldcomponents. +// 2. Add a corresponding free-function helper here that delegates to it. +package datasourcetest + +import ( + "time" + + "github.com/launchdarkly/go-server-sdk/v7/ldcomponents" +) + +// WithStreamingExtendedInitialReconnectDelay overrides the RETRY-spec default +// base delay for the streaming extended-regime retry curve. Test-only. +func WithStreamingExtendedInitialReconnectDelay( + b *ldcomponents.StreamingDataSourceBuilder, + delay time.Duration, +) *ldcomponents.StreamingDataSourceBuilder { + b.Internal().ExtendedInitialReconnectDelay(delay) + return b +} + +// WithStreamingRetryResetInterval overrides the threshold of continuous healthy +// stream operation before the SDK resets its retry backoff to the normal regime. +// Test-only. +func WithStreamingRetryResetInterval( + b *ldcomponents.StreamingDataSourceBuilder, + interval time.Duration, +) *ldcomponents.StreamingDataSourceBuilder { + b.Internal().RetryResetInterval(interval) + return b +} + +// WithPollingExtendedInitialPollInterval overrides the RETRY-spec default base +// delay for the polling extended-regime backoff. Test-only. +func WithPollingExtendedInitialPollInterval( + b *ldcomponents.PollingDataSourceBuilder, + delay time.Duration, +) *ldcomponents.PollingDataSourceBuilder { + b.Internal().ExtendedInitialPollInterval(delay) + return b +} diff --git a/testservice/go.mod b/testservice/go.mod index 574bc5aa..7cf7ef24 100644 --- a/testservice/go.mod +++ b/testservice/go.mod @@ -1,6 +1,6 @@ module github.com/launchdarkly/go-server-sdk/v7/testservice -go 1.24.0 +go 1.25.0 require ( github.com/aws/aws-sdk-go-v2/config v1.17.5 @@ -56,7 +56,12 @@ require ( github.com/patrickmn/go-cache v2.1.0+incompatible // indirect github.com/redis/go-redis/v9 v9.0.2 // indirect golang.org/x/sync v0.8.0 // indirect - golang.org/x/sys v0.38.0 // indirect + golang.org/x/sys v0.45.0 // indirect ) replace github.com/launchdarkly/go-server-sdk/v7 => ../ + +// TODO(SDK-2788): temporary local replace so this branch can build against the +// unreleased RetryCurve API in eventsource PR #68. Remove when the eventsource +// PR merges and a release with the API is tagged (>=1.12.0 expected). +replace github.com/launchdarkly/eventsource => ../../eventsource diff --git a/testservice/go.sum b/testservice/go.sum index ce85f8b9..b3cb975c 100644 --- a/testservice/go.sum +++ b/testservice/go.sum @@ -104,8 +104,6 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/launchdarkly/ccache v1.1.0 h1:voD1M+ZJXR3MREOKtBwgTF9hYHl1jg+vFKS/+VAkR2k= github.com/launchdarkly/ccache v1.1.0/go.mod h1:TlxzrlnzvYeXiLHmesMuvoZetu4Z97cV1SsdqqBJi1Q= -github.com/launchdarkly/eventsource v1.10.0 h1:H9Tp6AfGu/G2qzBJC26iperrvwhzdbiA/gx7qE2nDFI= -github.com/launchdarkly/eventsource v1.10.0/go.mod h1:J3oa50bPvJesZqNAJtb5btSIo5N6roDWhiAS3IpsKck= github.com/launchdarkly/go-jsonstream/v3 v3.1.1 h1:ugupp2eNtwVbr69KCdeUrm1vUf1/3ju4Wdliaob95uY= github.com/launchdarkly/go-jsonstream/v3 v3.1.1/go.mod h1:ZBjhKq8mhArCtqotGRGnteY6eXpNm1GaOdUSZHh+ZjM= github.com/launchdarkly/go-sdk-common/v3 v3.5.0 h1:DsfTimg4BZO2hQPeeeEZYZVsP25pAWEdq/rKv/b7HJU= @@ -198,8 +196,8 @@ golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210303074136-134d130e1a04/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= -golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= diff --git a/testservice/sdk_client_entity.go b/testservice/sdk_client_entity.go index 4d59b516..b7761b6d 100644 --- a/testservice/sdk_client_entity.go +++ b/testservice/sdk_client_entity.go @@ -27,6 +27,7 @@ import ( "github.com/launchdarkly/go-server-sdk/v7/ldcomponents" "github.com/launchdarkly/go-server-sdk/v7/ldhooks" "github.com/launchdarkly/go-server-sdk/v7/subsystems" + "github.com/launchdarkly/go-server-sdk/v7/testhelpers/datasourcetest" "github.com/launchdarkly/go-server-sdk/v7/testservice/servicedef" "github.com/launchdarkly/go-sdk-common/v3/ldcontext" @@ -507,6 +508,14 @@ func makeSDKConfig(config servicedef.SDKConfigParams, sdkLog ldlog.Loggers) (ld. if config.Streaming.InitialRetryDelayMS != nil { builder.InitialReconnectDelay(time.Millisecond * time.Duration(*config.Streaming.InitialRetryDelayMS)) } + if config.Streaming.ExtendedInitialDelayMS != nil { + datasourcetest.WithStreamingExtendedInitialReconnectDelay(builder, + time.Millisecond*time.Duration(*config.Streaming.ExtendedInitialDelayMS)) + } + if config.Streaming.ResetThresholdMS != nil { + datasourcetest.WithStreamingRetryResetInterval(builder, + time.Millisecond*time.Duration(*config.Streaming.ResetThresholdMS)) + } if config.Streaming.Filter.IsDefined() { builder.PayloadFilter(config.Streaming.Filter.String()) } @@ -519,6 +528,10 @@ func makeSDKConfig(config servicedef.SDKConfigParams, sdkLog ldlog.Loggers) (ld. if config.Polling.PollIntervalMS != nil { builder.PollInterval(time.Millisecond * time.Duration(*config.Polling.PollIntervalMS)) } + if config.Polling.ExtendedInitialDelayMS != nil { + datasourcetest.WithPollingExtendedInitialPollInterval(builder, + time.Millisecond*time.Duration(*config.Polling.ExtendedInitialDelayMS)) + } if config.Polling.Filter.IsDefined() { builder.PayloadFilter(config.Polling.Filter.String()) } diff --git a/testservice/service.go b/testservice/service.go index 9f5135b6..1579bb2d 100644 --- a/testservice/service.go +++ b/testservice/service.go @@ -53,6 +53,8 @@ var capabilities = []string{ servicedef.CapabilityFlagValueChangeListeners, servicedef.CapabilityFDv1Fallback, servicedef.CapabilityInstanceID, + servicedef.CapabilityRetryConformanceFDv1Streaming, + servicedef.CapabilityRetryConformanceFDv1Polling, } // gets the specified environment variable, or the default if not set diff --git a/testservice/servicedef/sdk_config.go b/testservice/servicedef/sdk_config.go index d8d2716f..7eb4f5c1 100644 --- a/testservice/servicedef/sdk_config.go +++ b/testservice/servicedef/sdk_config.go @@ -62,15 +62,18 @@ type Synchronizer struct { } type SDKConfigStreamingParams struct { - BaseURI string `json:"baseUri,omitempty"` - InitialRetryDelayMS *ldtime.UnixMillisecondTime `json:"initialRetryDelayMs,omitempty"` - Filter ldvalue.OptionalString `json:"filter,omitempty"` + BaseURI string `json:"baseUri,omitempty"` + InitialRetryDelayMS *ldtime.UnixMillisecondTime `json:"initialRetryDelayMs,omitempty"` + ExtendedInitialDelayMS *ldtime.UnixMillisecondTime `json:"extendedInitialDelayMs,omitempty"` + ResetThresholdMS *ldtime.UnixMillisecondTime `json:"resetThresholdMs,omitempty"` + Filter ldvalue.OptionalString `json:"filter,omitempty"` } type SDKConfigPollingParams struct { - BaseURI string `json:"baseUri,omitempty"` - PollIntervalMS *ldtime.UnixMillisecondTime `json:"pollIntervalMs,omitempty"` - Filter ldvalue.OptionalString `json:"filter,omitempty"` + BaseURI string `json:"baseUri,omitempty"` + PollIntervalMS *ldtime.UnixMillisecondTime `json:"pollIntervalMs,omitempty"` + ExtendedInitialDelayMS *ldtime.UnixMillisecondTime `json:"extendedInitialDelayMs,omitempty"` + Filter ldvalue.OptionalString `json:"filter,omitempty"` } type SDKConfigEventParams struct { diff --git a/testservice/servicedef/service_params.go b/testservice/servicedef/service_params.go index 7bd60124..f1e95e6b 100644 --- a/testservice/servicedef/service_params.go +++ b/testservice/servicedef/service_params.go @@ -9,31 +9,33 @@ const ( CapabilityAllFlagsClientSideOnly = "all-flags-client-side-only" CapabilityAllFlagsDetailsOnlyForTrackedFlags = "all-flags-details-only-for-tracked-flags" - CapabilityBigSegments = "big-segments" - CapabilitySecureModeHash = "secure-mode-hash" - CapabilityServerSidePolling = "server-side-polling" - CapabilityServiceEndpoints = "service-endpoints" - CapabilityTags = "tags" - CapabilityFiltering = "filtering" - CapabilityContextType = "context-type" - CapabilityMigrations = "migrations" - CapabilityEventSampling = "event-sampling" - CapabilityInlineContextAll = "inline-context-all" - CapabilityAnonymousRedaction = "anonymous-redaction" - CapabilityEvaluationHooks = "evaluation-hooks" - CapabilityTrackHooks = "track-hooks" - CapabilityOmitAnonymousContexts = "omit-anonymous-contexts" - CapabilityEventGzip = "event-gzip" - CapabilityOptionalEventGzip = "optional-event-gzip" - CapabilityPollingGzip = "polling-gzip" - CapabilityClientPrereqEvents = "client-prereq-events" - CapabilityPersistentDataStoreRedis = "persistent-data-store-redis" - CapabilityPersistentDataStoreConsul = "persistent-data-store-consul" - CapabilityPersistentDataStoreDynamoDB = "persistent-data-store-dynamodb" - CapabilityFlagChangeListeners = "flag-change-listeners" - CapabilityFlagValueChangeListeners = "flag-value-change-listeners" - CapabilityFDv1Fallback = "fdv1-fallback" - CapabilityInstanceID = "instance-id" + CapabilityBigSegments = "big-segments" + CapabilitySecureModeHash = "secure-mode-hash" + CapabilityServerSidePolling = "server-side-polling" + CapabilityServiceEndpoints = "service-endpoints" + CapabilityTags = "tags" + CapabilityFiltering = "filtering" + CapabilityContextType = "context-type" + CapabilityMigrations = "migrations" + CapabilityEventSampling = "event-sampling" + CapabilityInlineContextAll = "inline-context-all" + CapabilityAnonymousRedaction = "anonymous-redaction" + CapabilityEvaluationHooks = "evaluation-hooks" + CapabilityTrackHooks = "track-hooks" + CapabilityOmitAnonymousContexts = "omit-anonymous-contexts" + CapabilityEventGzip = "event-gzip" + CapabilityOptionalEventGzip = "optional-event-gzip" + CapabilityPollingGzip = "polling-gzip" + CapabilityClientPrereqEvents = "client-prereq-events" + CapabilityPersistentDataStoreRedis = "persistent-data-store-redis" + CapabilityPersistentDataStoreConsul = "persistent-data-store-consul" + CapabilityPersistentDataStoreDynamoDB = "persistent-data-store-dynamodb" + CapabilityFlagChangeListeners = "flag-change-listeners" + CapabilityFlagValueChangeListeners = "flag-value-change-listeners" + CapabilityFDv1Fallback = "fdv1-fallback" + CapabilityInstanceID = "instance-id" + CapabilityRetryConformanceFDv1Streaming = "retry-conformance-fdv1-streaming" + CapabilityRetryConformanceFDv1Polling = "retry-conformance-fdv1-polling" ) type StatusRep struct { From b68347d6d259a5b5a79fbd3ad5d4fdabbe2eafa9 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 7 Aug 2026 16:31:25 -0400 Subject: [PATCH 02/12] build: drop local eventsource replace from PR diff The branch still depends on the unreleased RetryCurve API in eventsource PR #68 to compile, but committing the local replace directive to the PR would mask the fact that this PR cannot merge until that API is released. Reviewers should treat the pending CI failure ("undefined: eventsource. NewRetryCurve" etc.) as the intended signal. To iterate locally in the meantime, add a personal (uncommitted) replace github.com/launchdarkly/eventsource => ../eventsource to go.mod / testservice/go.mod. Once an eventsource release with the RetryCurve API is tagged, bump the eventsource require line here. --- go.mod | 5 ----- go.sum | 2 ++ testservice/go.mod | 9 ++------- testservice/go.sum | 6 ++++-- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index 8bf2c2fc..9a345af1 100644 --- a/go.mod +++ b/go.mod @@ -35,8 +35,3 @@ require ( ) retract [v7.15.1, v7.15.2] // Introduced unintentional breaking changes; use version v7.15.3 or later. - -// TODO(SDK-2788): temporary local replace so this branch can build against the -// unreleased RetryCurve API in eventsource PR #68. Remove after the eventsource -// PR merges and a release with the API is tagged (>=1.12.0 expected). -replace github.com/launchdarkly/eventsource => ../eventsource diff --git a/go.sum b/go.sum index 418259bd..3f81bb71 100644 --- a/go.sum +++ b/go.sum @@ -17,6 +17,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/launchdarkly/ccache v1.1.0 h1:voD1M+ZJXR3MREOKtBwgTF9hYHl1jg+vFKS/+VAkR2k= github.com/launchdarkly/ccache v1.1.0/go.mod h1:TlxzrlnzvYeXiLHmesMuvoZetu4Z97cV1SsdqqBJi1Q= +github.com/launchdarkly/eventsource v1.10.0 h1:H9Tp6AfGu/G2qzBJC26iperrvwhzdbiA/gx7qE2nDFI= +github.com/launchdarkly/eventsource v1.10.0/go.mod h1:J3oa50bPvJesZqNAJtb5btSIo5N6roDWhiAS3IpsKck= github.com/launchdarkly/go-jsonstream/v3 v3.1.1 h1:ugupp2eNtwVbr69KCdeUrm1vUf1/3ju4Wdliaob95uY= github.com/launchdarkly/go-jsonstream/v3 v3.1.1/go.mod h1:ZBjhKq8mhArCtqotGRGnteY6eXpNm1GaOdUSZHh+ZjM= github.com/launchdarkly/go-ntlm-proxy-auth v1.0.3 h1:i3V0N+R0Fd2nXfGEVKCBIZ8kyttZ+SRKvBG8cdcphO4= diff --git a/testservice/go.mod b/testservice/go.mod index 7cf7ef24..574bc5aa 100644 --- a/testservice/go.mod +++ b/testservice/go.mod @@ -1,6 +1,6 @@ module github.com/launchdarkly/go-server-sdk/v7/testservice -go 1.25.0 +go 1.24.0 require ( github.com/aws/aws-sdk-go-v2/config v1.17.5 @@ -56,12 +56,7 @@ require ( github.com/patrickmn/go-cache v2.1.0+incompatible // indirect github.com/redis/go-redis/v9 v9.0.2 // indirect golang.org/x/sync v0.8.0 // indirect - golang.org/x/sys v0.45.0 // indirect + golang.org/x/sys v0.38.0 // indirect ) replace github.com/launchdarkly/go-server-sdk/v7 => ../ - -// TODO(SDK-2788): temporary local replace so this branch can build against the -// unreleased RetryCurve API in eventsource PR #68. Remove when the eventsource -// PR merges and a release with the API is tagged (>=1.12.0 expected). -replace github.com/launchdarkly/eventsource => ../../eventsource diff --git a/testservice/go.sum b/testservice/go.sum index b3cb975c..ce85f8b9 100644 --- a/testservice/go.sum +++ b/testservice/go.sum @@ -104,6 +104,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/launchdarkly/ccache v1.1.0 h1:voD1M+ZJXR3MREOKtBwgTF9hYHl1jg+vFKS/+VAkR2k= github.com/launchdarkly/ccache v1.1.0/go.mod h1:TlxzrlnzvYeXiLHmesMuvoZetu4Z97cV1SsdqqBJi1Q= +github.com/launchdarkly/eventsource v1.10.0 h1:H9Tp6AfGu/G2qzBJC26iperrvwhzdbiA/gx7qE2nDFI= +github.com/launchdarkly/eventsource v1.10.0/go.mod h1:J3oa50bPvJesZqNAJtb5btSIo5N6roDWhiAS3IpsKck= github.com/launchdarkly/go-jsonstream/v3 v3.1.1 h1:ugupp2eNtwVbr69KCdeUrm1vUf1/3ju4Wdliaob95uY= github.com/launchdarkly/go-jsonstream/v3 v3.1.1/go.mod h1:ZBjhKq8mhArCtqotGRGnteY6eXpNm1GaOdUSZHh+ZjM= github.com/launchdarkly/go-sdk-common/v3 v3.5.0 h1:DsfTimg4BZO2hQPeeeEZYZVsP25pAWEdq/rKv/b7HJU= @@ -196,8 +198,8 @@ golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210303074136-134d130e1a04/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= -golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= +golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= From a2972adb6c4f802fdf3613446800e396a5d62fbb Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 11 Aug 2026 13:19:54 -0400 Subject: [PATCH 03/12] reset polling n on transition into extended regime (SDK-2788) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pollingStrategy.OnFailure previously incremented the counter unconditionally before applying the regime swap. A sequence of "normal, normal, unexpected" would leave the counter at 3 at the moment of the regime swap, producing a first-extended-regime wait of initialDelay * 2^2 = 20min rather than the intended 5min (bounded by extendedPollMaxDelay = 1h in the worst case). Fix: on the transition from normal into extended regime (detected via initialDelay == normalInterval at the moment of the unexpected failure), reset n to 1 so the formula yields initialDelay * 2^0 = initialDelay. RETRY §1.5.3 explicitly delegates the increment behavior on unexpected failure to the component's own specification, so this is spec-conformant. Also rename the field from `attempts` to `n` to (a) name the field for its role as the formula input (RETRY §1.4.1) and (b) match the naming convention used in the RETRY spec and streaming Confluence spec. Adds two regression tests: - TestPollingStrategy_UnexpectedAfterNormalFailuresStartsAtInitialDelay covers the specific case that was broken (normal, normal, normal, unexpected → first extended wait ∈ [2.5min, 5min]). - TestPollingStrategy_UnexpectedWhileAlreadyExtendedContinuesDoubling covers the counterpart (second unexpected in extended does NOT re-reset n; normal failure in extended increments n without exiting the regime). Contract-test scenarios in sdk-test-harness all begin with an unexpected failure as the first failure of the SDK's lifetime, so the bug was not triggered by the harness. Discovered during retro discussion. --- .../datasource/polling_data_source_test.go | 6 +- internal/datasource/polling_strategy.go | 26 +++--- internal/datasource/polling_strategy_test.go | 79 ++++++++++++++++--- 3 files changed, 90 insertions(+), 21 deletions(-) diff --git a/internal/datasource/polling_data_source_test.go b/internal/datasource/polling_data_source_test.go index 4d8b8acb..ce6497b9 100644 --- a/internal/datasource/polling_data_source_test.go +++ b/internal/datasource/polling_data_source_test.go @@ -213,8 +213,8 @@ func testPollingProcessorUnexpectedError( } // After two consecutive successful polls, the processor must reset to the normal -// regime: attempts=0, and subsequent waits equal PollInterval (not the extended -// initial delay). This exercises the RETRY §1.8 polling binding, where the reset +// regime: n=0, and subsequent waits equal PollInterval (not the extended initial +// delay). This exercises the RETRY §1.8 polling binding, where the reset // condition is a fixed count of successful polls rather than a time threshold. func TestPollingResetsToNormalAfterTwoConsecutiveSuccesses(t *testing.T) { req := mocks.NewPollingRequester() @@ -242,7 +242,7 @@ func TestPollingResetsToNormalAfterTwoConsecutiveSuccesses(t *testing.T) { <-req.PollsCh // Poll #2: success. priorPollWasSuccessful flips true; regime not yet reset. <-req.PollsCh - // Poll #3: second consecutive success — reset should fire (attempts=0, back to normal). + // Poll #3: second consecutive success — reset should fire (n=0, back to normal). <-req.PollsCh // Poll #4: normal regime. The wait between polls should be ~PollInterval (10ms), not // the extended base. Bounding at 200ms keeps a big margin against goroutine diff --git a/internal/datasource/polling_strategy.go b/internal/datasource/polling_strategy.go index ac99189b..194fb3a5 100644 --- a/internal/datasource/polling_strategy.go +++ b/internal/datasource/polling_strategy.go @@ -14,7 +14,9 @@ const extendedPollMaxDelay = 1 * time.Hour // pollingStrategy implements the RETRY §1.4 timing mechanics for the polling // data source. It owns: // -// - the attempts counter n (RETRY §1.4); +// - the formula-input counter n (RETRY §1.4's "attempts", used as the +// exponent in T = initialDelay * 2^(n-1); resets on regime transition +// per RETRY §1.5.3 binding); // - the current regime's (initialDelay, maxDelay), toggled by classification // (RETRY §1.5–§1.7 via the caller's FailureClass); // - the two-consecutive-success reset gate (RETRY §1.8 polling binding); @@ -27,7 +29,7 @@ type pollingStrategy struct { normalInterval time.Duration extendedInitialPollInterval time.Duration rng *rand.Rand - attempts int + n int priorPollWasSuccessful bool initialDelay time.Duration maxDelay time.Duration @@ -48,10 +50,14 @@ func newPollingStrategy(pollInterval, extendedInitialPollInterval time.Duration) // classification engages the extended regime for subsequent waits; // initialDelay is clamped to at least PollInterval so the extended regime // is never faster than the customer-configured normal cadence. +// +// On the transition from normal into extended regime, n is reset to 1 so +// that the first extended-regime wait uses the new initialDelay. func (s *pollingStrategy) OnFailure(class FailureClass) { s.priorPollWasSuccessful = false - s.attempts++ - if class == FailureClassUnexpected { + if class == FailureClassUnexpected && s.initialDelay == s.normalInterval { + // transition from normal into extended regime. + s.n = 1 s.initialDelay = s.extendedInitialPollInterval if s.initialDelay < s.normalInterval { s.initialDelay = s.normalInterval @@ -60,16 +66,18 @@ func (s *pollingStrategy) OnFailure(class FailureClass) { if s.maxDelay < s.normalInterval { s.maxDelay = s.normalInterval } + return } + s.n++ } // OnSuccess updates the strategy state after a successful poll. Two consecutive -// successes reset attempts and return the SDK to the normal regime (RETRY §1.8 +// successes reset n and return the SDK to the normal regime (RETRY §1.8 // polling binding). A single success is a necessary precondition but not // sufficient — any failure between the first and second success clears it. func (s *pollingStrategy) OnSuccess() { if s.priorPollWasSuccessful { - s.attempts = 0 + s.n = 0 s.initialDelay = s.normalInterval s.maxDelay = s.normalInterval } @@ -77,16 +85,16 @@ func (s *pollingStrategy) OnSuccess() { } // NextWait returns the delay before the next poll attempt per RETRY §1.4. -// Formula: T = initialDelay * 2^(attempts-1), clamped to maxDelay. +// Formula: T = initialDelay * 2^(n-1), clamped to maxDelay. // Jitter J is a uniform random in [0, T/2]. The final wait = max(PollInterval, // T − J) ensures the interval never drops below the caller's configured // PollInterval. func (s *pollingStrategy) NextWait() time.Duration { - if s.attempts <= 0 { + if s.n <= 0 { return s.normalInterval } t := time.Duration(math.Min( - float64(s.initialDelay)*math.Pow(2, float64(s.attempts-1)), + float64(s.initialDelay)*math.Pow(2, float64(s.n-1)), float64(s.maxDelay), )) var jitter time.Duration diff --git a/internal/datasource/polling_strategy_test.go b/internal/datasource/polling_strategy_test.go index 60fce21f..044ee406 100644 --- a/internal/datasource/polling_strategy_test.go +++ b/internal/datasource/polling_strategy_test.go @@ -13,13 +13,13 @@ func TestPollingStrategy_NormalRegimeReturnsPollInterval(t *testing.T) { assert.Equal(t, 30*time.Second, s.NextWait()) } -// A normal-classified failure advances the attempts counter but does not engage +// A normal-classified failure advances n but does not engage // the extended regime. Wait is still pinned to PollInterval by the wait floor. func TestPollingStrategy_NormalFailureDoesNotEngageExtended(t *testing.T) { s := newPollingStrategy(30*time.Second, 5*time.Minute) s.OnFailure(FailureClassNormal) - assert.Equal(t, 1, s.attempts) + assert.Equal(t, 1, s.n) assert.Equal(t, 30*time.Second, s.initialDelay) assert.Equal(t, 30*time.Second, s.maxDelay) assert.Equal(t, 30*time.Second, s.NextWait()) @@ -49,6 +49,67 @@ func TestPollingStrategy_ExtendedInitialClampedToPollInterval(t *testing.T) { assert.Equal(t, time.Hour, s.NextWait()) } +// After prior normal-classified failures, the first unexpected failure MUST +// reset n to 1 so the first extended-regime wait uses the new initialDelay +// (5min) directly, not initialDelay * 2^k where k is the count of prior +// normal failures. Guards against conflating the two roles of a single +// counter — formula input (this field, n; resets on regime transition per +// RETRY §1.5.3 / streaming Confluence spec) and total-attempts observability +// (a separate concept not tracked by this struct). Without this behavior, a +// sequence of "normal, normal, unexpected" would inflate the first extended +// wait to 20min or more (bounded by extendedPollMaxDelay). +func TestPollingStrategy_UnexpectedAfterNormalFailuresStartsAtInitialDelay(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + + // Prior normal failures accumulate. Normal-regime NextWait is bounded by + // normalInterval regardless of n, so these aren't observable in delay — + // but they DO advance n. + s.OnFailure(FailureClassNormal) + s.OnFailure(FailureClassNormal) + s.OnFailure(FailureClassNormal) + assert.Equal(t, 3, s.n, "n should advance on normal failures") + + // First unexpected failure. Transition into extended regime must reset + // n to 1 so the first extended-regime formula yields + // initialDelay * 2^0 = initialDelay = 5min. + s.OnFailure(FailureClassUnexpected) + assert.Equal(t, 1, s.n, "n must reset to 1 on transition into extended regime") + assert.Equal(t, 5*time.Minute, s.initialDelay, "extended regime initialDelay engaged") + assert.Equal(t, time.Hour, s.maxDelay, "extended regime maxDelay engaged") + + // First extended-regime wait: T = 5min * 2^0 = 5min, minus jitter in + // [0, T/2]. Actual wait is in [2.5min, 5min]. + w := s.NextWait() + assert.GreaterOrEqual(t, w, 2*time.Minute+30*time.Second) + assert.LessOrEqual(t, w, 5*time.Minute) +} + +// Once in the extended regime, subsequent unexpected failures continue the +// doubling from where n left off — they do NOT re-reset n to 1. The +// reset-on-transition only fires on the first crossing from normal into +// extended, detected via initialDelay == normalInterval. +func TestPollingStrategy_UnexpectedWhileAlreadyExtendedContinuesDoubling(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + + // Enter extended regime. + s.OnFailure(FailureClassUnexpected) + assert.Equal(t, 1, s.n) + + // Second unexpected while already in extended. n advances to 2; + // initialDelay/maxDelay stay at extended values. + s.OnFailure(FailureClassUnexpected) + assert.Equal(t, 2, s.n, "second unexpected in extended increments, does not reset") + assert.Equal(t, 5*time.Minute, s.initialDelay) + assert.Equal(t, time.Hour, s.maxDelay) + + // A normal failure while in extended also advances n without + // changing regime. + s.OnFailure(FailureClassNormal) + assert.Equal(t, 3, s.n, "normal failure in extended increments n") + assert.Equal(t, 5*time.Minute, s.initialDelay, "normal failure does not exit extended regime") + assert.Equal(t, time.Hour, s.maxDelay) +} + // Extended regime doubles per attempt per RETRY §1.4, capped at // extendedPollMaxDelay (1 hour). Jitter subtracts up to T/2, so each wait falls // in [T/2, T] before the (in these cases irrelevant) wait floor. @@ -87,18 +148,18 @@ func TestPollingStrategy_WaitFloorAtPollInterval(t *testing.T) { } // The 2-consecutive-success reset gate: first success flips the gate flag but -// does not clear attempts or exit the extended regime. +// does not clear n or exit the extended regime. func TestPollingStrategy_FirstSuccessDoesNotReset(t *testing.T) { s := newPollingStrategy(30*time.Second, 5*time.Minute) s.OnFailure(FailureClassUnexpected) s.OnSuccess() assert.True(t, s.priorPollWasSuccessful, "reset gate should be armed") - assert.Equal(t, 1, s.attempts, "one success must not reset attempts") + assert.Equal(t, 1, s.n, "one success must not reset n") assert.Equal(t, 5*time.Minute, s.initialDelay, "one success must not exit extended regime") } -// Two consecutive successes clear attempts and return the strategy to the +// Two consecutive successes clear n and return the strategy to the // normal regime (RETRY §1.8 polling binding). func TestPollingStrategy_TwoConsecutiveSuccessesReset(t *testing.T) { s := newPollingStrategy(30*time.Second, 5*time.Minute) @@ -106,7 +167,7 @@ func TestPollingStrategy_TwoConsecutiveSuccessesReset(t *testing.T) { s.OnSuccess() s.OnSuccess() - assert.Equal(t, 0, s.attempts) + assert.Equal(t, 0, s.n) assert.Equal(t, 30*time.Second, s.initialDelay) assert.Equal(t, 30*time.Second, s.maxDelay) assert.Equal(t, 30*time.Second, s.NextWait()) @@ -119,11 +180,11 @@ func TestPollingStrategy_FailureClearsResetGate(t *testing.T) { s := newPollingStrategy(30*time.Second, 5*time.Minute) s.OnFailure(FailureClassUnexpected) s.OnSuccess() // gate armed - s.OnFailure(FailureClassNormal) // gate cleared, attempts=2 + s.OnFailure(FailureClassNormal) // gate cleared, n=2 s.OnSuccess() // gate armed again but does not fire reset assert.True(t, s.priorPollWasSuccessful) - assert.Equal(t, 2, s.attempts, "reset must not have fired") + assert.Equal(t, 2, s.n, "reset must not have fired") } // A single normal failure after a reset does not re-engage extended-regime @@ -137,7 +198,7 @@ func TestPollingStrategy_NormalFailureAfterResetStaysNormal(t *testing.T) { // A subsequent normal failure must not re-engage extended. s.OnFailure(FailureClassNormal) - assert.Equal(t, 1, s.attempts) + assert.Equal(t, 1, s.n) assert.Equal(t, 30*time.Second, s.initialDelay, "normal failure must not engage extended regime") assert.Equal(t, 30*time.Second, s.maxDelay) } From 1c0064b4060eca51fcfbb15d02a183aeb12ee8af Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 11 Aug 2026 14:43:25 -0400 Subject: [PATCH 04/12] adopt eventsource RetryProfile API rename (SDK-2788) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The eventsource library renamed RetryCurve to RetryProfile in response to review feedback on launchdarkly/eventsource#68. Update the streaming data-source wire-up to consume the new API. - es.NewRetryCurve / RetryCurveBaseDelay / MaxDelay / Jitter → es.NewRetryProfile / RetryProfileBaseDelay / … - result.ActivateCurve → result.ActivateProfile - es.StreamOptionDefaultRetryCurve / RegisterRetryCurve → es.StreamOptionDefaultRetryProfile / RegisterRetryProfile - Local vars defaultCurve / extendedCurve → defaultProfile / extendedProfile - Comment references to "retry curve" / "extended-regime curve" → "profile" go.mod is intentionally left pinned at eventsource v1.10.0. CI will be red on this PR until eventsource releases the renamed API and go.mod is bumped, matching the sequencing the epic assumes. --- internal/datasource/helpers.go | 2 +- internal/datasource/streaming_data_source.go | 24 +++++++++---------- .../datasource/streaming_data_source_test.go | 2 +- ldclient_end_to_end_test.go | 2 +- ldcomponents/streaming_data_source_builder.go | 2 +- testhelpers/datasourcetest/datasourcetest.go | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/internal/datasource/helpers.go b/internal/datasource/helpers.go index b304fd4e..b814d38a 100644 --- a/internal/datasource/helpers.go +++ b/internal/datasource/helpers.go @@ -26,7 +26,7 @@ func (e httpStatusError) Error() string { // FailureClass categorizes a data source failure per RETRY §1.5–§1.7. Under the // RETRY spec no failure is permanently terminal: every failure is either "normal" // (regular backoff and retry) or "unexpected" (extended backoff via a longer -// retry curve or wait interval, still retrying indefinitely). +// retry profile or wait interval, still retrying indefinitely). type FailureClass int const ( diff --git a/internal/datasource/streaming_data_source.go b/internal/datasource/streaming_data_source.go index 56352736..c60bf52e 100644 --- a/internal/datasource/streaming_data_source.go +++ b/internal/datasource/streaming_data_source.go @@ -312,16 +312,16 @@ func (sp *StreamProcessor) subscribe(closeWhenReady chan<- struct{}) { retryResetInterval = streamRetryResetInterval } - defaultCurve := es.NewRetryCurve( - es.RetryCurveBaseDelay(initialRetryDelay), - es.RetryCurveMaxDelay(streamMaxRetryDelay), - es.RetryCurveJitter(streamJitterRatio), + defaultProfile := es.NewRetryProfile( + es.RetryProfileBaseDelay(initialRetryDelay), + es.RetryProfileMaxDelay(streamMaxRetryDelay), + es.RetryProfileJitter(streamJitterRatio), ) - extendedCurve := es.NewRetryCurve( - es.RetryCurveBaseDelay(extendedInitialDelay), - es.RetryCurveMaxDelay(streamExtendedMaxRetryDelay), - es.RetryCurveJitter(streamJitterRatio), + extendedProfile := es.NewRetryProfile( + es.RetryProfileBaseDelay(extendedInitialDelay), + es.RetryProfileMaxDelay(streamExtendedMaxRetryDelay), + es.RetryProfileJitter(streamJitterRatio), ) errorHandler := func(err error) es.StreamErrorHandlerResult { @@ -361,12 +361,12 @@ func (sp *StreamProcessor) subscribe(closeWhenReady chan<- struct{}) { sp.logConnectionStarted() // Per RETRY §1.2.1: no failure is permanently terminal. Unexpected failures - // engage the extended-regime curve; the library keeps retrying at extended + // engage the extended-regime profile; the library keeps retrying at extended // cadence until a healthy-op reset (retryResetInterval of continuous // connection) reverts. result := es.StreamErrorHandlerResult{CloseNow: false} if class == FailureClassUnexpected { - result.ActivateCurve = extendedCurve + result.ActivateProfile = extendedProfile } return result } @@ -374,8 +374,8 @@ func (sp *StreamProcessor) subscribe(closeWhenReady chan<- struct{}) { stream, err := es.SubscribeWithRequestAndOptions(req, es.StreamOptionHTTPClient(sp.client), es.StreamOptionReadTimeout(streamReadTimeout), - es.StreamOptionDefaultRetryCurve(defaultCurve), - es.StreamOptionRegisterRetryCurve(extendedCurve), + es.StreamOptionDefaultRetryProfile(defaultProfile), + es.StreamOptionRegisterRetryProfile(extendedProfile), es.StreamOptionRetryResetInterval(retryResetInterval), es.StreamOptionErrorHandler(errorHandler), es.StreamOptionCanRetryFirstConnection(-1), diff --git a/internal/datasource/streaming_data_source_test.go b/internal/datasource/streaming_data_source_test.go index b7f09adc..9bd8b907 100644 --- a/internal/datasource/streaming_data_source_test.go +++ b/internal/datasource/streaming_data_source_test.go @@ -402,7 +402,7 @@ func testStreamProcessorUnexpectedHTTPError(t *testing.T, statusCode int) { } // Short retry delays so we can observe at least two attempts within the - // assertion window. The extended-regime curve activates immediately on the + // assertion window. The extended-regime profile activates immediately on the // first failure (per the RETRY spec — no grace period for initial-connection // unexpected classifications), so we need to shorten ExtendedInitialReconnectDelay // too, not just the normal InitialReconnectDelay. diff --git a/ldclient_end_to_end_test.go b/ldclient_end_to_end_test.go index 77fd0656..8181799c 100644 --- a/ldclient_end_to_end_test.go +++ b/ldclient_end_to_end_test.go @@ -115,7 +115,7 @@ func TestClientInStreamingModeWith401KeepsRetrying(t *testing.T) { logCapture := ldlogtest.NewMockLog() // Short reconnect delays so multiple attempts fit within the init wait window. - // Extended-regime curve activates immediately on 401 (unexpected), so we need + // Extended-regime profile activates immediately on 401 (unexpected), so we need // to shorten its base too, not just the normal-regime InitialReconnectDelay. streamingBuilder := ldcomponents.StreamingDataSource().InitialReconnectDelay(10 * time.Millisecond) datasourcetest.WithStreamingExtendedInitialReconnectDelay(streamingBuilder, 20*time.Millisecond) diff --git a/ldcomponents/streaming_data_source_builder.go b/ldcomponents/streaming_data_source_builder.go index 3ab6e385..9bf987f5 100644 --- a/ldcomponents/streaming_data_source_builder.go +++ b/ldcomponents/streaming_data_source_builder.go @@ -84,7 +84,7 @@ func (b *StreamingDataSourceBuilder) Internal() StreamingDataSourceBuilderIntern } // ExtendedInitialReconnectDelay sets the base delay for the extended-regime retry -// curve that engages after RETRY-classified unexpected failures (401, 403, TLS/cert). +// profile that engages after RETRY-classified unexpected failures (401, 403, TLS/cert). // Values ≤ 0 are clamped to [DefaultExtendedInitialReconnectDelay]. func (i StreamingDataSourceBuilderInternal) ExtendedInitialReconnectDelay( delay time.Duration, diff --git a/testhelpers/datasourcetest/datasourcetest.go b/testhelpers/datasourcetest/datasourcetest.go index 3b929fa7..ed356b25 100644 --- a/testhelpers/datasourcetest/datasourcetest.go +++ b/testhelpers/datasourcetest/datasourcetest.go @@ -19,7 +19,7 @@ import ( ) // WithStreamingExtendedInitialReconnectDelay overrides the RETRY-spec default -// base delay for the streaming extended-regime retry curve. Test-only. +// base delay for the streaming extended-regime retry profile. Test-only. func WithStreamingExtendedInitialReconnectDelay( b *ldcomponents.StreamingDataSourceBuilder, delay time.Duration, From 2d496230a7b359c35af5d2bc94960b3c0a541474 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Wed, 12 Aug 2026 13:34:17 -0400 Subject: [PATCH 05/12] bump eventsource to v1.13.0 with RetryProfile API (SDK-2788) Consumes the RetryProfile API introduced in launchdarkly/eventsource#68 and released as v1.13.0. This is the final piece of the SDK-2788 chain; CI on this PR should now go green. - go.mod: launchdarkly/eventsource v1.10.0 -> v1.13.0 - go.sum updated accordingly --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 9a345af1..abd3878e 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/google/uuid v1.1.1 github.com/gregjones/httpcache v0.0.0-20171119193500-2bcd89a1743f github.com/launchdarkly/ccache v1.1.0 - github.com/launchdarkly/eventsource v1.10.0 + github.com/launchdarkly/eventsource v1.13.0 github.com/launchdarkly/go-jsonstream/v3 v3.1.1 github.com/launchdarkly/go-ntlm-proxy-auth v1.0.3 github.com/launchdarkly/go-sdk-common/v3 v3.5.0 diff --git a/go.sum b/go.sum index 3f81bb71..38661def 100644 --- a/go.sum +++ b/go.sum @@ -17,8 +17,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/launchdarkly/ccache v1.1.0 h1:voD1M+ZJXR3MREOKtBwgTF9hYHl1jg+vFKS/+VAkR2k= github.com/launchdarkly/ccache v1.1.0/go.mod h1:TlxzrlnzvYeXiLHmesMuvoZetu4Z97cV1SsdqqBJi1Q= -github.com/launchdarkly/eventsource v1.10.0 h1:H9Tp6AfGu/G2qzBJC26iperrvwhzdbiA/gx7qE2nDFI= -github.com/launchdarkly/eventsource v1.10.0/go.mod h1:J3oa50bPvJesZqNAJtb5btSIo5N6roDWhiAS3IpsKck= +github.com/launchdarkly/eventsource v1.13.0 h1:SjC1LgNSR+ip8BgZcphGa/ar6N0I+pFTYQ/1ZzDnP98= +github.com/launchdarkly/eventsource v1.13.0/go.mod h1:dU+rZxkPOlGPsyJPpiDqiepAcFwIITDUClY9+A6RrMw= github.com/launchdarkly/go-jsonstream/v3 v3.1.1 h1:ugupp2eNtwVbr69KCdeUrm1vUf1/3ju4Wdliaob95uY= github.com/launchdarkly/go-jsonstream/v3 v3.1.1/go.mod h1:ZBjhKq8mhArCtqotGRGnteY6eXpNm1GaOdUSZHh+ZjM= github.com/launchdarkly/go-ntlm-proxy-auth v1.0.3 h1:i3V0N+R0Fd2nXfGEVKCBIZ8kyttZ+SRKvBG8cdcphO4= From 655f9c6cd111d916ff1427a4dc44890111688993 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Mon, 17 Aug 2026 15:23:46 -0400 Subject: [PATCH 06/12] fix(datasource): address Cursor review feedback on RETRY-conformance PR Threads on PR #429: 1. Close cannot stop stream retries Replace halt chan with streamReqCtx / streamReqCancel context. Build the streaming request via http.NewRequestWithContext so Close() interrupts an in-flight Do; retry-sleep interruption arrives with eventsource #71 once it releases. 2. Contradictory polling retry logs Fold both classifyAndLog branches to Warnf per CLM 1.1.4 (under RETRY no failure is permanent, so per-attempt logs are always temporary conditions). Drop the hardcoded "will continue retrying with extended backoff" suffix that contradicted polling's "will retry at next scheduled poll interval". Add a one-time Info log on the transition into extended regime for both streaming and polling: "Classified failure as UNEXPECTED; engaging extended backoff." 3, 5. Unicode dashes in Go comments -- replaced with ASCII across all PR-touched files. 4. Broken extended-regime transition detection Replace equality-based detection (initialDelay == normalInterval) with an explicit inExtended flag on pollingStrategy. The prior check re-fired the transition path on every unexpected failure whenever PollInterval >= extendedInitialPollInterval (via the clamp), which held n at 1 and defeated RETRY 1.4.1's doubling. OnFailure now returns a bool signalling the transition once, so the caller emits the extended-backoff Info log exactly once per transition. OnSuccess's two-consecutive-success reset also clears inExtended so subsequent unexpected failures re-transition properly. Regression tests: doubling under PollInterval == extendedInitial, OnFailure transition-return semantics, post-reset re-transition, PollInterval = 10min (moderate), PollInterval = 2h (collapse case where extended regime is indistinguishable from normal cadence). SDK-2788 --- internal/datasource/helpers.go | 28 +--- internal/datasource/polling_data_source.go | 4 +- .../datasource/polling_data_source_test.go | 6 +- internal/datasource/polling_strategy.go | 22 +-- internal/datasource/polling_strategy_test.go | 125 +++++++++++++++++- internal/datasource/streaming_data_source.go | 36 +++-- .../datasource/streaming_data_source_test.go | 8 +- ldclient_end_to_end_test.go | 10 +- ldcomponents/streaming_data_source_builder.go | 2 +- 9 files changed, 181 insertions(+), 60 deletions(-) diff --git a/internal/datasource/helpers.go b/internal/datasource/helpers.go index b814d38a..570a2e2d 100644 --- a/internal/datasource/helpers.go +++ b/internal/datasource/helpers.go @@ -23,7 +23,7 @@ func (e httpStatusError) Error() string { return e.Message } -// FailureClass categorizes a data source failure per RETRY §1.5–§1.7. Under the +// FailureClass categorizes a data source failure per RETRY §1.5--§1.7. Under the // RETRY spec no failure is permanently terminal: every failure is either "normal" // (regular backoff and retry) or "unexpected" (extended backoff via a longer // retry profile or wait interval, still retrying indefinitely). @@ -88,24 +88,16 @@ func httpErrorDescription(statusCode int) string { return fmt.Sprintf("HTTP error %d%s", statusCode, message) } -// classifyAndLogHTTPFailure classifies an HTTP failure per RETRY §1.6, logs it -// at the appropriate level, and returns the classification for the caller to act -// on. Never signals "give up permanently" — under the RETRY spec the caller must -// continue retrying, though possibly with an extended backoff regime. +// classifyAndLogHTTPFailure classifies an HTTP failure per RETRY §1.6, logs it, +// and returns the classification for the caller to act on. func classifyAndLogHTTPFailure( loggers ldlog.Loggers, errorDesc, errorContext string, statusCode int, willRetryMessage string, ) FailureClass { - class := classifyHTTPFailure(statusCode) - if class == FailureClassUnexpected { - loggers.Errorf("Error %s (%s; will continue retrying with extended backoff): %s", - errorContext, willRetryMessage, errorDesc) - } else { - loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, errorDesc) - } - return class + loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, errorDesc) + return classifyHTTPFailure(statusCode) } // classifyAndLogTransportFailure classifies a transport-layer failure per RETRY @@ -115,14 +107,8 @@ func classifyAndLogTransportFailure( err error, errorContext, willRetryMessage string, ) FailureClass { - class := classifyTransportFailure(err) - if class == FailureClassUnexpected { - loggers.Errorf("Error %s (%s; will continue retrying with extended backoff): %s", - errorContext, willRetryMessage, err.Error()) - } else { - loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, err.Error()) - } - return class + loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, err.Error()) + return classifyTransportFailure(err) } func checkForHTTPError(statusCode int, url string) error { diff --git a/internal/datasource/polling_data_source.go b/internal/datasource/polling_data_source.go index fce7d340..23cc5ecf 100644 --- a/internal/datasource/polling_data_source.go +++ b/internal/datasource/polling_data_source.go @@ -139,7 +139,9 @@ func (pp *PollingProcessor) Start(closeWhenReady chan<- struct{}) { ) pp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateInterrupted, errorInfo) } - pp.strategy.OnFailure(class) + if pp.strategy.OnFailure(class) { + pp.loggers.Info("Classified failure as UNEXPECTED; engaging extended backoff.") + } } else { pp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateValid, interfaces.DataSourceErrorInfo{}) pp.setInitializedOnce.Do(func() { diff --git a/internal/datasource/polling_data_source_test.go b/internal/datasource/polling_data_source_test.go index ce6497b9..4109677a 100644 --- a/internal/datasource/polling_data_source_test.go +++ b/internal/datasource/polling_data_source_test.go @@ -194,7 +194,7 @@ func testPollingProcessorUnexpectedError( // Initialization must not complete: no permanent stop, no successful poll. select { case <-closeWhenReady: - t.Fatal("closeWhenReady should not be closed — RETRY §1.2.1 forbids permanent stops on 4xx") + t.Fatal("closeWhenReady should not be closed -- RETRY §1.2.1 forbids permanent stops on 4xx") case <-time.After(500 * time.Millisecond): } @@ -202,7 +202,7 @@ func testPollingProcessorUnexpectedError( status := dataSourceUpdates.RequireStatusOf(t, interfaces.DataSourceStateInterrupted) verifyError(status.LastError) - // Confirm the polling processor kept polling — at least one additional poll + // Confirm the polling processor kept polling -- at least one additional poll // attempt landed at the mock requester. select { case <-req.PollsCh: @@ -242,7 +242,7 @@ func TestPollingResetsToNormalAfterTwoConsecutiveSuccesses(t *testing.T) { <-req.PollsCh // Poll #2: success. priorPollWasSuccessful flips true; regime not yet reset. <-req.PollsCh - // Poll #3: second consecutive success — reset should fire (n=0, back to normal). + // Poll #3: second consecutive success -- reset should fire (n=0, back to normal). <-req.PollsCh // Poll #4: normal regime. The wait between polls should be ~PollInterval (10ms), not // the extended base. Bounding at 200ms keeps a big margin against goroutine diff --git a/internal/datasource/polling_strategy.go b/internal/datasource/polling_strategy.go index 194fb3a5..8327caac 100644 --- a/internal/datasource/polling_strategy.go +++ b/internal/datasource/polling_strategy.go @@ -18,12 +18,12 @@ const extendedPollMaxDelay = 1 * time.Hour // exponent in T = initialDelay * 2^(n-1); resets on regime transition // per RETRY §1.5.3 binding); // - the current regime's (initialDelay, maxDelay), toggled by classification -// (RETRY §1.5–§1.7 via the caller's FailureClass); +// (RETRY §1.5--§1.7 via the caller's FailureClass); // - the two-consecutive-success reset gate (RETRY §1.8 polling binding); // - jitter (RETRY §1.4.3); // - the PollInterval wait floor (RETRY §1.4.4 polling override). // -// All state is owned and mutated by the polling run() goroutine only — no +// All state is owned and mutated by the polling run() goroutine only -- no // locking required. type pollingStrategy struct { normalInterval time.Duration @@ -33,6 +33,7 @@ type pollingStrategy struct { priorPollWasSuccessful bool initialDelay time.Duration maxDelay time.Duration + inExtended bool } func newPollingStrategy(pollInterval, extendedInitialPollInterval time.Duration) *pollingStrategy { @@ -52,10 +53,12 @@ func newPollingStrategy(pollInterval, extendedInitialPollInterval time.Duration) // is never faster than the customer-configured normal cadence. // // On the transition from normal into extended regime, n is reset to 1 so -// that the first extended-regime wait uses the new initialDelay. -func (s *pollingStrategy) OnFailure(class FailureClass) { +// that the first extended-regime wait uses the new initialDelay. Returns +// true iff this call transitioned the strategy from normal into extended +// regime, so the caller can log a one-time notice. +func (s *pollingStrategy) OnFailure(class FailureClass) (transitionedToExtended bool) { s.priorPollWasSuccessful = false - if class == FailureClassUnexpected && s.initialDelay == s.normalInterval { + if class == FailureClassUnexpected && !s.inExtended { // transition from normal into extended regime. s.n = 1 s.initialDelay = s.extendedInitialPollInterval @@ -66,20 +69,23 @@ func (s *pollingStrategy) OnFailure(class FailureClass) { if s.maxDelay < s.normalInterval { s.maxDelay = s.normalInterval } - return + s.inExtended = true + return true } s.n++ + return false } // OnSuccess updates the strategy state after a successful poll. Two consecutive // successes reset n and return the SDK to the normal regime (RETRY §1.8 // polling binding). A single success is a necessary precondition but not -// sufficient — any failure between the first and second success clears it. +// sufficient -- any failure between the first and second success clears it. func (s *pollingStrategy) OnSuccess() { if s.priorPollWasSuccessful { s.n = 0 s.initialDelay = s.normalInterval s.maxDelay = s.normalInterval + s.inExtended = false } s.priorPollWasSuccessful = true } @@ -87,7 +93,7 @@ func (s *pollingStrategy) OnSuccess() { // NextWait returns the delay before the next poll attempt per RETRY §1.4. // Formula: T = initialDelay * 2^(n-1), clamped to maxDelay. // Jitter J is a uniform random in [0, T/2]. The final wait = max(PollInterval, -// T − J) ensures the interval never drops below the caller's configured +// T - J) ensures the interval never drops below the caller's configured // PollInterval. func (s *pollingStrategy) NextWait() time.Duration { if s.n <= 0 { diff --git a/internal/datasource/polling_strategy_test.go b/internal/datasource/polling_strategy_test.go index 044ee406..ab9962d4 100644 --- a/internal/datasource/polling_strategy_test.go +++ b/internal/datasource/polling_strategy_test.go @@ -53,7 +53,7 @@ func TestPollingStrategy_ExtendedInitialClampedToPollInterval(t *testing.T) { // reset n to 1 so the first extended-regime wait uses the new initialDelay // (5min) directly, not initialDelay * 2^k where k is the count of prior // normal failures. Guards against conflating the two roles of a single -// counter — formula input (this field, n; resets on regime transition per +// counter -- formula input (this field, n; resets on regime transition per // RETRY §1.5.3 / streaming Confluence spec) and total-attempts observability // (a separate concept not tracked by this struct). Without this behavior, a // sequence of "normal, normal, unexpected" would inflate the first extended @@ -62,7 +62,7 @@ func TestPollingStrategy_UnexpectedAfterNormalFailuresStartsAtInitialDelay(t *te s := newPollingStrategy(30*time.Second, 5*time.Minute) // Prior normal failures accumulate. Normal-regime NextWait is bounded by - // normalInterval regardless of n, so these aren't observable in delay — + // normalInterval regardless of n, so these aren't observable in delay -- // but they DO advance n. s.OnFailure(FailureClassNormal) s.OnFailure(FailureClassNormal) @@ -85,9 +85,9 @@ func TestPollingStrategy_UnexpectedAfterNormalFailuresStartsAtInitialDelay(t *te } // Once in the extended regime, subsequent unexpected failures continue the -// doubling from where n left off — they do NOT re-reset n to 1. The +// doubling from where n left off -- they do NOT re-reset n to 1. The // reset-on-transition only fires on the first crossing from normal into -// extended, detected via initialDelay == normalInterval. +// extended, gated by the inExtended flag. func TestPollingStrategy_UnexpectedWhileAlreadyExtendedContinuesDoubling(t *testing.T) { s := newPollingStrategy(30*time.Second, 5*time.Minute) @@ -173,7 +173,7 @@ func TestPollingStrategy_TwoConsecutiveSuccessesReset(t *testing.T) { assert.Equal(t, 30*time.Second, s.NextWait()) } -// A failure between two successes clears the reset gate — reset requires +// A failure between two successes clears the reset gate -- reset requires // STRICTLY consecutive successes, so a first-then-fail-then-first pattern does // not fire the reset. func TestPollingStrategy_FailureClearsResetGate(t *testing.T) { @@ -188,7 +188,7 @@ func TestPollingStrategy_FailureClearsResetGate(t *testing.T) { } // A single normal failure after a reset does not re-engage extended-regime -// parameters — extended engagement requires an Unexpected classification. +// parameters -- extended engagement requires an Unexpected classification. func TestPollingStrategy_NormalFailureAfterResetStaysNormal(t *testing.T) { s := newPollingStrategy(30*time.Second, 5*time.Minute) // Engage extended, then reset back to normal. @@ -202,3 +202,116 @@ func TestPollingStrategy_NormalFailureAfterResetStaysNormal(t *testing.T) { assert.Equal(t, 30*time.Second, s.initialDelay, "normal failure must not engage extended regime") assert.Equal(t, 30*time.Second, s.maxDelay) } + +// Regression: when PollInterval equals extendedInitialPollInterval (the default +// combo of 5min/5min was the flagged customer config), the extended-regime +// initialDelay clamps up to equal normalInterval. If transition detection +// relies on that equality, every subsequent unexpected failure re-fires the +// transition path and resets n to 1, and RETRY spec 1.4.1's doubling never +// engages. Explicit regime state (inExtended) avoids the clamp collision. +func TestPollingStrategy_ExtendedDoublingWhenClampedToPollInterval(t *testing.T) { + s := newPollingStrategy(5*time.Minute, 5*time.Minute) + + // Drive five unexpected failures; n must advance monotonically. + for i, expectedN := range []int{1, 2, 3, 4, 5} { + s.OnFailure(FailureClassUnexpected) + assert.Equal(t, expectedN, s.n, "failure #%d: n did not advance", i+1) + } + assert.Equal(t, 5*time.Minute, s.initialDelay, "initialDelay clamped to PollInterval") + assert.Equal(t, time.Hour, s.maxDelay, "maxDelay is the extended ceiling") + + // With n=5 the formula T = initialDelay * 2^4 = 80m, clamped to maxDelay=1h. + // Jitter subtracts up to T/2 = 30m, so wait is in [30m, 1h]. Floor at + // PollInterval=5m is well below and does not affect the result. + w := s.NextWait() + assert.GreaterOrEqual(t, w, 30*time.Minute) + assert.LessOrEqual(t, w, time.Hour) +} + +// OnFailure returns true on the transition into extended, false on every +// subsequent unexpected failure. The caller uses this signal to log +// "engaging extended backoff" exactly once per transition. +func TestPollingStrategy_OnFailureReturnsTrueOnceOnTransition(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + + assert.True(t, s.OnFailure(FailureClassUnexpected), "first unexpected must signal transition") + assert.False(t, s.OnFailure(FailureClassUnexpected), "second unexpected must not re-signal transition") + assert.False(t, s.OnFailure(FailureClassUnexpected), "third unexpected must not re-signal transition") + assert.False(t, s.OnFailure(FailureClassNormal), "normal failure must not signal transition") +} + +// Also test the clamped-config variant, since that's the specific bug scenario +// where equality-based detection re-signalled transition on every unexpected. +func TestPollingStrategy_OnFailureReturnsTrueOnceOnTransitionWhenClamped(t *testing.T) { + s := newPollingStrategy(5*time.Minute, 5*time.Minute) + + assert.True(t, s.OnFailure(FailureClassUnexpected), "first unexpected must signal transition") + assert.False(t, s.OnFailure(FailureClassUnexpected), "second unexpected must not re-signal transition") + assert.False(t, s.OnFailure(FailureClassUnexpected), "third unexpected must not re-signal transition") +} + +// After the two-consecutive-successes reset (RETRY §1.8), the strategy is +// back in the normal regime and a subsequent unexpected failure is treated +// as a new transition, signalling to the caller for a fresh log line. +func TestPollingStrategy_OnSuccessResetAllowsRetransition(t *testing.T) { + s := newPollingStrategy(30*time.Second, 5*time.Minute) + + assert.True(t, s.OnFailure(FailureClassUnexpected), "initial transition") + assert.False(t, s.OnFailure(FailureClassUnexpected), "already in extended") + + // Two consecutive successes: full reset per RETRY §1.8. + s.OnSuccess() + s.OnSuccess() + + // Fresh unexpected failure must be treated as a new transition. + assert.True(t, s.OnFailure(FailureClassUnexpected), "post-reset unexpected must signal a fresh transition") +} + +// Case B: PollInterval > extendedInitialPollInterval, but < extendedPollMaxDelay. +// After transition, initialDelay is clamped up to PollInterval so the first +// extended wait equals PollInterval (via the output floor). Doubling engages +// visibly from n=2 and reaches the extended ceiling at n=4. +func TestPollingStrategy_ExtendedDoublingWithModeratePollInterval(t *testing.T) { + s := newPollingStrategy(10*time.Minute, 5*time.Minute) + + tests := []struct { + n int + lowerBound, upperBound time.Duration + }{ + {1, 10 * time.Minute, 10 * time.Minute}, // T=10m, jitter [0, 5m], wait ∈ [5m, 10m], floor 10m + {2, 10 * time.Minute, 20 * time.Minute}, // T=20m, wait ∈ [10m, 20m] + {3, 20 * time.Minute, 40 * time.Minute}, // T=40m, wait ∈ [20m, 40m] + {4, 30 * time.Minute, time.Hour}, // T=80m -> capped to 1h, wait ∈ [30m, 60m] + {5, 30 * time.Minute, time.Hour}, // still capped + } + for _, tc := range tests { + s.OnFailure(FailureClassUnexpected) + w := s.NextWait() + assert.GreaterOrEqual(t, w, tc.lowerBound, "n=%d lower bound", tc.n) + assert.LessOrEqual(t, w, tc.upperBound, "n=%d upper bound", tc.n) + } + + // Confirm the delay bounds after transition: + assert.Equal(t, 10*time.Minute, s.initialDelay, "initialDelay clamped up to PollInterval") + assert.Equal(t, time.Hour, s.maxDelay, "maxDelay at extended ceiling") +} + +// Case C: PollInterval > extendedPollMaxDelay. Both initialDelay and maxDelay +// are clamped up to PollInterval, so the entire delay range collapses to +// PollInterval and the doubling formula produces no observable variation. +// Extended regime is behaviorally identical to normal regime for this config; +// the transition-log signal still fires but the delay never changes. +func TestPollingStrategy_ExtendedRegimeCollapsesWhenPollIntervalExceedsExtendedCeiling(t *testing.T) { + s := newPollingStrategy(2*time.Hour, 5*time.Minute) + + // Multiple unexpected failures -- every wait must be exactly PollInterval. + for i := 1; i <= 6; i++ { + s.OnFailure(FailureClassUnexpected) + assert.Equal(t, 2*time.Hour, s.NextWait(), + "failure #%d: wait must collapse to PollInterval when PollInterval > extendedPollMaxDelay", i) + } + + // Both bounds clamped up to PollInterval. + assert.Equal(t, 2*time.Hour, s.initialDelay, "initialDelay clamped up to PollInterval") + assert.Equal(t, 2*time.Hour, s.maxDelay, "maxDelay clamped up above extendedPollMaxDelay to PollInterval") +} diff --git a/internal/datasource/streaming_data_source.go b/internal/datasource/streaming_data_source.go index c60bf52e..cdfacc8c 100644 --- a/internal/datasource/streaming_data_source.go +++ b/internal/datasource/streaming_data_source.go @@ -1,6 +1,7 @@ package datasource import ( + gocontext "context" "net/http" "net/url" "sync" @@ -83,7 +84,8 @@ type StreamProcessor struct { diagnosticsManager *ldevents.DiagnosticsManager loggers ldlog.Loggers isInitialized internal.AtomicBoolean - halt chan struct{} + streamReqCtx gocontext.Context + streamReqCancel gocontext.CancelFunc storeStatusCh <-chan interfaces.DataStoreStatus connectionAttemptStartTime ldtime.UnixMillisecondTime connectionAttemptLock sync.Mutex @@ -97,11 +99,13 @@ func NewStreamProcessor( dataSourceUpdates subsystems.DataSourceUpdateSink, cfg StreamConfig, ) *StreamProcessor { + streamReqCtx, streamReqCancel := gocontext.WithCancel(gocontext.Background()) sp := &StreamProcessor{ dataSourceUpdates: dataSourceUpdates, headers: context.GetHTTP().DefaultHeaders, loggers: context.GetLogging().Loggers, - halt: make(chan struct{}), + streamReqCtx: streamReqCtx, + streamReqCancel: streamReqCancel, cfg: cfg, } if cci, ok := context.(*internal.ClientContextImpl); ok { @@ -147,11 +151,11 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, closeWhenReady chan< select { case event, ok := <-stream.Events: if !ok { - // COVERAGE: stream.Events is only closed if the EventSource has been closed. However, that - // only happens when we have received from sp.halt, in which case we return immediately - // after calling stream.Close(), terminating the for loop-- so we should not actually reach - // this point. Still, in case the channel is somehow closed unexpectedly, we do want to - // terminate the loop. + // COVERAGE: stream.Events is only closed if the EventSource has been closed. That + // happens after sp.streamReqCtx is cancelled (via the AfterFunc bridge in eventsource), + // but our own sp.streamReqCtx.Done() arm below returns first-- so we should not + // actually reach this point. Still, in case the channel is somehow closed unexpectedly, + // we do want to terminate the loop. return } sp.logConnectionResult(true) @@ -264,15 +268,14 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, closeWhenReady chan< sp.setInitializedAndNotifyClient(true, closeWhenReady) } - case <-sp.halt: - stream.Close() + case <-sp.streamReqCtx.Done(): return } } } func (sp *StreamProcessor) subscribe(closeWhenReady chan<- struct{}) { - req, reqErr := http.NewRequest("GET", endpoints.AddPath(sp.cfg.URI, endpoints.StreamingRequestPath), nil) + req, reqErr := http.NewRequestWithContext(sp.streamReqCtx, "GET", endpoints.AddPath(sp.cfg.URI, endpoints.StreamingRequestPath), nil) if reqErr != nil { sp.loggers.Errorf( "Unable to create a stream request; this is not a network problem, most likely a bad base URI: %s", @@ -324,6 +327,13 @@ func (sp *StreamProcessor) subscribe(closeWhenReady chan<- struct{}) { es.RetryProfileJitter(streamJitterRatio), ) + // loggedActivatedExtended gates the "engaging extended backoff" info log so + // it fires at most once per Subscribe cycle. The library handles the + // healthy-op reset back to normal internally (via + // StreamOptionRetryResetInterval); we don't observe it, so we don't + // re-log if the SDK re-transitions during a long-lived stream. + loggedActivatedExtended := false + errorHandler := func(err error) es.StreamErrorHandlerResult { sp.logConnectionResult(false) @@ -366,6 +376,10 @@ func (sp *StreamProcessor) subscribe(closeWhenReady chan<- struct{}) { // connection) reverts. result := es.StreamErrorHandlerResult{CloseNow: false} if class == FailureClassUnexpected { + if !loggedActivatedExtended { + sp.loggers.Info("Classified failure as UNEXPECTED; engaging extended backoff.") + loggedActivatedExtended = true + } result.ActivateProfile = extendedProfile } return result @@ -425,7 +439,7 @@ func (sp *StreamProcessor) logConnectionResult(success bool) { //nolint:revive // no doc comment for standard method func (sp *StreamProcessor) Close() error { sp.closeOnce.Do(func() { - close(sp.halt) + sp.streamReqCancel() if sp.storeStatusCh != nil { sp.dataSourceUpdates.GetDataStoreStatusProvider().RemoveStatusListener(sp.storeStatusCh) } diff --git a/internal/datasource/streaming_data_source_test.go b/internal/datasource/streaming_data_source_test.go index 9bd8b907..13c83668 100644 --- a/internal/datasource/streaming_data_source_test.go +++ b/internal/datasource/streaming_data_source_test.go @@ -249,7 +249,7 @@ func TestStreamProcessorRecoverableErrorsCauseStreamRestart(t *testing.T) { }) } -// Under the RETRY spec (SDK-2775), 401 / 403 / other 4xx are no longer terminal — +// Under the RETRY spec (SDK-2775), 401 / 403 / other 4xx are no longer terminal -- // they engage an extended-regime backoff but keep retrying indefinitely. The SDK // transitions to Interrupted (not Off) and does not close the initialization // channel. This test replaces the pre-RETRY TestStreamProcessorUnrecoverableErrors @@ -403,7 +403,7 @@ func testStreamProcessorUnexpectedHTTPError(t *testing.T, statusCode int) { // Short retry delays so we can observe at least two attempts within the // assertion window. The extended-regime profile activates immediately on the - // first failure (per the RETRY spec — no grace period for initial-connection + // first failure (per the RETRY spec -- no grace period for initial-connection // unexpected classifications), so we need to shorten ExtendedInitialReconnectDelay // too, not just the normal InitialReconnectDelay. sp := NewStreamProcessor(context, dataSourceUpdates, StreamConfig{ @@ -421,12 +421,12 @@ func testStreamProcessorUnexpectedHTTPError(t *testing.T, statusCode int) { // we just assert the channel stays open. select { case <-closeWhenReady: - t.Fatal("closeWhenReady should not be closed — RETRY §1.2.1 forbids permanent stops on 4xx") + t.Fatal("closeWhenReady should not be closed -- RETRY §1.2.1 forbids permanent stops on 4xx") case <-time.After(time.Second): } // The mock data source updates records the raw UpdateStatus calls made - // by the processor — so we see the Interrupted call the processor tried + // by the processor -- so we see the Interrupted call the processor tried // to make. (The real DataSourceUpdateSinkImpl would clamp it to // Initializing since we never reached Valid; that's tested at the // LDClient-level in the RETRY end-to-end tests. Here we just verify the diff --git a/ldclient_end_to_end_test.go b/ldclient_end_to_end_test.go index 8181799c..610b6c7f 100644 --- a/ldclient_end_to_end_test.go +++ b/ldclient_end_to_end_test.go @@ -128,7 +128,7 @@ func TestClientInStreamingModeWith401KeepsRetrying(t *testing.T) { } client, err := MakeCustomClient(testSdkKey, config, 500*time.Millisecond) - require.Error(t, err) // init timed out — no permanent stop, no successful put either + require.Error(t, err) // init timed out -- no permanent stop, no successful put either require.NotNil(t, client) defer client.Close() @@ -138,7 +138,7 @@ func TestClientInStreamingModeWith401KeepsRetrying(t *testing.T) { // reached a Valid state, the SinkImpl keeps state as Initializing rather // than transitioning to Interrupted (see maybeUpdateStatus's Initializing // clamp), but LastError records the failure. The key assertion is - // "state is NOT Off" — no permanent stop. + // "state is NOT Off" -- no permanent stop. require.Eventually(t, func() bool { s := client.GetDataSourceStatusProvider().GetStatus() return s.LastError.Kind == interfaces.DataSourceErrorKindErrorResponse && @@ -153,7 +153,7 @@ func TestClientInStreamingModeWith401KeepsRetrying(t *testing.T) { value, _ := client.BoolVariation(alwaysTrueFlag.Key, testUser, false) assert.False(t, value) - // Confirm the client kept retrying — at least one additional request landed at the mock server. + // Confirm the client kept retrying -- at least one additional request landed at the mock server. r := <-requestsCh assert.Equal(t, testSdkKey, r.Request.Header.Get("Authorization")) require.Eventually(t, func() bool { return len(requestsCh) >= 1 }, @@ -314,7 +314,7 @@ func TestClientInPollingModeWith401KeepsRetrying(t *testing.T) { // Under RETRY the SDK does not permanently stop on 401. Since we never // reached a Valid state, the SinkImpl keeps state as Initializing rather // than transitioning to Interrupted. The key assertion is "state is NOT - // Off" — no permanent stop — and LastError records the failure. + // Off" -- no permanent stop -- and LastError records the failure. require.Eventually(t, func() bool { s := client.GetDataSourceStatusProvider().GetStatus() return s.LastError.Kind == interfaces.DataSourceErrorKindErrorResponse && @@ -329,7 +329,7 @@ func TestClientInPollingModeWith401KeepsRetrying(t *testing.T) { assert.False(t, value) // Confirm the polling goroutine hit the server. Not asserting a second - // poll here — the polling B1 wait floor is PollInterval (30s default), + // poll here -- the polling B1 wait floor is PollInterval (30s default), // so observing multiple polls at unit-test timescales requires the // internal-constructor pathway used by polling_data_source_test.go. r := <-requestsCh diff --git a/ldcomponents/streaming_data_source_builder.go b/ldcomponents/streaming_data_source_builder.go index 9bf987f5..d33f5be7 100644 --- a/ldcomponents/streaming_data_source_builder.go +++ b/ldcomponents/streaming_data_source_builder.go @@ -72,7 +72,7 @@ func (b *StreamingDataSourceBuilder) InitialReconnectDelay( // StreamingDataSourceBuilderInternal is an internal test-only accessor for a // StreamingDataSourceBuilder. It exposes knobs that are not part of the SDK's -// stable public API and must not be used in production code — the LaunchDarkly +// stable public API and must not be used in production code -- the LaunchDarkly // RETRY conformance test suite is the only intended caller. type StreamingDataSourceBuilderInternal struct{ builder *StreamingDataSourceBuilder } From fba7a6aaed7018cba4e48179950e57298cd261eb Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Mon, 17 Aug 2026 16:25:05 -0400 Subject: [PATCH 07/12] fix(datasource): resolve golangci-lint gosec G118 and lll findings Two lint findings from the previous commit's changes to NewStreamProcessor / subscribe: - gosec G118: streamReqCancel is stored on the returned StreamProcessor and invoked from Close(). gosec's scope-local heuristic doesn't see the cross-scope call. Suppress explicitly with a nolint directive and a justification comment. - lll (line too long): wrap the http.NewRequestWithContext call across four lines to get under the 120-char limit. Local make lint clean across all four modules. --- internal/datasource/streaming_data_source.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/datasource/streaming_data_source.go b/internal/datasource/streaming_data_source.go index cdfacc8c..41028224 100644 --- a/internal/datasource/streaming_data_source.go +++ b/internal/datasource/streaming_data_source.go @@ -99,6 +99,9 @@ func NewStreamProcessor( dataSourceUpdates subsystems.DataSourceUpdateSink, cfg StreamConfig, ) *StreamProcessor { + // streamReqCancel is stored on sp and invoked from Close(); gosec's + // G118 heuristic doesn't see the cross-scope call. + //nolint:gosec // G118: cancel invoked from Close() streamReqCtx, streamReqCancel := gocontext.WithCancel(gocontext.Background()) sp := &StreamProcessor{ dataSourceUpdates: dataSourceUpdates, @@ -275,7 +278,10 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, closeWhenReady chan< } func (sp *StreamProcessor) subscribe(closeWhenReady chan<- struct{}) { - req, reqErr := http.NewRequestWithContext(sp.streamReqCtx, "GET", endpoints.AddPath(sp.cfg.URI, endpoints.StreamingRequestPath), nil) + req, reqErr := http.NewRequestWithContext( + sp.streamReqCtx, "GET", + endpoints.AddPath(sp.cfg.URI, endpoints.StreamingRequestPath), nil, + ) if reqErr != nil { sp.loggers.Errorf( "Unable to create a stream request; this is not a network problem, most likely a bad base URI: %s", From c536a6794d99bd088c6c715132dd9aa6964452c1 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 18 Aug 2026 14:23:54 -0400 Subject: [PATCH 08/12] refactor: drop public API additions from RETRY-conformance work Reviewers rejected exposing extended-regime timing knobs on the SDK's public builders -- those would put a test-harness bypass in the customer-facing surface. Delete every public addition while preserving the RETRY-spec behavior. Removed: - Builder.Internal() accessors and *Internal types - DefaultExtendedInitialReconnectDelay, DefaultRetryResetInterval, DefaultExtendedInitialPollInterval public constants - testhelpers/datasourcetest/ package - Servicedef ExtendedInitialDelayMS / ResetThresholdMS fields (matches sdk-test-harness PR #404 refactor) Kept: full RETRY behavior (401/403/other-4xx no longer terminal; extended regime engages), all defaults unchanged (5-min extended-initial, 60s activeSince reset). Contract test capabilities retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling remain declared. Added: defaultExtendedInitialPollDelay fallback in internal/datasource so removing the public constant doesn't silently drop the 5-min default. Streaming already had the equivalent fallback pre-refactor. E2E tests exercising 401 retry now use a test-local ComponentConfigurer (streaming) or default polling config (polling) instead of the deleted datasourcetest helpers. Also tidied polling_strategy_test.go to reference the new fallback constant instead of scattered 5*time.Minute literals where semantics matched. Validated: full RETRY-conformance suite (13 leaf tests, 9 streaming + 4 polling) passed against the sdk-test-harness PR #404 branch with -enable-long-running-tests at real production 5-minute extended-regime timing. --- internal/datasource/polling_data_source.go | 11 ++- internal/datasource/polling_strategy_test.go | 68 +++++++++---------- ldclient_end_to_end_test.go | 35 ++++++++-- ldcomponents/polling_data_source_builder.go | 45 ++---------- ldcomponents/streaming_data_source_builder.go | 67 ++---------------- testhelpers/datasourcetest/datasourcetest.go | 50 -------------- testservice/sdk_client_entity.go | 13 ---- testservice/servicedef/sdk_config.go | 15 ++-- 8 files changed, 91 insertions(+), 213 deletions(-) delete mode 100644 testhelpers/datasourcetest/datasourcetest.go diff --git a/internal/datasource/polling_data_source.go b/internal/datasource/polling_data_source.go index 23cc5ecf..8e029524 100644 --- a/internal/datasource/polling_data_source.go +++ b/internal/datasource/polling_data_source.go @@ -14,8 +14,9 @@ import ( ) const ( - pollingErrorContext = "on polling request" - pollingWillRetryMessage = "will retry at next scheduled poll interval" + pollingErrorContext = "on polling request" + pollingWillRetryMessage = "will retry at next scheduled poll interval" + defaultExtendedInitialPollDelay = 5 * time.Minute ) // PollingConfig describes the configuration for a polling data source. It is exported so that @@ -59,9 +60,13 @@ func NewPollingProcessor( cfg PollingConfig, ) *PollingProcessor { httpRequester := NewPollingRequester(context, context.GetHTTP().CreateHTTPClient(), cfg.BaseURI, cfg.FilterKey) + extendedInitialPollInterval := cfg.ExtendedInitialPollInterval + if extendedInitialPollInterval <= 0 { + extendedInitialPollInterval = defaultExtendedInitialPollDelay + } return newPollingProcessor( context, dataSourceUpdates, httpRequester, - cfg.PollInterval, cfg.ExtendedInitialPollInterval, + cfg.PollInterval, extendedInitialPollInterval, ) } diff --git a/internal/datasource/polling_strategy_test.go b/internal/datasource/polling_strategy_test.go index ab9962d4..741a9ab9 100644 --- a/internal/datasource/polling_strategy_test.go +++ b/internal/datasource/polling_strategy_test.go @@ -9,14 +9,14 @@ import ( // In the normal regime (no failures observed), NextWait returns exactly PollInterval. func TestPollingStrategy_NormalRegimeReturnsPollInterval(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) assert.Equal(t, 30*time.Second, s.NextWait()) } // A normal-classified failure advances n but does not engage // the extended regime. Wait is still pinned to PollInterval by the wait floor. func TestPollingStrategy_NormalFailureDoesNotEngageExtended(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) s.OnFailure(FailureClassNormal) assert.Equal(t, 1, s.n) @@ -28,10 +28,10 @@ func TestPollingStrategy_NormalFailureDoesNotEngageExtended(t *testing.T) { // An unexpected-classified failure engages the extended regime: initialDelay // becomes the configured extended base and maxDelay becomes the RETRY-spec cap. func TestPollingStrategy_UnexpectedFailureEngagesExtended(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) s.OnFailure(FailureClassUnexpected) - assert.Equal(t, 5*time.Minute, s.initialDelay) + assert.Equal(t, defaultExtendedInitialPollDelay, s.initialDelay) assert.Equal(t, time.Hour, s.maxDelay) } @@ -40,7 +40,7 @@ func TestPollingStrategy_UnexpectedFailureEngagesExtended(t *testing.T) { // extended regime uses PollInterval as its base. Result: no observable // differentiation between regimes. func TestPollingStrategy_ExtendedInitialClampedToPollInterval(t *testing.T) { - s := newPollingStrategy(time.Hour, 5*time.Minute) + s := newPollingStrategy(time.Hour, defaultExtendedInitialPollDelay) s.OnFailure(FailureClassUnexpected) assert.Equal(t, time.Hour, s.initialDelay) @@ -59,7 +59,7 @@ func TestPollingStrategy_ExtendedInitialClampedToPollInterval(t *testing.T) { // sequence of "normal, normal, unexpected" would inflate the first extended // wait to 20min or more (bounded by extendedPollMaxDelay). func TestPollingStrategy_UnexpectedAfterNormalFailuresStartsAtInitialDelay(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) // Prior normal failures accumulate. Normal-regime NextWait is bounded by // normalInterval regardless of n, so these aren't observable in delay -- @@ -74,14 +74,14 @@ func TestPollingStrategy_UnexpectedAfterNormalFailuresStartsAtInitialDelay(t *te // initialDelay * 2^0 = initialDelay = 5min. s.OnFailure(FailureClassUnexpected) assert.Equal(t, 1, s.n, "n must reset to 1 on transition into extended regime") - assert.Equal(t, 5*time.Minute, s.initialDelay, "extended regime initialDelay engaged") + assert.Equal(t, defaultExtendedInitialPollDelay, s.initialDelay, "extended regime initialDelay engaged") assert.Equal(t, time.Hour, s.maxDelay, "extended regime maxDelay engaged") // First extended-regime wait: T = 5min * 2^0 = 5min, minus jitter in // [0, T/2]. Actual wait is in [2.5min, 5min]. w := s.NextWait() assert.GreaterOrEqual(t, w, 2*time.Minute+30*time.Second) - assert.LessOrEqual(t, w, 5*time.Minute) + assert.LessOrEqual(t, w, defaultExtendedInitialPollDelay) } // Once in the extended regime, subsequent unexpected failures continue the @@ -89,7 +89,7 @@ func TestPollingStrategy_UnexpectedAfterNormalFailuresStartsAtInitialDelay(t *te // reset-on-transition only fires on the first crossing from normal into // extended, gated by the inExtended flag. func TestPollingStrategy_UnexpectedWhileAlreadyExtendedContinuesDoubling(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) // Enter extended regime. s.OnFailure(FailureClassUnexpected) @@ -99,14 +99,14 @@ func TestPollingStrategy_UnexpectedWhileAlreadyExtendedContinuesDoubling(t *test // initialDelay/maxDelay stay at extended values. s.OnFailure(FailureClassUnexpected) assert.Equal(t, 2, s.n, "second unexpected in extended increments, does not reset") - assert.Equal(t, 5*time.Minute, s.initialDelay) + assert.Equal(t, defaultExtendedInitialPollDelay, s.initialDelay) assert.Equal(t, time.Hour, s.maxDelay) // A normal failure while in extended also advances n without // changing regime. s.OnFailure(FailureClassNormal) assert.Equal(t, 3, s.n, "normal failure in extended increments n") - assert.Equal(t, 5*time.Minute, s.initialDelay, "normal failure does not exit extended regime") + assert.Equal(t, defaultExtendedInitialPollDelay, s.initialDelay, "normal failure does not exit extended regime") assert.Equal(t, time.Hour, s.maxDelay) } @@ -114,18 +114,18 @@ func TestPollingStrategy_UnexpectedWhileAlreadyExtendedContinuesDoubling(t *test // extendedPollMaxDelay (1 hour). Jitter subtracts up to T/2, so each wait falls // in [T/2, T] before the (in these cases irrelevant) wait floor. func TestPollingStrategy_ExtendedDoubling(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) tests := []struct { n int lowerBound, upperBound time.Duration }{ - {1, 2*time.Minute + 30*time.Second, 5 * time.Minute}, // T = 5m - {2, 5 * time.Minute, 10 * time.Minute}, // T = 10m - {3, 10 * time.Minute, 20 * time.Minute}, // T = 20m - {4, 20 * time.Minute, 40 * time.Minute}, // T = 40m - {5, 30 * time.Minute, time.Hour}, // T capped at 60m - {6, 30 * time.Minute, time.Hour}, // still capped + {1, 2*time.Minute + 30*time.Second, defaultExtendedInitialPollDelay}, // T = 5m + {2, 5 * time.Minute, 10 * time.Minute}, // T = 10m + {3, 10 * time.Minute, 20 * time.Minute}, // T = 20m + {4, 20 * time.Minute, 40 * time.Minute}, // T = 40m + {5, 30 * time.Minute, time.Hour}, // T capped at 60m + {6, 30 * time.Minute, time.Hour}, // still capped } for _, tc := range tests { s.OnFailure(FailureClassUnexpected) @@ -150,19 +150,19 @@ func TestPollingStrategy_WaitFloorAtPollInterval(t *testing.T) { // The 2-consecutive-success reset gate: first success flips the gate flag but // does not clear n or exit the extended regime. func TestPollingStrategy_FirstSuccessDoesNotReset(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) s.OnFailure(FailureClassUnexpected) s.OnSuccess() assert.True(t, s.priorPollWasSuccessful, "reset gate should be armed") assert.Equal(t, 1, s.n, "one success must not reset n") - assert.Equal(t, 5*time.Minute, s.initialDelay, "one success must not exit extended regime") + assert.Equal(t, defaultExtendedInitialPollDelay, s.initialDelay, "one success must not exit extended regime") } // Two consecutive successes clear n and return the strategy to the // normal regime (RETRY §1.8 polling binding). func TestPollingStrategy_TwoConsecutiveSuccessesReset(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) s.OnFailure(FailureClassUnexpected) s.OnSuccess() s.OnSuccess() @@ -177,7 +177,7 @@ func TestPollingStrategy_TwoConsecutiveSuccessesReset(t *testing.T) { // STRICTLY consecutive successes, so a first-then-fail-then-first pattern does // not fire the reset. func TestPollingStrategy_FailureClearsResetGate(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) s.OnFailure(FailureClassUnexpected) s.OnSuccess() // gate armed s.OnFailure(FailureClassNormal) // gate cleared, n=2 @@ -190,7 +190,7 @@ func TestPollingStrategy_FailureClearsResetGate(t *testing.T) { // A single normal failure after a reset does not re-engage extended-regime // parameters -- extended engagement requires an Unexpected classification. func TestPollingStrategy_NormalFailureAfterResetStaysNormal(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) // Engage extended, then reset back to normal. s.OnFailure(FailureClassUnexpected) s.OnSuccess() @@ -210,7 +210,7 @@ func TestPollingStrategy_NormalFailureAfterResetStaysNormal(t *testing.T) { // transition path and resets n to 1, and RETRY spec 1.4.1's doubling never // engages. Explicit regime state (inExtended) avoids the clamp collision. func TestPollingStrategy_ExtendedDoublingWhenClampedToPollInterval(t *testing.T) { - s := newPollingStrategy(5*time.Minute, 5*time.Minute) + s := newPollingStrategy(5*time.Minute, defaultExtendedInitialPollDelay) // Drive five unexpected failures; n must advance monotonically. for i, expectedN := range []int{1, 2, 3, 4, 5} { @@ -232,7 +232,7 @@ func TestPollingStrategy_ExtendedDoublingWhenClampedToPollInterval(t *testing.T) // subsequent unexpected failure. The caller uses this signal to log // "engaging extended backoff" exactly once per transition. func TestPollingStrategy_OnFailureReturnsTrueOnceOnTransition(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) assert.True(t, s.OnFailure(FailureClassUnexpected), "first unexpected must signal transition") assert.False(t, s.OnFailure(FailureClassUnexpected), "second unexpected must not re-signal transition") @@ -243,7 +243,7 @@ func TestPollingStrategy_OnFailureReturnsTrueOnceOnTransition(t *testing.T) { // Also test the clamped-config variant, since that's the specific bug scenario // where equality-based detection re-signalled transition on every unexpected. func TestPollingStrategy_OnFailureReturnsTrueOnceOnTransitionWhenClamped(t *testing.T) { - s := newPollingStrategy(5*time.Minute, 5*time.Minute) + s := newPollingStrategy(5*time.Minute, defaultExtendedInitialPollDelay) assert.True(t, s.OnFailure(FailureClassUnexpected), "first unexpected must signal transition") assert.False(t, s.OnFailure(FailureClassUnexpected), "second unexpected must not re-signal transition") @@ -254,7 +254,7 @@ func TestPollingStrategy_OnFailureReturnsTrueOnceOnTransitionWhenClamped(t *test // back in the normal regime and a subsequent unexpected failure is treated // as a new transition, signalling to the caller for a fresh log line. func TestPollingStrategy_OnSuccessResetAllowsRetransition(t *testing.T) { - s := newPollingStrategy(30*time.Second, 5*time.Minute) + s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) assert.True(t, s.OnFailure(FailureClassUnexpected), "initial transition") assert.False(t, s.OnFailure(FailureClassUnexpected), "already in extended") @@ -272,17 +272,17 @@ func TestPollingStrategy_OnSuccessResetAllowsRetransition(t *testing.T) { // extended wait equals PollInterval (via the output floor). Doubling engages // visibly from n=2 and reaches the extended ceiling at n=4. func TestPollingStrategy_ExtendedDoublingWithModeratePollInterval(t *testing.T) { - s := newPollingStrategy(10*time.Minute, 5*time.Minute) + s := newPollingStrategy(10*time.Minute, defaultExtendedInitialPollDelay) tests := []struct { n int lowerBound, upperBound time.Duration }{ - {1, 10 * time.Minute, 10 * time.Minute}, // T=10m, jitter [0, 5m], wait ∈ [5m, 10m], floor 10m - {2, 10 * time.Minute, 20 * time.Minute}, // T=20m, wait ∈ [10m, 20m] - {3, 20 * time.Minute, 40 * time.Minute}, // T=40m, wait ∈ [20m, 40m] - {4, 30 * time.Minute, time.Hour}, // T=80m -> capped to 1h, wait ∈ [30m, 60m] - {5, 30 * time.Minute, time.Hour}, // still capped + {1, 10 * time.Minute, 10 * time.Minute}, // T=10m, jitter [0, 5m], wait ∈ [5m, 10m], floor 10m + {2, 10 * time.Minute, 20 * time.Minute}, // T=20m, wait ∈ [10m, 20m] + {3, 20 * time.Minute, 40 * time.Minute}, // T=40m, wait ∈ [20m, 40m] + {4, 30 * time.Minute, time.Hour}, // T=80m -> capped to 1h, wait ∈ [30m, 60m] + {5, 30 * time.Minute, time.Hour}, // still capped } for _, tc := range tests { s.OnFailure(FailureClassUnexpected) @@ -302,7 +302,7 @@ func TestPollingStrategy_ExtendedDoublingWithModeratePollInterval(t *testing.T) // Extended regime is behaviorally identical to normal regime for this config; // the transition-log signal still fires but the delay never changes. func TestPollingStrategy_ExtendedRegimeCollapsesWhenPollIntervalExceedsExtendedCeiling(t *testing.T) { - s := newPollingStrategy(2*time.Hour, 5*time.Minute) + s := newPollingStrategy(2*time.Hour, defaultExtendedInitialPollDelay) // Multiple unexpected failures -- every wait must be exactly PollInterval. for i := 1; i <= 6; i++ { diff --git a/ldclient_end_to_end_test.go b/ldclient_end_to_end_test.go index 610b6c7f..519301ae 100644 --- a/ldclient_end_to_end_test.go +++ b/ldclient_end_to_end_test.go @@ -15,9 +15,11 @@ import ( "github.com/launchdarkly/go-sdk-common/v3/ldvalue" "github.com/launchdarkly/go-server-sdk-evaluation/v3/ldbuilders" "github.com/launchdarkly/go-server-sdk/v7/interfaces" + "github.com/launchdarkly/go-server-sdk/v7/internal/datasource" + "github.com/launchdarkly/go-server-sdk/v7/internal/endpoints" "github.com/launchdarkly/go-server-sdk/v7/internal/sharedtest" "github.com/launchdarkly/go-server-sdk/v7/ldcomponents" - "github.com/launchdarkly/go-server-sdk/v7/testhelpers/datasourcetest" + "github.com/launchdarkly/go-server-sdk/v7/subsystems" "github.com/launchdarkly/go-server-sdk/v7/testhelpers/ldservices" "github.com/launchdarkly/go-test-helpers/v3/httphelpers" @@ -117,8 +119,12 @@ func TestClientInStreamingModeWith401KeepsRetrying(t *testing.T) { // Short reconnect delays so multiple attempts fit within the init wait window. // Extended-regime profile activates immediately on 401 (unexpected), so we need // to shorten its base too, not just the normal-regime InitialReconnectDelay. - streamingBuilder := ldcomponents.StreamingDataSource().InitialReconnectDelay(10 * time.Millisecond) - datasourcetest.WithStreamingExtendedInitialReconnectDelay(streamingBuilder, 20*time.Millisecond) + // Uses a test-local bypass because the SDK's public streaming builder + // intentionally does not expose the extended-regime timing knobs. + streamingBuilder := &compressedStreamingBuilder{ + initialReconnectDelay: 10 * time.Millisecond, + extendedInitialReconnectDelay: 20 * time.Millisecond, + } config := Config{ Events: ldcomponents.NoEvents(), @@ -295,7 +301,6 @@ func TestClientInPollingModeWith401KeepsRetrying(t *testing.T) { logCapture := ldlogtest.NewMockLog() pollingBuilder := ldcomponents.PollingDataSource() - datasourcetest.WithPollingExtendedInitialPollInterval(pollingBuilder, 20*time.Millisecond) config := Config{ DataSource: pollingBuilder, @@ -449,3 +454,25 @@ func TestClientStartupTimesOut(t *testing.T) { assert.Len(t, logCapture.GetOutput(ldlog.Error), 0) }) } + +// compressedStreamingBuilder is a test-only ComponentConfigurer that constructs +// the streaming data source with compressed retry timings. It bypasses the SDK's +// public builder because that builder intentionally does not expose the +// extended-regime timing knobs. +type compressedStreamingBuilder struct { + initialReconnectDelay time.Duration + extendedInitialReconnectDelay time.Duration +} + +func (b *compressedStreamingBuilder) Build(context subsystems.ClientContext) (subsystems.DataSource, error) { + baseURI := endpoints.SelectBaseURI( + context.GetServiceEndpoints(), + endpoints.StreamingService, + context.GetLogging().Loggers, + ) + return datasource.NewStreamProcessor(context, context.GetDataSourceUpdateSink(), datasource.StreamConfig{ + URI: baseURI, + InitialReconnectDelay: b.initialReconnectDelay, + ExtendedInitialReconnectDelay: b.extendedInitialReconnectDelay, + }), nil +} diff --git a/ldcomponents/polling_data_source_builder.go b/ldcomponents/polling_data_source_builder.go index 775fe8fe..72f4cf3e 100644 --- a/ldcomponents/polling_data_source_builder.go +++ b/ldcomponents/polling_data_source_builder.go @@ -16,17 +16,12 @@ const DefaultPollingBaseURI = "https://app.launchdarkly.com" // DefaultPollInterval is the default value for [PollingDataSourceBuilder.PollInterval]. This is also the minimum value. const DefaultPollInterval = 30 * time.Second -// DefaultExtendedInitialPollInterval is the default value for -// [PollingDataSourceBuilder.ExtendedInitialPollInterval]. -const DefaultExtendedInitialPollInterval = 5 * time.Minute - // PollingDataSourceBuilder provides methods for configuring the polling data source. // // See [PollingDataSource] for usage. type PollingDataSourceBuilder struct { - pollInterval time.Duration - extendedInitialPollInterval time.Duration - filterKey ldvalue.OptionalString + pollInterval time.Duration + filterKey ldvalue.OptionalString } // PollingDataSource returns a configurable factory for using polling mode to get feature flag data. @@ -45,8 +40,7 @@ type PollingDataSourceBuilder struct { // } func PollingDataSource() *PollingDataSourceBuilder { return &PollingDataSourceBuilder{ - pollInterval: DefaultPollInterval, - extendedInitialPollInterval: DefaultExtendedInitialPollInterval, + pollInterval: DefaultPollInterval, } } @@ -72,32 +66,6 @@ func (b *PollingDataSourceBuilder) forcePollInterval( return b } -// PollingDataSourceBuilderInternal is an internal test-only accessor for a -// PollingDataSourceBuilder. It exposes knobs that are not part of the SDK's -// stable public API and must not be used in production code. -type PollingDataSourceBuilderInternal struct{ builder *PollingDataSourceBuilder } - -// Internal returns a test-only accessor for setting fields not exposed on the -// public builder surface. This is not part of the SDK's stable public API and -// must not be used in production code. -func (b *PollingDataSourceBuilder) Internal() PollingDataSourceBuilderInternal { - return PollingDataSourceBuilderInternal{builder: b} -} - -// ExtendedInitialPollInterval sets the base delay for the extended-regime backoff -// that engages after RETRY-classified unexpected failures (401, 403, TLS/cert). -// Values ≤ 0 are clamped to [DefaultExtendedInitialPollInterval]. -func (i PollingDataSourceBuilderInternal) ExtendedInitialPollInterval( - delay time.Duration, -) PollingDataSourceBuilderInternal { - if delay <= 0 { - i.builder.extendedInitialPollInterval = DefaultExtendedInitialPollInterval - } else { - i.builder.extendedInitialPollInterval = delay - } - return i -} - // PayloadFilter sets the filter key for the polling connection. // // By default, the SDK is able to evaluate all flags in an environment. If this is undesirable - @@ -124,10 +92,9 @@ func (b *PollingDataSourceBuilder) Build(context subsystems.ClientContext) (subs context.GetLogging().Loggers, ) cfg := datasource.PollingConfig{ - BaseURI: configuredBaseURI, - PollInterval: b.pollInterval, - ExtendedInitialPollInterval: b.extendedInitialPollInterval, - FilterKey: filterKey, + BaseURI: configuredBaseURI, + PollInterval: b.pollInterval, + FilterKey: filterKey, } pp := datasource.NewPollingProcessor(context, context.GetDataSourceUpdateSink(), cfg) return pp, nil diff --git a/ldcomponents/streaming_data_source_builder.go b/ldcomponents/streaming_data_source_builder.go index d33f5be7..ade67b04 100644 --- a/ldcomponents/streaming_data_source_builder.go +++ b/ldcomponents/streaming_data_source_builder.go @@ -16,22 +16,12 @@ const DefaultStreamingBaseURI = endpoints.DefaultStreamingBaseURI // DefaultInitialReconnectDelay is the default value for [StreamingDataSourceBuilder.InitialReconnectDelay]. const DefaultInitialReconnectDelay = time.Second -// DefaultExtendedInitialReconnectDelay is the default value for -// [StreamingDataSourceBuilder.ExtendedInitialReconnectDelay]. -const DefaultExtendedInitialReconnectDelay = 5 * time.Minute - -// DefaultRetryResetInterval is the default value for -// [StreamingDataSourceBuilder.RetryResetInterval]. -const DefaultRetryResetInterval = 60 * time.Second - // StreamingDataSourceBuilder provides methods for configuring the streaming data source. // // See StreamingDataSource for usage. type StreamingDataSourceBuilder struct { - initialReconnectDelay time.Duration - extendedInitialReconnectDelay time.Duration - retryResetInterval time.Duration - filterKey ldvalue.OptionalString + initialReconnectDelay time.Duration + filterKey ldvalue.OptionalString } // StreamingDataSource returns a configurable factory for using streaming mode to get feature flag data. @@ -46,9 +36,7 @@ type StreamingDataSourceBuilder struct { // } func StreamingDataSource() *StreamingDataSourceBuilder { return &StreamingDataSourceBuilder{ - initialReconnectDelay: DefaultInitialReconnectDelay, - extendedInitialReconnectDelay: DefaultExtendedInitialReconnectDelay, - retryResetInterval: DefaultRetryResetInterval, + initialReconnectDelay: DefaultInitialReconnectDelay, } } @@ -70,47 +58,6 @@ func (b *StreamingDataSourceBuilder) InitialReconnectDelay( return b } -// StreamingDataSourceBuilderInternal is an internal test-only accessor for a -// StreamingDataSourceBuilder. It exposes knobs that are not part of the SDK's -// stable public API and must not be used in production code -- the LaunchDarkly -// RETRY conformance test suite is the only intended caller. -type StreamingDataSourceBuilderInternal struct{ builder *StreamingDataSourceBuilder } - -// Internal returns a test-only accessor for setting fields not exposed on the -// public builder surface. This is not part of the SDK's stable public API and -// must not be used in production code. -func (b *StreamingDataSourceBuilder) Internal() StreamingDataSourceBuilderInternal { - return StreamingDataSourceBuilderInternal{builder: b} -} - -// ExtendedInitialReconnectDelay sets the base delay for the extended-regime retry -// profile that engages after RETRY-classified unexpected failures (401, 403, TLS/cert). -// Values ≤ 0 are clamped to [DefaultExtendedInitialReconnectDelay]. -func (i StreamingDataSourceBuilderInternal) ExtendedInitialReconnectDelay( - delay time.Duration, -) StreamingDataSourceBuilderInternal { - if delay <= 0 { - i.builder.extendedInitialReconnectDelay = DefaultExtendedInitialReconnectDelay - } else { - i.builder.extendedInitialReconnectDelay = delay - } - return i -} - -// RetryResetInterval sets the threshold of continuous healthy stream operation -// before the SDK resets its retry backoff to the normal regime. -// Values ≤ 0 are clamped to [DefaultRetryResetInterval]. -func (i StreamingDataSourceBuilderInternal) RetryResetInterval( - interval time.Duration, -) StreamingDataSourceBuilderInternal { - if interval <= 0 { - i.builder.retryResetInterval = DefaultRetryResetInterval - } else { - i.builder.retryResetInterval = interval - } - return i -} - // PayloadFilter sets the payload filter key for this streaming connection. The filter key // cannot be an empty string. // @@ -136,11 +83,9 @@ func (b *StreamingDataSourceBuilder) Build(context subsystems.ClientContext) (su context.GetLogging().Loggers, ) cfg := datasource.StreamConfig{ - URI: configuredBaseURI, - InitialReconnectDelay: b.initialReconnectDelay, - ExtendedInitialReconnectDelay: b.extendedInitialReconnectDelay, - RetryResetInterval: b.retryResetInterval, - FilterKey: filterKey, + URI: configuredBaseURI, + InitialReconnectDelay: b.initialReconnectDelay, + FilterKey: filterKey, } return datasource.NewStreamProcessor( context, diff --git a/testhelpers/datasourcetest/datasourcetest.go b/testhelpers/datasourcetest/datasourcetest.go deleted file mode 100644 index ed356b25..00000000 --- a/testhelpers/datasourcetest/datasourcetest.go +++ /dev/null @@ -1,50 +0,0 @@ -// Package datasourcetest provides test-only helpers for configuring knobs on the -// SDK's streaming and polling data source builders that are not part of the -// SDK's stable public API. It is intended for LaunchDarkly's own contract-test -// tooling and for SDK integration tests that need to observe extended-regime -// behavior within a test-relevant time budget. -// -// Production code must not import this package. -// -// This package is a companion to the Internal() escape hatches on the builders -// in ldcomponents. Adding a new test-only knob to a builder means: -// 1. Add the field and the setter to the Internal type in ldcomponents. -// 2. Add a corresponding free-function helper here that delegates to it. -package datasourcetest - -import ( - "time" - - "github.com/launchdarkly/go-server-sdk/v7/ldcomponents" -) - -// WithStreamingExtendedInitialReconnectDelay overrides the RETRY-spec default -// base delay for the streaming extended-regime retry profile. Test-only. -func WithStreamingExtendedInitialReconnectDelay( - b *ldcomponents.StreamingDataSourceBuilder, - delay time.Duration, -) *ldcomponents.StreamingDataSourceBuilder { - b.Internal().ExtendedInitialReconnectDelay(delay) - return b -} - -// WithStreamingRetryResetInterval overrides the threshold of continuous healthy -// stream operation before the SDK resets its retry backoff to the normal regime. -// Test-only. -func WithStreamingRetryResetInterval( - b *ldcomponents.StreamingDataSourceBuilder, - interval time.Duration, -) *ldcomponents.StreamingDataSourceBuilder { - b.Internal().RetryResetInterval(interval) - return b -} - -// WithPollingExtendedInitialPollInterval overrides the RETRY-spec default base -// delay for the polling extended-regime backoff. Test-only. -func WithPollingExtendedInitialPollInterval( - b *ldcomponents.PollingDataSourceBuilder, - delay time.Duration, -) *ldcomponents.PollingDataSourceBuilder { - b.Internal().ExtendedInitialPollInterval(delay) - return b -} diff --git a/testservice/sdk_client_entity.go b/testservice/sdk_client_entity.go index b7761b6d..4d59b516 100644 --- a/testservice/sdk_client_entity.go +++ b/testservice/sdk_client_entity.go @@ -27,7 +27,6 @@ import ( "github.com/launchdarkly/go-server-sdk/v7/ldcomponents" "github.com/launchdarkly/go-server-sdk/v7/ldhooks" "github.com/launchdarkly/go-server-sdk/v7/subsystems" - "github.com/launchdarkly/go-server-sdk/v7/testhelpers/datasourcetest" "github.com/launchdarkly/go-server-sdk/v7/testservice/servicedef" "github.com/launchdarkly/go-sdk-common/v3/ldcontext" @@ -508,14 +507,6 @@ func makeSDKConfig(config servicedef.SDKConfigParams, sdkLog ldlog.Loggers) (ld. if config.Streaming.InitialRetryDelayMS != nil { builder.InitialReconnectDelay(time.Millisecond * time.Duration(*config.Streaming.InitialRetryDelayMS)) } - if config.Streaming.ExtendedInitialDelayMS != nil { - datasourcetest.WithStreamingExtendedInitialReconnectDelay(builder, - time.Millisecond*time.Duration(*config.Streaming.ExtendedInitialDelayMS)) - } - if config.Streaming.ResetThresholdMS != nil { - datasourcetest.WithStreamingRetryResetInterval(builder, - time.Millisecond*time.Duration(*config.Streaming.ResetThresholdMS)) - } if config.Streaming.Filter.IsDefined() { builder.PayloadFilter(config.Streaming.Filter.String()) } @@ -528,10 +519,6 @@ func makeSDKConfig(config servicedef.SDKConfigParams, sdkLog ldlog.Loggers) (ld. if config.Polling.PollIntervalMS != nil { builder.PollInterval(time.Millisecond * time.Duration(*config.Polling.PollIntervalMS)) } - if config.Polling.ExtendedInitialDelayMS != nil { - datasourcetest.WithPollingExtendedInitialPollInterval(builder, - time.Millisecond*time.Duration(*config.Polling.ExtendedInitialDelayMS)) - } if config.Polling.Filter.IsDefined() { builder.PayloadFilter(config.Polling.Filter.String()) } diff --git a/testservice/servicedef/sdk_config.go b/testservice/servicedef/sdk_config.go index 7eb4f5c1..d8d2716f 100644 --- a/testservice/servicedef/sdk_config.go +++ b/testservice/servicedef/sdk_config.go @@ -62,18 +62,15 @@ type Synchronizer struct { } type SDKConfigStreamingParams struct { - BaseURI string `json:"baseUri,omitempty"` - InitialRetryDelayMS *ldtime.UnixMillisecondTime `json:"initialRetryDelayMs,omitempty"` - ExtendedInitialDelayMS *ldtime.UnixMillisecondTime `json:"extendedInitialDelayMs,omitempty"` - ResetThresholdMS *ldtime.UnixMillisecondTime `json:"resetThresholdMs,omitempty"` - Filter ldvalue.OptionalString `json:"filter,omitempty"` + BaseURI string `json:"baseUri,omitempty"` + InitialRetryDelayMS *ldtime.UnixMillisecondTime `json:"initialRetryDelayMs,omitempty"` + Filter ldvalue.OptionalString `json:"filter,omitempty"` } type SDKConfigPollingParams struct { - BaseURI string `json:"baseUri,omitempty"` - PollIntervalMS *ldtime.UnixMillisecondTime `json:"pollIntervalMs,omitempty"` - ExtendedInitialDelayMS *ldtime.UnixMillisecondTime `json:"extendedInitialDelayMs,omitempty"` - Filter ldvalue.OptionalString `json:"filter,omitempty"` + BaseURI string `json:"baseUri,omitempty"` + PollIntervalMS *ldtime.UnixMillisecondTime `json:"pollIntervalMs,omitempty"` + Filter ldvalue.OptionalString `json:"filter,omitempty"` } type SDKConfigEventParams struct { From c355d5eb014cc58a428b025b6da15a32db382643 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 18 Aug 2026 16:31:31 -0400 Subject: [PATCH 09/12] build(deps): bump eventsource to v1.14.0 eventsource v1.14.0 (via launchdarkly/eventsource#71, released as #72) teaches Subscribe to observe the HTTP request's context as a stream- lifetime cancel signal. StreamProcessor.Close already cancels its streamReqCtx, so this consumption makes Close interrupt any in-flight retry timer instead of blocking until the timer fires -- addressing the "Shutdown skips explicit stream close" Cursor Bugbot finding on this PR. No SDK code change is needed: the existing streamReqCtx cancellation now propagates through eventsource's new context observer. --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 95df1af2..d5e0e866 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/google/uuid v1.1.1 github.com/gregjones/httpcache v0.0.0-20171119193500-2bcd89a1743f github.com/launchdarkly/ccache v1.1.0 - github.com/launchdarkly/eventsource v1.13.0 + github.com/launchdarkly/eventsource v1.14.0 github.com/launchdarkly/go-jsonstream/v3 v3.1.2 github.com/launchdarkly/go-ntlm-proxy-auth v1.0.3 github.com/launchdarkly/go-sdk-common/v3 v3.5.1 diff --git a/go.sum b/go.sum index c73efb4c..64aff968 100644 --- a/go.sum +++ b/go.sum @@ -17,8 +17,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/launchdarkly/ccache v1.1.0 h1:voD1M+ZJXR3MREOKtBwgTF9hYHl1jg+vFKS/+VAkR2k= github.com/launchdarkly/ccache v1.1.0/go.mod h1:TlxzrlnzvYeXiLHmesMuvoZetu4Z97cV1SsdqqBJi1Q= -github.com/launchdarkly/eventsource v1.13.0 h1:SjC1LgNSR+ip8BgZcphGa/ar6N0I+pFTYQ/1ZzDnP98= -github.com/launchdarkly/eventsource v1.13.0/go.mod h1:dU+rZxkPOlGPsyJPpiDqiepAcFwIITDUClY9+A6RrMw= +github.com/launchdarkly/eventsource v1.14.0 h1:6lxVkUwxCAEbBrx8J/N9i25yOajC6NNwq0Voo+fT0Kc= +github.com/launchdarkly/eventsource v1.14.0/go.mod h1:dU+rZxkPOlGPsyJPpiDqiepAcFwIITDUClY9+A6RrMw= github.com/launchdarkly/go-jsonstream/v3 v3.1.2 h1:Od0QEKHesbKnD3GgAt+LkZzhcUiSP+yhrclrhsoox6w= github.com/launchdarkly/go-jsonstream/v3 v3.1.2/go.mod h1:ZBjhKq8mhArCtqotGRGnteY6eXpNm1GaOdUSZHh+ZjM= github.com/launchdarkly/go-ntlm-proxy-auth v1.0.3 h1:i3V0N+R0Fd2nXfGEVKCBIZ8kyttZ+SRKvBG8cdcphO4= From 5d760369cbad2bd4f9414d68b52a75f54dd818b4 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Wed, 19 Aug 2026 15:05:38 -0400 Subject: [PATCH 10/12] docs, log: clean up RETRY API docs and restore Error level for unexpected failures The RETRY refactor changed SDK behavior (keep retrying instead of stopping) but not the semantic meaning of unexpected errors -- 401/403 and TLS/cert failures still almost always indicate a real customer-side misconfiguration requiring operator attention. Restore pre-RETRY loudness for those: - classifyAndLogHTTPFailure and classifyAndLogTransportFailure now log at Error for Unexpected classification, Warn for Normal (was Warn for all). - httpErrorDescription: restore "(invalid SDK key)" for 401/403 (was "(authentication failed)" post-refactor). Documentation updates: refresh docstrings on ErrInitializationFailed, MakeClient, MakeCustomClient, dataSystem.Start, DataSourceStateInitializing, DataSourceStateOff, DataSourceStatus.StateSince (Off case), UpdateStatus on both DataSourceUpdateSink and DataSourceStatusReporter, and the FDv1 streaming file-header comment. All reflect that HTTP-level failures no longer permanently stop the data source, and DataSourceStateOff is now reached only via explicit shutdown or unrecoverable startup configuration errors. --- interfaces/data_source_status_provider.go | 12 ++++---- internal/datasource/helpers.go | 32 +++++++++++++++----- internal/datasource/helpers_test.go | 4 +-- internal/datasource/streaming_data_source.go | 8 +++-- ldclient.go | 13 ++++---- subsystems/data_source_status_reporter.go | 4 +-- subsystems/data_source_update_sink.go | 6 ++-- 7 files changed, 49 insertions(+), 30 deletions(-) diff --git a/interfaces/data_source_status_provider.go b/interfaces/data_source_status_provider.go index 999e2f20..1d0c6b38 100644 --- a/interfaces/data_source_status_provider.go +++ b/interfaces/data_source_status_provider.go @@ -109,8 +109,9 @@ type DataSourceStatus struct { // state, after previously having been either Initializing or Interrupted. // - For DataSourceStateInterrupted, it is the time that the data source most recently entered an // error state, after previously having been Valid. - // - For DataSourceStateOff, it is the time that the data source encountered an unrecoverable error - // or that the SDK was explicitly shut down. + // - For DataSourceStateOff, it is the time that the SDK was explicitly shut down, or (rarely) + // the time that the data source encountered a startup configuration error that prevented any + // connection attempt. StateSince time.Time // LastError is information about the last error that the data source encountered, if any. @@ -141,7 +142,7 @@ const ( // initialized. // // If it encounters an error that requires it to retry initialization, the state will remain at - // Initializing until it either succeeds and becomes DataSourceStateValid, or permanently fails and + // Initializing until it either succeeds and becomes DataSourceStateValid, or permanently stops and // becomes DataSourceStateOff. DataSourceStateInitializing DataSourceState = "INITIALIZING" @@ -163,9 +164,8 @@ const ( // DataSourceStateOff indicates that the data source has been permanently shut down. // - // This could be because it encountered an unrecoverable error (for instance, the LaunchDarkly service - // rejected the SDK key; an invalid SDK key will never become valid), or because the SDK client was - // explicitly shut down. + // This could be because the SDK client was explicitly shut down, or (rarely) because the + // data source encountered a startup configuration error that prevented any connection attempt. DataSourceStateOff DataSourceState = "OFF" ) diff --git a/internal/datasource/helpers.go b/internal/datasource/helpers.go index 570a2e2d..c4c4e9b0 100644 --- a/internal/datasource/helpers.go +++ b/internal/datasource/helpers.go @@ -83,32 +83,50 @@ func classifyTransportFailure(err error) FailureClass { func httpErrorDescription(statusCode int) string { message := "" if statusCode == 401 || statusCode == 403 { - message = " (authentication failed)" + message = " (invalid SDK key)" } return fmt.Sprintf("HTTP error %d%s", statusCode, message) } // classifyAndLogHTTPFailure classifies an HTTP failure per RETRY §1.6, logs it, -// and returns the classification for the caller to act on. +// and returns the classification for the caller to act on. Unexpected +// failures (401, 403, other 4xx) log at Error since they almost always +// indicate a real customer-side problem (invalid or expired SDK key, +// misconfiguration) even though the SDK will keep retrying; normal failures +// (400, 408, 429, 5xx) log at Warn since they are typically transient. func classifyAndLogHTTPFailure( loggers ldlog.Loggers, errorDesc, errorContext string, statusCode int, willRetryMessage string, ) FailureClass { - loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, errorDesc) - return classifyHTTPFailure(statusCode) + class := classifyHTTPFailure(statusCode) + if class == FailureClassUnexpected { + loggers.Errorf("Error %s (%s): %s", errorContext, willRetryMessage, errorDesc) + } else { + loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, errorDesc) + } + return class } // classifyAndLogTransportFailure classifies a transport-layer failure per RETRY -// §1.7, logs it, and returns the classification. +// §1.7, logs it, and returns the classification. Unexpected failures (TLS +// or certificate validation errors) log at Error since they almost always +// indicate a real customer-side problem (misconfigured trust store, +// expired cert) even though the SDK will keep retrying; other transport +// failures log at Warn since they are typically transient. func classifyAndLogTransportFailure( loggers ldlog.Loggers, err error, errorContext, willRetryMessage string, ) FailureClass { - loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, err.Error()) - return classifyTransportFailure(err) + class := classifyTransportFailure(err) + if class == FailureClassUnexpected { + loggers.Errorf("Error %s (%s): %s", errorContext, willRetryMessage, err.Error()) + } else { + loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, err.Error()) + } + return class } func checkForHTTPError(statusCode int, url string) error { diff --git a/internal/datasource/helpers_test.go b/internal/datasource/helpers_test.go index 80f2fc29..c95fb9c0 100644 --- a/internal/datasource/helpers_test.go +++ b/internal/datasource/helpers_test.go @@ -55,8 +55,8 @@ func TestClassifyTransportFailure(t *testing.T) { func TestHTTPErrorDescription(t *testing.T) { assert.Equal(t, "HTTP error 400", httpErrorDescription(400)) - assert.Equal(t, "HTTP error 401 (authentication failed)", httpErrorDescription(401)) - assert.Equal(t, "HTTP error 403 (authentication failed)", httpErrorDescription(403)) + assert.Equal(t, "HTTP error 401 (invalid SDK key)", httpErrorDescription(401)) + assert.Equal(t, "HTTP error 403 (invalid SDK key)", httpErrorDescription(403)) assert.Equal(t, "HTTP error 500", httpErrorDescription(500)) } diff --git a/internal/datasource/streaming_data_source.go b/internal/datasource/streaming_data_source.go index 41028224..a6a8a9db 100644 --- a/internal/datasource/streaming_data_source.go +++ b/internal/datasource/streaming_data_source.go @@ -37,10 +37,12 @@ import ( // 2b. If the data store doesn't support status notifications (which is normally only true of the in-memory store) // then we don't know the significance of the error, but we must assume that updates have been lost, so we'll // restart the stream. -// 3. If we receive an unrecoverable error like HTTP 401, we close the stream and don't retry, and set the state -// to OFF. Any other HTTP error or network error causes a retry with backoff, with a state of INTERRUPTED. +// 3. If we receive an HTTP or transport-level error, we classify it and continue retrying: +// normal errors (400, 408, 429, 5xx, most I/O errors) use the default backoff profile; unexpected errors +// (401, 403, other 4xx, TLS/cert failures) engage the extended-regime profile. +// No HTTP status is terminal and in every case we set state to INTERRUPTED. // 4. We set the Future returned by start() to tell the client initialization logic that initialization has either -// succeeded (we got an initial payload and successfully stored it) or permanently failed (we got a 401, etc.). +// succeeded (we got an initial payload and successfully stored it) or permanently failed (unparseable base URI). // Otherwise, the client initialization method may time out but we will still be retrying in the background, and // if we succeed then the client can detect that we're initialized now by calling our Initialized method. diff --git a/ldclient.go b/ldclient.go index 2b0e11ae..1d1e37a9 100644 --- a/ldclient.go +++ b/ldclient.go @@ -77,7 +77,7 @@ type dataSystem interface { FlagChangeEventBroadcaster() *internal.Broadcaster[interfaces.FlagChangeEvent] // Start starts the data system; the given channel will be closed when the system has reached an initial state - // (either permanently failed, e.g. due to bad auth, or succeeded, where Initialized() == true). + // (either permanently failed, e.g. due to invalid configuration, or succeeded, where Initialized() == true). Start(closeWhenReady chan struct{}) // Stop halts the data system. Should be called when the client is closed to stop any long-running operations. @@ -136,8 +136,7 @@ var ( ErrInitializationTimeout = errors.New("timeout encountered waiting for LaunchDarkly client initialization") // MakeClient and MakeCustomClient will return this error if the SDK detected an error that makes it - // impossible for a LaunchDarkly connection to succeed. Currently, the only such condition is if the - // SDK key is invalid, since an invalid SDK key will never become valid. + // impossible for a LaunchDarkly connection to succeed (e.g. an unparseable service URI). ErrInitializationFailed = errors.New("LaunchDarkly client initialization failed") // This error is returned by the Variation/VariationDetail methods if feature flags are not available @@ -161,8 +160,8 @@ var ( // uninitialized state, where feature flags will return default values-- and the error value is // [ErrInitializationTimeout]. In this case, it will still continue trying to connect in the background. // -// If there was an unrecoverable error such that it cannot succeed by retrying-- for instance, the SDK key is -// invalid-- it will return a client instance in an uninitialized state, and the error value is +// If there was a startup configuration error such that no connection could be attempted-- for instance, an +// unparseable service URI-- it will return a client instance in an uninitialized state, and the error value is // [ErrInitializationFailed]. // // If you set waitFor to zero, the function will return immediately after creating the client instance, and @@ -197,8 +196,8 @@ func MakeClient(sdkKey string, waitFor time.Duration) (*LDClient, error) { // uninitialized state, where feature flags will return default values-- and the error value is // [ErrInitializationTimeout]. In this case, it will still continue trying to connect in the background. // -// If there was an unrecoverable error such that it cannot succeed by retrying-- for instance, the SDK key is -// invalid-- it will return a client instance in an uninitialized state, and the error value is +// If there was a startup configuration error such that no connection could be attempted-- for instance, an +// unparseable service URI-- it will return a client instance in an uninitialized state, and the error value is // [ErrInitializationFailed]. // // If you set waitFor to zero, the function will return immediately after creating the client instance, and diff --git a/subsystems/data_source_status_reporter.go b/subsystems/data_source_status_reporter.go index fa984d52..7427d824 100644 --- a/subsystems/data_source_status_reporter.go +++ b/subsystems/data_source_status_reporter.go @@ -16,8 +16,8 @@ type DataSourceStatusReporter interface { // DataSourceStatusProvider.GetStatus(), and will trigger status change events to any // registered listeners. // - // A special case is that if newState is DataSourceStateInterrupted, but the previous state was + // A special case is that if newState is DataSourceStateInterrupted but the previous state was // DataSourceStateInitializing, the state will remain at Initializing because Interrupted is - // only meaningful after a successful startup. + // only meaningful after a Valid state. UpdateStatus(newState interfaces.DataSourceState, newError interfaces.DataSourceErrorInfo) } diff --git a/subsystems/data_source_update_sink.go b/subsystems/data_source_update_sink.go index 6ffdbf28..015c7a00 100644 --- a/subsystems/data_source_update_sink.go +++ b/subsystems/data_source_update_sink.go @@ -45,9 +45,9 @@ type DataSourceUpdateSink interface { // DataSourceStatusProvider.GetStatus(), and will trigger status change events to any // registered listeners. // - // A special case is that if newState is DataSourceStateInterrupted, but the previous state was - // but the previous state was DataSourceStateInitializing, the state will remain at Initializing - // because Interrupted is only meaningful after a successful startup. + // A special case is that if newState is DataSourceStateInterrupted but the previous state was + // DataSourceStateInitializing, the state will remain at Initializing because Interrupted is + // only meaningful after a Valid state. UpdateStatus(newState interfaces.DataSourceState, newError interfaces.DataSourceErrorInfo) // GetDataStoreStatusProvider returns an object that provides status tracking for the data store, if From 28d906e9855c3292539e5279710fc7212b18900b Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 21 Aug 2026 09:58:10 -0400 Subject: [PATCH 11/12] docs(datasource): tighten comments per PR review Strip spec-section citations and stale ticket references from comments in the polling/streaming data source and its tests. Rewrite test comments that framed behavior in terms of prior implementation to describe current behavior only. Simplify the hard-to-follow comment on the polling-strategy n-reset test. --- internal/datasource/helpers.go | 42 +++++++++---------- internal/datasource/helpers_test.go | 8 ++-- internal/datasource/polling_data_source.go | 4 +- .../datasource/polling_data_source_test.go | 14 +++---- internal/datasource/polling_strategy.go | 31 +++++++------- internal/datasource/polling_strategy_test.go | 40 ++++++++---------- internal/datasource/streaming_data_source.go | 8 ++-- .../datasource/streaming_data_source_test.go | 16 ++++--- ldclient_end_to_end_test.go | 37 +++++++--------- 9 files changed, 93 insertions(+), 107 deletions(-) diff --git a/internal/datasource/helpers.go b/internal/datasource/helpers.go index c4c4e9b0..5c5d4551 100644 --- a/internal/datasource/helpers.go +++ b/internal/datasource/helpers.go @@ -23,10 +23,10 @@ func (e httpStatusError) Error() string { return e.Message } -// FailureClass categorizes a data source failure per RETRY §1.5--§1.7. Under the -// RETRY spec no failure is permanently terminal: every failure is either "normal" -// (regular backoff and retry) or "unexpected" (extended backoff via a longer -// retry profile or wait interval, still retrying indefinitely). +// FailureClass categorizes a data source failure. No failure is permanently +// terminal: every failure is either "normal" (regular backoff and retry) or +// "unexpected" (extended backoff via a longer retry profile or wait interval, +// still retrying indefinitely). type FailureClass int const ( @@ -39,8 +39,8 @@ const ( ) // classifyHTTPFailure returns the failure classification for an HTTP status code -// received during a data source request, per RETRY §1.6. Called only when the -// status indicates failure (non-2xx). +// received during a data source request. Called only when the status indicates +// failure (non-2xx). func classifyHTTPFailure(statusCode int) FailureClass { if statusCode >= 400 && statusCode < 500 { switch statusCode { @@ -54,9 +54,9 @@ func classifyHTTPFailure(statusCode int) FailureClass { } // classifyTransportFailure returns the failure classification for a transport-layer -// error (i.e., not an HTTP response, but a lower-level network or TLS failure) -// per RETRY §1.7. TLS/certificate validation failures are treated as unexpected; -// everything else is treated as normal. +// error (i.e., not an HTTP response, but a lower-level network or TLS failure). +// TLS/certificate validation failures are treated as unexpected; everything else +// is treated as normal. func classifyTransportFailure(err error) FailureClass { if err == nil { return FailureClassNormal @@ -88,12 +88,12 @@ func httpErrorDescription(statusCode int) string { return fmt.Sprintf("HTTP error %d%s", statusCode, message) } -// classifyAndLogHTTPFailure classifies an HTTP failure per RETRY §1.6, logs it, -// and returns the classification for the caller to act on. Unexpected -// failures (401, 403, other 4xx) log at Error since they almost always -// indicate a real customer-side problem (invalid or expired SDK key, -// misconfiguration) even though the SDK will keep retrying; normal failures -// (400, 408, 429, 5xx) log at Warn since they are typically transient. +// classifyAndLogHTTPFailure classifies an HTTP failure, logs it, and returns +// the classification for the caller to act on. Unexpected failures (401, 403, +// other 4xx) log at Error since they almost always indicate a real +// customer-side problem (invalid or expired SDK key, misconfiguration) even +// though the SDK will keep retrying; normal failures (400, 408, 429, 5xx) log +// at Warn since they are typically transient. func classifyAndLogHTTPFailure( loggers ldlog.Loggers, errorDesc, errorContext string, @@ -109,12 +109,12 @@ func classifyAndLogHTTPFailure( return class } -// classifyAndLogTransportFailure classifies a transport-layer failure per RETRY -// §1.7, logs it, and returns the classification. Unexpected failures (TLS -// or certificate validation errors) log at Error since they almost always -// indicate a real customer-side problem (misconfigured trust store, -// expired cert) even though the SDK will keep retrying; other transport -// failures log at Warn since they are typically transient. +// classifyAndLogTransportFailure classifies a transport-layer failure, logs it, +// and returns the classification. Unexpected failures (TLS or certificate +// validation errors) log at Error since they almost always indicate a real +// customer-side problem (misconfigured trust store, expired cert) even though +// the SDK will keep retrying; other transport failures log at Warn since they +// are typically transient. func classifyAndLogTransportFailure( loggers ldlog.Loggers, err error, diff --git a/internal/datasource/helpers_test.go b/internal/datasource/helpers_test.go index c95fb9c0..3e01357a 100644 --- a/internal/datasource/helpers_test.go +++ b/internal/datasource/helpers_test.go @@ -16,8 +16,8 @@ func TestHTTPStatusError(t *testing.T) { assert.Equal(t, "message", error.Error()) } -// classifyHTTPFailure per RETRY §1.6: 400, 408, 429 are normal (transient -// server-side conditions); all other 4xx are unexpected (durable client-side +// classifyHTTPFailure: 400, 408, 429 are normal (transient server-side +// conditions); all other 4xx are unexpected (durable client-side // misconfiguration); 5xx and everything else are normal. func TestClassifyHTTPFailure(t *testing.T) { for i := 400; i < 500; i++ { @@ -32,8 +32,8 @@ func TestClassifyHTTPFailure(t *testing.T) { } } -// classifyTransportFailure per RETRY §1.7: TLS/certificate validation failures -// are unexpected; other transport-layer errors are normal. +// classifyTransportFailure: TLS/certificate validation failures are unexpected; +// other transport-layer errors are normal. func TestClassifyTransportFailure(t *testing.T) { assert.Equal(t, FailureClassNormal, classifyTransportFailure(nil)) assert.Equal(t, FailureClassNormal, classifyTransportFailure(errors.New("boom"))) diff --git a/internal/datasource/polling_data_source.go b/internal/datasource/polling_data_source.go index 8e029524..cf032f8d 100644 --- a/internal/datasource/polling_data_source.go +++ b/internal/datasource/polling_data_source.go @@ -93,8 +93,8 @@ func (pp *PollingProcessor) Start(closeWhenReady chan<- struct{}) { pp.loggers.Infof("Starting LaunchDarkly polling with interval: %+v", pp.pollInterval) // Fires immediately for the first poll; Reset after each iteration to schedule - // the next. Under RETRY (SDK-2775), the interval between polls is dynamic per - // the pollingStrategy state machine, so we can't use a fixed-period Ticker. + // the next. The interval between polls is dynamic per the pollingStrategy + // state machine, so we can't use a fixed-period Ticker. timer := time.NewTimer(0) go func() { diff --git a/internal/datasource/polling_data_source_test.go b/internal/datasource/polling_data_source_test.go index 4109677a..2ff76012 100644 --- a/internal/datasource/polling_data_source_test.go +++ b/internal/datasource/polling_data_source_test.go @@ -141,11 +141,9 @@ func testPollingProcessorRecoverableError(t *testing.T, err error, verifyError f }) } -// Under the RETRY spec (SDK-2775), previously-terminal 4xx errors (401, 403, 404, -// 405) engage an extended-regime backoff but keep polling indefinitely. The -// processor transitions to Interrupted (not Off) and continues to poll. Replaces -// the pre-RETRY TestPollingProcessorUnrecoverableErrors, which asserted the old -// permanent-stop behavior. +// 4xx errors (401, 403, 404, 405) engage an extended-regime backoff but keep +// polling indefinitely. The processor transitions to Interrupted (not Off) +// and continues to poll. func TestPollingProcessorUnexpectedErrorsEngageExtendedRegimeAndKeepRetrying(t *testing.T) { for _, statusCode := range []int{401, 403, 404, 405} { t.Run(fmt.Sprintf("HTTP %d", statusCode), func(t *testing.T) { @@ -194,7 +192,7 @@ func testPollingProcessorUnexpectedError( // Initialization must not complete: no permanent stop, no successful poll. select { case <-closeWhenReady: - t.Fatal("closeWhenReady should not be closed -- RETRY §1.2.1 forbids permanent stops on 4xx") + t.Fatal("closeWhenReady should not be closed -- no permanent stops on 4xx") case <-time.After(500 * time.Millisecond): } @@ -214,8 +212,8 @@ func testPollingProcessorUnexpectedError( // After two consecutive successful polls, the processor must reset to the normal // regime: n=0, and subsequent waits equal PollInterval (not the extended initial -// delay). This exercises the RETRY §1.8 polling binding, where the reset -// condition is a fixed count of successful polls rather than a time threshold. +// delay). The reset condition is a fixed count of successful polls rather than +// a time threshold. func TestPollingResetsToNormalAfterTwoConsecutiveSuccesses(t *testing.T) { req := mocks.NewPollingRequester() defer req.Close() diff --git a/internal/datasource/polling_strategy.go b/internal/datasource/polling_strategy.go index 8327caac..059a7b8a 100644 --- a/internal/datasource/polling_strategy.go +++ b/internal/datasource/polling_strategy.go @@ -6,22 +6,21 @@ import ( "time" ) -// extendedPollMaxDelay is the RETRY-spec extended-regime ceiling on the polling -// backoff. Effective ceiling is max(extendedPollMaxDelay, PollInterval); see +// extendedPollMaxDelay is the extended-regime ceiling on the polling backoff. +// Effective ceiling is max(extendedPollMaxDelay, PollInterval); see // pollingStrategy.OnFailure. const extendedPollMaxDelay = 1 * time.Hour -// pollingStrategy implements the RETRY §1.4 timing mechanics for the polling -// data source. It owns: +// pollingStrategy implements the timing mechanics for the polling data source. +// It owns: // -// - the formula-input counter n (RETRY §1.4's "attempts", used as the -// exponent in T = initialDelay * 2^(n-1); resets on regime transition -// per RETRY §1.5.3 binding); -// - the current regime's (initialDelay, maxDelay), toggled by classification -// (RETRY §1.5--§1.7 via the caller's FailureClass); -// - the two-consecutive-success reset gate (RETRY §1.8 polling binding); -// - jitter (RETRY §1.4.3); -// - the PollInterval wait floor (RETRY §1.4.4 polling override). +// - the formula-input counter n (used as the exponent in +// T = initialDelay * 2^(n-1); resets on regime transition); +// - the current regime's (initialDelay, maxDelay), toggled by the caller's +// FailureClass classification; +// - the two-consecutive-success reset gate; +// - jitter; +// - the PollInterval wait floor. // // All state is owned and mutated by the polling run() goroutine only -- no // locking required. @@ -77,9 +76,9 @@ func (s *pollingStrategy) OnFailure(class FailureClass) (transitionedToExtended } // OnSuccess updates the strategy state after a successful poll. Two consecutive -// successes reset n and return the SDK to the normal regime (RETRY §1.8 -// polling binding). A single success is a necessary precondition but not -// sufficient -- any failure between the first and second success clears it. +// successes reset n and return the SDK to the normal regime. A single success +// is a necessary precondition but not sufficient -- any failure between the +// first and second success clears it. func (s *pollingStrategy) OnSuccess() { if s.priorPollWasSuccessful { s.n = 0 @@ -90,7 +89,7 @@ func (s *pollingStrategy) OnSuccess() { s.priorPollWasSuccessful = true } -// NextWait returns the delay before the next poll attempt per RETRY §1.4. +// NextWait returns the delay before the next poll attempt. // Formula: T = initialDelay * 2^(n-1), clamped to maxDelay. // Jitter J is a uniform random in [0, T/2]. The final wait = max(PollInterval, // T - J) ensures the interval never drops below the caller's configured diff --git a/internal/datasource/polling_strategy_test.go b/internal/datasource/polling_strategy_test.go index 741a9ab9..7477ef38 100644 --- a/internal/datasource/polling_strategy_test.go +++ b/internal/datasource/polling_strategy_test.go @@ -26,7 +26,7 @@ func TestPollingStrategy_NormalFailureDoesNotEngageExtended(t *testing.T) { } // An unexpected-classified failure engages the extended regime: initialDelay -// becomes the configured extended base and maxDelay becomes the RETRY-spec cap. +// becomes the configured extended base and maxDelay becomes the extended cap. func TestPollingStrategy_UnexpectedFailureEngagesExtended(t *testing.T) { s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) s.OnFailure(FailureClassUnexpected) @@ -49,15 +49,11 @@ func TestPollingStrategy_ExtendedInitialClampedToPollInterval(t *testing.T) { assert.Equal(t, time.Hour, s.NextWait()) } -// After prior normal-classified failures, the first unexpected failure MUST -// reset n to 1 so the first extended-regime wait uses the new initialDelay -// (5min) directly, not initialDelay * 2^k where k is the count of prior -// normal failures. Guards against conflating the two roles of a single -// counter -- formula input (this field, n; resets on regime transition per -// RETRY §1.5.3 / streaming Confluence spec) and total-attempts observability -// (a separate concept not tracked by this struct). Without this behavior, a -// sequence of "normal, normal, unexpected" would inflate the first extended -// wait to 20min or more (bounded by extendedPollMaxDelay). +// On transition from normal into extended, n resets to 1. This ensures the +// first extended-regime wait is initialDelay (5min by default), not +// initialDelay * 2^(prior_normal_failures). Without the reset, a sequence of +// "normal, normal, unexpected" would produce a first extended wait of 20min +// instead of 5min. func TestPollingStrategy_UnexpectedAfterNormalFailuresStartsAtInitialDelay(t *testing.T) { s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) @@ -110,9 +106,9 @@ func TestPollingStrategy_UnexpectedWhileAlreadyExtendedContinuesDoubling(t *test assert.Equal(t, time.Hour, s.maxDelay) } -// Extended regime doubles per attempt per RETRY §1.4, capped at -// extendedPollMaxDelay (1 hour). Jitter subtracts up to T/2, so each wait falls -// in [T/2, T] before the (in these cases irrelevant) wait floor. +// Extended regime doubles per attempt, capped at extendedPollMaxDelay (1 hour). +// Jitter subtracts up to T/2, so each wait falls in [T/2, T] before the +// (in these cases irrelevant) wait floor. func TestPollingStrategy_ExtendedDoubling(t *testing.T) { s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) @@ -135,8 +131,8 @@ func TestPollingStrategy_ExtendedDoubling(t *testing.T) { } } -// RETRY §1.4.4 polling B1 override: NextWait never returns less than PollInterval, -// even when the exponential math (or jitter) would drive it below. +// Polling wait floor: NextWait never returns less than PollInterval, even when +// the exponential math (or jitter) would drive it below. func TestPollingStrategy_WaitFloorAtPollInterval(t *testing.T) { // Extended base is much smaller than PollInterval. Every wait should be // clamped up to PollInterval until doubling grows T past it. @@ -160,7 +156,7 @@ func TestPollingStrategy_FirstSuccessDoesNotReset(t *testing.T) { } // Two consecutive successes clear n and return the strategy to the -// normal regime (RETRY §1.8 polling binding). +// normal regime. func TestPollingStrategy_TwoConsecutiveSuccessesReset(t *testing.T) { s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) s.OnFailure(FailureClassUnexpected) @@ -207,8 +203,8 @@ func TestPollingStrategy_NormalFailureAfterResetStaysNormal(t *testing.T) { // combo of 5min/5min was the flagged customer config), the extended-regime // initialDelay clamps up to equal normalInterval. If transition detection // relies on that equality, every subsequent unexpected failure re-fires the -// transition path and resets n to 1, and RETRY spec 1.4.1's doubling never -// engages. Explicit regime state (inExtended) avoids the clamp collision. +// transition path and resets n to 1, and the doubling never engages. Explicit +// regime state (inExtended) avoids the clamp collision. func TestPollingStrategy_ExtendedDoublingWhenClampedToPollInterval(t *testing.T) { s := newPollingStrategy(5*time.Minute, defaultExtendedInitialPollDelay) @@ -250,16 +246,16 @@ func TestPollingStrategy_OnFailureReturnsTrueOnceOnTransitionWhenClamped(t *test assert.False(t, s.OnFailure(FailureClassUnexpected), "third unexpected must not re-signal transition") } -// After the two-consecutive-successes reset (RETRY §1.8), the strategy is -// back in the normal regime and a subsequent unexpected failure is treated -// as a new transition, signalling to the caller for a fresh log line. +// After the two-consecutive-successes reset, the strategy is back in the normal +// regime and a subsequent unexpected failure is treated as a new transition, +// signalling to the caller for a fresh log line. func TestPollingStrategy_OnSuccessResetAllowsRetransition(t *testing.T) { s := newPollingStrategy(30*time.Second, defaultExtendedInitialPollDelay) assert.True(t, s.OnFailure(FailureClassUnexpected), "initial transition") assert.False(t, s.OnFailure(FailureClassUnexpected), "already in extended") - // Two consecutive successes: full reset per RETRY §1.8. + // Two consecutive successes: full reset. s.OnSuccess() s.OnSuccess() diff --git a/internal/datasource/streaming_data_source.go b/internal/datasource/streaming_data_source.go index a6a8a9db..f057d43e 100644 --- a/internal/datasource/streaming_data_source.go +++ b/internal/datasource/streaming_data_source.go @@ -378,10 +378,10 @@ func (sp *StreamProcessor) subscribe(closeWhenReady chan<- struct{}) { sp.dataSourceUpdates.UpdateStatus(interfaces.DataSourceStateInterrupted, errorInfo) sp.logConnectionStarted() - // Per RETRY §1.2.1: no failure is permanently terminal. Unexpected failures - // engage the extended-regime profile; the library keeps retrying at extended - // cadence until a healthy-op reset (retryResetInterval of continuous - // connection) reverts. + // No failure is permanently terminal. Unexpected failures engage the + // extended-regime profile; the library keeps retrying at extended cadence + // until a healthy-op reset (retryResetInterval of continuous connection) + // reverts. result := es.StreamErrorHandlerResult{CloseNow: false} if class == FailureClassUnexpected { if !loggedActivatedExtended { diff --git a/internal/datasource/streaming_data_source_test.go b/internal/datasource/streaming_data_source_test.go index 13c83668..48a00dc4 100644 --- a/internal/datasource/streaming_data_source_test.go +++ b/internal/datasource/streaming_data_source_test.go @@ -249,11 +249,9 @@ func TestStreamProcessorRecoverableErrorsCauseStreamRestart(t *testing.T) { }) } -// Under the RETRY spec (SDK-2775), 401 / 403 / other 4xx are no longer terminal -- -// they engage an extended-regime backoff but keep retrying indefinitely. The SDK -// transitions to Interrupted (not Off) and does not close the initialization -// channel. This test replaces the pre-RETRY TestStreamProcessorUnrecoverableErrors -// CauseStreamShutdown, which asserted the old (permanent-stop) behavior. +// 401 / 403 / other 4xx engage an extended-regime backoff but keep retrying +// indefinitely. The SDK transitions to Interrupted (not Off) and does not +// close the initialization channel. func TestStreamProcessorUnexpectedErrorsEngageExtendedRegimeAndKeepRetrying(t *testing.T) { for _, status := range []int{401, 403, 404} { t.Run(fmt.Sprintf("HTTP status %d", status), func(t *testing.T) { @@ -403,8 +401,8 @@ func testStreamProcessorUnexpectedHTTPError(t *testing.T, statusCode int) { // Short retry delays so we can observe at least two attempts within the // assertion window. The extended-regime profile activates immediately on the - // first failure (per the RETRY spec -- no grace period for initial-connection - // unexpected classifications), so we need to shorten ExtendedInitialReconnectDelay + // first failure (no grace period for initial-connection unexpected + // classifications), so we need to shorten ExtendedInitialReconnectDelay // too, not just the normal InitialReconnectDelay. sp := NewStreamProcessor(context, dataSourceUpdates, StreamConfig{ URI: ts.URL, @@ -421,7 +419,7 @@ func testStreamProcessorUnexpectedHTTPError(t *testing.T, statusCode int) { // we just assert the channel stays open. select { case <-closeWhenReady: - t.Fatal("closeWhenReady should not be closed -- RETRY §1.2.1 forbids permanent stops on 4xx") + t.Fatal("closeWhenReady should not be closed -- no permanent stops on 4xx") case <-time.After(time.Second): } @@ -429,7 +427,7 @@ func testStreamProcessorUnexpectedHTTPError(t *testing.T, statusCode int) { // by the processor -- so we see the Interrupted call the processor tried // to make. (The real DataSourceUpdateSinkImpl would clamp it to // Initializing since we never reached Valid; that's tested at the - // LDClient-level in the RETRY end-to-end tests. Here we just verify the + // LDClient-level end-to-end tests. Here we just verify the // processor emitted the correct call.) status := dataSourceUpdates.RequireStatusOf(t, interfaces.DataSourceStateInterrupted) assert.Equal(t, interfaces.DataSourceErrorKindErrorResponse, status.LastError.Kind) diff --git a/ldclient_end_to_end_test.go b/ldclient_end_to_end_test.go index 519301ae..6f53aa67 100644 --- a/ldclient_end_to_end_test.go +++ b/ldclient_end_to_end_test.go @@ -105,12 +105,9 @@ func TestClientStartsInStreamingMode(t *testing.T) { }) } -// Under the RETRY spec (SDK-2775), a 401 is no longer terminal: the client -// times out waiting for initial data but the data source keeps retrying -// indefinitely. Init returns the usual timeout error; the data source state is -// Interrupted (not Off); the stream keeps hitting the server. Replaces the -// pre-RETRY TestClientFailsToStartInStreamingModeWith401Error, which asserted -// the old permanent-stop behavior. +// On a 401 the client times out waiting for initial data but the data source +// keeps retrying indefinitely. Init returns the usual timeout error; the data +// source state is Interrupted (not Off); the stream keeps hitting the server. func TestClientInStreamingModeWith401KeepsRetrying(t *testing.T) { handler, requestsCh := httphelpers.RecordingHandler(httphelpers.HandlerWithStatus(401)) httphelpers.WithServer(handler, func(streamServer *httptest.Server) { @@ -140,11 +137,11 @@ func TestClientInStreamingModeWith401KeepsRetrying(t *testing.T) { assert.Equal(t, ErrInitializationTimeout, err) - // Under RETRY the SDK does not permanently stop on 401. Since we never - // reached a Valid state, the SinkImpl keeps state as Initializing rather - // than transitioning to Interrupted (see maybeUpdateStatus's Initializing - // clamp), but LastError records the failure. The key assertion is - // "state is NOT Off" -- no permanent stop. + // The SDK does not permanently stop on 401. Since we never reached a Valid + // state, the SinkImpl keeps state as Initializing rather than transitioning + // to Interrupted (see maybeUpdateStatus's Initializing clamp), but LastError + // records the failure. The key assertion is "state is NOT Off" -- no + // permanent stop. require.Eventually(t, func() bool { s := client.GetDataSourceStatusProvider().GetStatus() return s.LastError.Kind == interfaces.DataSourceErrorKindErrorResponse && @@ -153,7 +150,7 @@ func TestClientInStreamingModeWith401KeepsRetrying(t *testing.T) { "data source should record the 401 as LastError") assert.NotEqual(t, string(interfaces.DataSourceStateOff), string(client.GetDataSourceStatusProvider().GetStatus().State), - "RETRY §1.2.1: no permanent stop on 401") + "no permanent stop on 401") // Flag evaluation still works and returns defaults. value, _ := client.BoolVariation(alwaysTrueFlag.Key, testUser, false) @@ -291,10 +288,8 @@ func TestInstanceIDIsDifferentBetweenClients(t *testing.T) { }) } -// Under the RETRY spec (SDK-2775), a 401 is no longer terminal for polling -// either: the goroutine keeps polling on the extended-regime cadence. Init -// times out; state is Interrupted; the client kept polling. Replaces the -// pre-RETRY TestClientFailsToStartInPollingModeWith401Error. +// On a 401 the polling goroutine keeps polling on the extended-regime cadence. +// Init times out; state is Interrupted; the client kept polling. func TestClientInPollingModeWith401KeepsRetrying(t *testing.T) { handler, requestsCh := httphelpers.RecordingHandler(httphelpers.HandlerWithStatus(401)) httphelpers.WithServer(handler, func(pollServer *httptest.Server) { @@ -316,10 +311,10 @@ func TestClientInPollingModeWith401KeepsRetrying(t *testing.T) { assert.Equal(t, ErrInitializationTimeout, err) - // Under RETRY the SDK does not permanently stop on 401. Since we never - // reached a Valid state, the SinkImpl keeps state as Initializing rather - // than transitioning to Interrupted. The key assertion is "state is NOT - // Off" -- no permanent stop -- and LastError records the failure. + // The SDK does not permanently stop on 401. Since we never reached a Valid + // state, the SinkImpl keeps state as Initializing rather than transitioning + // to Interrupted. The key assertion is "state is NOT Off" -- no permanent + // stop -- and LastError records the failure. require.Eventually(t, func() bool { s := client.GetDataSourceStatusProvider().GetStatus() return s.LastError.Kind == interfaces.DataSourceErrorKindErrorResponse && @@ -328,7 +323,7 @@ func TestClientInPollingModeWith401KeepsRetrying(t *testing.T) { "data source should record the 401 as LastError") assert.NotEqual(t, string(interfaces.DataSourceStateOff), string(client.GetDataSourceStatusProvider().GetStatus().State), - "RETRY §1.2.1: no permanent stop on 401") + "no permanent stop on 401") value, _ := client.BoolVariation(alwaysTrueFlag.Key, testUser, false) assert.False(t, value) From 576f4d3aa11d459f6f8b4f5ff9382e828c922e8a Mon Sep 17 00:00:00 2001 From: Todd Anderson <127344469+tanderson-ld@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:42:44 -0400 Subject: [PATCH 12/12] Update internal/datasource/polling_strategy.go Co-authored-by: Jason Bailey --- internal/datasource/polling_strategy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/datasource/polling_strategy.go b/internal/datasource/polling_strategy.go index 059a7b8a..c11bd974 100644 --- a/internal/datasource/polling_strategy.go +++ b/internal/datasource/polling_strategy.go @@ -53,7 +53,7 @@ func newPollingStrategy(pollInterval, extendedInitialPollInterval time.Duration) // // On the transition from normal into extended regime, n is reset to 1 so // that the first extended-regime wait uses the new initialDelay. Returns -// true iff this call transitioned the strategy from normal into extended +// true if this call transitioned the strategy from normal into extended // regime, so the caller can log a one-time notice. func (s *pollingStrategy) OnFailure(class FailureClass) (transitionedToExtended bool) { s.priorPollWasSuccessful = false