From d620840cacdea063edbf671121f532888fcabd83 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:25:57 +0000 Subject: [PATCH 1/5] fix: Make FDv1 fallback TTL-based and always recover to FDv2 Co-Authored-By: Bee Klimt --- .../datasourcev2/fdv1_polling_data_source.go | 6 +- internal/datasourcev2/helpers.go | 43 ++++ internal/datasourcev2/helpers_test.go | 60 ++++++ internal/datasourcev2/polling_data_source.go | 28 ++- .../datasourcev2/polling_data_source_test.go | 44 +++- internal/datasourcev2/polling_http_request.go | 55 +++-- .../datasourcev2/polling_http_request_test.go | 26 +-- .../release_clause_values_test.go | 4 +- .../datasourcev2/streaming_data_source.go | 61 ++++-- .../streaming_data_source_test.go | 29 ++- internal/datasystem/fdv2_datasystem.go | 156 ++++++++++---- internal/datasystem/fdv2_fallback_test.go | 4 +- .../datasystem/fdv2_fdv1_recovery_test.go | 191 ++++++++++++++++++ internal/datasystem/synchronizer_list.go | 36 +++- internal/datasystem/synchronizer_list_test.go | 37 +++- ldclient_end_to_end_fdv2_test.go | 49 ++--- ldclient_override_vectors_test.go | 4 +- ldclient_overrides_test.go | 8 +- ldfiledatav2/file_data_source_impl.go | 13 +- ldfiledatav2/file_data_source_test.go | 2 +- subsystems/data_source.go | 25 ++- subsystems/datasystem_configuration.go | 4 +- subsystems/events.go | 3 + testhelpers/ldtestdatav2/test_data_source.go | 7 +- 24 files changed, 726 insertions(+), 169 deletions(-) create mode 100644 internal/datasystem/fdv2_fdv1_recovery_test.go diff --git a/internal/datasourcev2/fdv1_polling_data_source.go b/internal/datasourcev2/fdv1_polling_data_source.go index 52ba80fc2..95e066c05 100644 --- a/internal/datasourcev2/fdv1_polling_data_source.go +++ b/internal/datasourcev2/fdv1_polling_data_source.go @@ -17,10 +17,10 @@ type fdv1ToFDv2Requester struct { func (r *fdv1ToFDv2Requester) Request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, error) { +) (*subsystems.ChangeSet, http.Header, subsystems.FDv1Fallback, error) { data, cached, headers, err := r.requester.Request() if err != nil { - return nil, headers, err + return nil, headers, subsystems.FDv1Fallback{}, err } code := subsystems.IntentTransferFull @@ -47,7 +47,7 @@ func (r *fdv1ToFDv2Requester) Request( subsystems.NewSelector("state", 1), data, ) - return changeset, headers, err + return changeset, headers, subsystems.FDv1Fallback{}, err } func (r *fdv1ToFDv2Requester) BaseURI() string { diff --git a/internal/datasourcev2/helpers.go b/internal/datasourcev2/helpers.go index d7442420d..6e35f6f95 100644 --- a/internal/datasourcev2/helpers.go +++ b/internal/datasourcev2/helpers.go @@ -6,10 +6,15 @@ package datasourcev2 // v2 implementation, we should clean this up. import ( + "encoding/json" "fmt" + "math" "net/http" + "strconv" + "time" "github.com/launchdarkly/go-sdk-common/v3/ldlog" + "github.com/launchdarkly/go-server-sdk/v7/subsystems" ) // fdv1FallbackHeader is the response header LaunchDarkly uses to instruct an SDK to abandon the @@ -22,6 +27,44 @@ func isFDv1FallbackRequested(h http.Header) bool { return h.Get(fdv1FallbackHeader) == "true" } +// fdv1FallbackTTLHeader is the response header that says how many seconds the SDK should stay +// on FDv1 before trying FDv2 again. +const fdv1FallbackTTLHeader = "X-LD-FD-Fallback-TTL" + +// fdv1FallbackFromHeaders returns the FDv1 fallback instruction carried by the response headers. +func fdv1FallbackFromHeaders(h http.Header) subsystems.FDv1Fallback { + if !isFDv1FallbackRequested(h) { + return subsystems.FDv1Fallback{} + } + seconds, err := strconv.ParseInt(h.Get(fdv1FallbackTTLHeader), 10, 64) + if err != nil { + return subsystems.FDv1Fallback{Requested: true} + } + return subsystems.FDv1Fallback{Requested: true, TTL: fdv1FallbackTTLFromSeconds(seconds)} +} + +// fdv1FallbackFromGoodbye returns the FDv1 fallback instruction carried by a goodbye event. A +// goodbye event is an instruction to fall back only if it has a protocolFallbackTTL. +func fdv1FallbackFromGoodbye(goodbye subsystems.Goodbye) subsystems.FDv1Fallback { + if len(goodbye.ProtocolFallbackTTL) == 0 || string(goodbye.ProtocolFallbackTTL) == "null" { + return subsystems.FDv1Fallback{} + } + var seconds int64 + if err := json.Unmarshal(goodbye.ProtocolFallbackTTL, &seconds); err != nil { + return subsystems.FDv1Fallback{Requested: true} + } + return subsystems.FDv1Fallback{Requested: true, TTL: fdv1FallbackTTLFromSeconds(seconds)} +} + +// fdv1FallbackTTLFromSeconds converts a TTL in seconds to a duration. A value that is too large to +// convert gives zero, which means that the TTL is not usable. +func fdv1FallbackTTLFromSeconds(seconds int64) time.Duration { + if seconds > math.MaxInt64/int64(time.Second) { + return 0 + } + return time.Duration(seconds) * time.Second +} + type httpStatusError struct { Message string Code int diff --git a/internal/datasourcev2/helpers_test.go b/internal/datasourcev2/helpers_test.go index 5ba768aac..7c198140c 100644 --- a/internal/datasourcev2/helpers_test.go +++ b/internal/datasourcev2/helpers_test.go @@ -1,10 +1,16 @@ package datasourcev2 import ( + "encoding/json" + "net/http" "strconv" "testing" + "time" + + "github.com/launchdarkly/go-server-sdk/v7/subsystems" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestHTTPStatusError(t *testing.T) { @@ -51,3 +57,57 @@ func testWithFilters(t *testing.T, test func(t *testing.T, filterTest filterTest }) } } + +func TestFDv1FallbackFromHeaders(t *testing.T) { + for _, tc := range []struct { + name string + fallback string + ttl string + want subsystems.FDv1Fallback + }{ + {name: "no fallback header", ttl: "120", want: subsystems.FDv1Fallback{}}, + {name: "no TTL header", fallback: "true", want: subsystems.FDv1Fallback{Requested: true}}, + {name: "valid TTL", fallback: "true", ttl: "120", want: subsystems.FDv1Fallback{Requested: true, TTL: 120 * time.Second}}, + {name: "malformed TTL", fallback: "true", ttl: "soon", want: subsystems.FDv1Fallback{Requested: true}}, + {name: "fractional TTL", fallback: "true", ttl: "1.5", want: subsystems.FDv1Fallback{Requested: true}}, + {name: "negative TTL", fallback: "true", ttl: "-5", want: subsystems.FDv1Fallback{Requested: true, TTL: -5 * time.Second}}, + {name: "huge TTL", fallback: "true", ttl: "9223372036854775807", want: subsystems.FDv1Fallback{Requested: true}}, + } { + t.Run(tc.name, func(t *testing.T) { + h := http.Header{} + if tc.fallback != "" { + h.Set(fdv1FallbackHeader, tc.fallback) + } + if tc.ttl != "" { + h.Set(fdv1FallbackTTLHeader, tc.ttl) + } + assert.Equal(t, tc.want, fdv1FallbackFromHeaders(h)) + }) + } +} + +func TestFDv1FallbackFromGoodbye(t *testing.T) { + for _, tc := range []struct { + name string + ttl string + want subsystems.FDv1Fallback + }{ + {name: "no TTL", want: subsystems.FDv1Fallback{}}, + {name: "null TTL", ttl: "null", want: subsystems.FDv1Fallback{}}, + {name: "valid TTL", ttl: "120", want: subsystems.FDv1Fallback{Requested: true, TTL: 120 * time.Second}}, + {name: "zero TTL", ttl: "0", want: subsystems.FDv1Fallback{Requested: true}}, + {name: "malformed TTL", ttl: `"soon"`, want: subsystems.FDv1Fallback{Requested: true}}, + } { + t.Run(tc.name, func(t *testing.T) { + goodbye := subsystems.Goodbye{Reason: "test", ProtocolFallbackTTL: json.RawMessage(tc.ttl)} + assert.Equal(t, tc.want, fdv1FallbackFromGoodbye(goodbye)) + }) + } +} + +func TestGoodbyeUnmarshalsProtocolFallbackTTL(t *testing.T) { + var goodbye subsystems.Goodbye + require.NoError(t, json.Unmarshal([]byte(`{"reason":"r","protocolFallbackTTL":120}`), &goodbye)) + + assert.Equal(t, subsystems.FDv1Fallback{Requested: true, TTL: 120 * time.Second}, fdv1FallbackFromGoodbye(goodbye)) +} diff --git a/internal/datasourcev2/polling_data_source.go b/internal/datasourcev2/polling_data_source.go index 27c6edcf3..3e33458e3 100644 --- a/internal/datasourcev2/polling_data_source.go +++ b/internal/datasourcev2/polling_data_source.go @@ -21,7 +21,7 @@ const ( // PollingRequester allows PollingProcessor to delegate fetching data to another component. // This is useful for testing the PollingProcessor without needing to set up a test HTTP server. type PollingRequester interface { - Request(context.Context, subsystems.Selector) (*subsystems.ChangeSet, http.Header, error) + Request(context.Context, subsystems.Selector) (*subsystems.ChangeSet, http.Header, subsystems.FDv1Fallback, error) BaseURI() string FilterKey() string } @@ -68,9 +68,11 @@ func (pp *PollingProcessor) Name() string { } //nolint:revive // DataInitializer method. -func (pp *PollingProcessor) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, bool, error) { - changeSet, headers, err := pp.requester.Request(ctx, ds.Selector()) - fallback := isFDv1FallbackRequested(headers) +func (pp *PollingProcessor) Fetch( + ds subsystems.DataSelector, + ctx context.Context, +) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + changeSet, headers, fallback, err := pp.requester.Request(ctx, ds.Selector()) if err != nil { return nil, fallback, err } @@ -150,9 +152,9 @@ func (pp *PollingProcessor) Sync(ds subsystems.DataSelector) <-chan subsystems.D } // poll performs a single polling request and builds a DataSynchronizerResult describing the -// outcome. The result's FallbackToFDv1 flag is always populated from the x-ld-fd-fallback response -// header, whether or not the request succeeded -- a 500 or a malformed-JSON body can still carry -// the fallback signal. +// outcome. The result's FallbackToFDv1 flag and FDv1FallbackTTL are always populated from the +// fallback instruction in the response, whether or not the request succeeded -- a 500 or a +// malformed-JSON body can still carry the fallback signal. // // On success: result.State = Valid, result.ChangeSet populated, err = nil. // On error: result.State = Off (the safer default), result.Error populated with Kind/Message/ @@ -164,10 +166,11 @@ func (pp *PollingProcessor) Sync(ds subsystems.DataSelector) <-chan subsystems.D func (pp *PollingProcessor) poll( ctx context.Context, ds subsystems.DataSelector, ) (subsystems.DataSynchronizerResult, error) { - changeSet, headers, err := pp.requester.Request(ctx, ds.Selector()) + changeSet, headers, fallback, err := pp.requester.Request(ctx, ds.Selector()) result := subsystems.DataSynchronizerResult{ - EnvironmentID: internal.NewInitMetadataFromHeaders(headers).GetEnvironmentID(), - FallbackToFDv1: isFDv1FallbackRequested(headers), + EnvironmentID: internal.NewInitMetadataFromHeaders(headers).GetEnvironmentID(), + FallbackToFDv1: fallback.Requested, + FDv1FallbackTTL: fallback.TTL, } if err == nil { @@ -189,8 +192,11 @@ func (pp *PollingProcessor) poll( Message: err.Error(), Time: time.Now(), } - if _, ok := err.(malformedJSONError); ok { + switch err.(type) { + case malformedJSONError: result.Error.Kind = interfaces.DataSourceErrorKindInvalidData + case goodbyeError: + result.Error.Kind = interfaces.DataSourceErrorKindUnknown } } return result, err diff --git a/internal/datasourcev2/polling_data_source_test.go b/internal/datasourcev2/polling_data_source_test.go index e9b4cf5c3..e9db01823 100644 --- a/internal/datasourcev2/polling_data_source_test.go +++ b/internal/datasourcev2/polling_data_source_test.go @@ -2,6 +2,7 @@ package datasourcev2 import ( "context" + "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -301,7 +302,8 @@ func TestPollingProcessorSynchronizerHandlesFallbackToFDv2(t *testing.T) { ds := mocks.NewMockDataSelector(subsystems.NoSelector()) header := http.Header{ - "X-LD-FD-Fallback": []string{"true"}, + "X-LD-FD-Fallback": []string{"true"}, + "X-LD-FD-Fallback-TTL": []string{"120"}, } handler, _ := httphelpers.RecordingHandler(httphelpers.HandlerWithResponse(500, header, nil)) httphelpers.WithServer(handler, func(ts *httptest.Server) { @@ -320,6 +322,40 @@ func TestPollingProcessorSynchronizerHandlesFallbackToFDv2(t *testing.T) { assert.Equal(t, result.State, interfaces.DataSourceStateOff) assert.Equal(t, result.Error.Kind, interfaces.DataSourceErrorKindErrorResponse) assert.True(t, result.FallbackToFDv1) + assert.Equal(t, 120*time.Second, result.FDv1FallbackTTL) + }) +} + +func TestPollingProcessorSynchronizerFallsBackOnGoodbyeWithFallbackTTL(t *testing.T) { + ds := mocks.NewMockDataSelector(subsystems.NoSelector()) + + payload := subsystems.PollingPayload{Events: []subsystems.RawEvent{ + { + Name: subsystems.EventServerIntent, + Data: json.RawMessage(`{"payloads":[{"id":"id","target":1,"intentCode":"xfer-full","reason":"r"}]}`), + }, + { + Name: subsystems.EventGoodbye, + Data: json.RawMessage(`{"reason":"fall back","protocolFallbackTTL":120}`), + }, + }} + + httphelpers.WithServer(ldservices.ServerSidePollingV2ServiceHandler(payload), func(ts *httptest.Server) { + processor := NewPollingProcessor( + sharedtest.BasicClientContext(), + datasource.PollingConfig{ + BaseURI: ts.URL, + PollInterval: time.Minute * 30, + }, + ) + defer processor.Close() + + result := <-processor.Sync(ds) + + assert.Equal(t, interfaces.DataSourceStateOff, result.State) + assert.Equal(t, interfaces.DataSourceErrorKindUnknown, result.Error.Kind) + assert.True(t, result.FallbackToFDv1) + assert.Equal(t, 120*time.Second, result.FDv1FallbackTTL) }) } @@ -345,7 +381,7 @@ func TestPollingProcessorInitializerHandlesFallbackOnSuccessfulResponse(t *testi defer processor.Close() basis, fallback, err := processor.Fetch(ds, context.Background()) - assert.True(t, fallback) + assert.True(t, fallback.Requested) assert.NoError(t, err) // Even when the server signals fallback, a valid payload in the same response // should still be surfaced so the caller can apply it before switching protocols. @@ -374,7 +410,7 @@ func TestPollingProcessorInitializerHandlesFallbackOnErrorResponse(t *testing.T) basis, fallback, err := processor.Fetch(ds, context.Background()) assert.Nil(t, basis) - assert.True(t, fallback) + assert.True(t, fallback.Requested) assert.Error(t, err, "underlying HTTP error should still be surfaced alongside the fallback signal") }) } @@ -395,7 +431,7 @@ func TestPollingProcessorInitializerErrorWithoutFallbackHeaderReturnsRegularErro basis, fallback, err := processor.Fetch(ds, context.Background()) assert.Nil(t, basis) - assert.False(t, fallback) + assert.False(t, fallback.Requested) assert.Error(t, err) }) } diff --git a/internal/datasourcev2/polling_http_request.go b/internal/datasourcev2/polling_http_request.go index 5f52255ea..144c295b1 100644 --- a/internal/datasourcev2/polling_http_request.go +++ b/internal/datasourcev2/polling_http_request.go @@ -37,6 +37,16 @@ func (e malformedJSONError) Error() string { return e.innerError.Error() } +// goodbyeError is returned when a polling payload ends with a goodbye event that tells the SDK +// to fall back to FDv1, so the payload carries no data. +type goodbyeError struct { + reason string +} + +func (e goodbyeError) Error() string { + return fmt.Sprintf("polling payload ended with goodbye: %s", e.reason) +} + func newPollingRequester( context subsystems.ClientContext, httpClient *http.Client, @@ -76,22 +86,34 @@ func (r *pollingRequester) FilterKey() string { func (r *pollingRequester) Request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, error) { +) (*subsystems.ChangeSet, http.Header, subsystems.FDv1Fallback, error) { + changeSet, headers, goodbyeFallback, err := r.request(ctx, selector) + fallback := fdv1FallbackFromHeaders(headers) + if goodbyeFallback.Requested { + fallback = goodbyeFallback + } + return changeSet, headers, fallback, err +} + +func (r *pollingRequester) request( + ctx context.Context, + selector subsystems.Selector, +) (*subsystems.ChangeSet, http.Header, subsystems.FDv1Fallback, error) { if r.loggers.IsDebugEnabled() { r.loggers.Debug("Polling LaunchDarkly for feature flag updates") } body, cached, headers, err := r.makeRequest(ctx, endpoints.PollingRequestV2Path, selector) if err != nil { - return nil, headers, err + return nil, headers, subsystems.FDv1Fallback{}, err } if cached { - return subsystems.NewChangeSetBuilder().NoChanges(), headers, nil + return subsystems.NewChangeSetBuilder().NoChanges(), headers, subsystems.FDv1Fallback{}, nil } var payload subsystems.PollingPayload if err = json.Unmarshal(body, &payload); err != nil { - return nil, headers, malformedJSONError{err} + return nil, headers, subsystems.FDv1Fallback{}, malformedJSONError{err} } changeSet := subsystems.NewChangeSetBuilder().WithDeserializeOptions(r.deserializeOptions) @@ -99,43 +121,52 @@ func (r *pollingRequester) Request( for _, event := range payload.Events { select { case <-ctx.Done(): - return nil, headers, ctx.Err() + return nil, headers, subsystems.FDv1Fallback{}, ctx.Err() default: switch event.Name { case subsystems.EventServerIntent: var serverIntent subsystems.ServerIntent err := json.Unmarshal(event.Data, &serverIntent) if err != nil { - return nil, headers, err + return nil, headers, subsystems.FDv1Fallback{}, err } if serverIntent.Payload.Code == subsystems.IntentNone { - return changeSet.NoChanges(), headers, nil + return changeSet.NoChanges(), headers, subsystems.FDv1Fallback{}, nil } changeSet.Start(serverIntent) case subsystems.EventPutObject: var put subsystems.PutObject if err := json.Unmarshal(event.Data, &put); err != nil { - return nil, headers, err + return nil, headers, subsystems.FDv1Fallback{}, err } changeSet.AddPut(put.Kind, put.Key, put.Version, put.Object) case subsystems.EventDeleteObject: var deleteObject subsystems.DeleteObject if err := json.Unmarshal(event.Data, &deleteObject); err != nil { - return nil, headers, err + return nil, headers, subsystems.FDv1Fallback{}, err } changeSet.AddDelete(deleteObject.Kind, deleteObject.Key, deleteObject.Version) + case subsystems.EventGoodbye: + var goodbye subsystems.Goodbye + if err := json.Unmarshal(event.Data, &goodbye); err != nil { + return nil, headers, subsystems.FDv1Fallback{}, err + } + if fallback := fdv1FallbackFromGoodbye(goodbye); fallback.Requested { + return nil, headers, fallback, goodbyeError{reason: goodbye.Reason} + } case subsystems.EventPayloadTransferred: var selector subsystems.Selector if err := json.Unmarshal(event.Data, &selector); err != nil { - return nil, headers, err + return nil, headers, subsystems.FDv1Fallback{}, err } changeset, err := changeSet.Finish(selector) - return changeset, headers, err + return changeset, headers, subsystems.FDv1Fallback{}, err } } } - return nil, headers, fmt.Errorf("didn't receive any known protocol events in polling payload") + err = fmt.Errorf("didn't receive any known protocol events in polling payload") + return nil, headers, subsystems.FDv1Fallback{}, err } func (r *pollingRequester) makeRequest( diff --git a/internal/datasourcev2/polling_http_request_test.go b/internal/datasourcev2/polling_http_request_test.go index 4453aa416..1714de803 100644 --- a/internal/datasourcev2/polling_http_request_test.go +++ b/internal/datasourcev2/polling_http_request_test.go @@ -33,7 +33,7 @@ func TestV2RequestorImplRequestAll(t *testing.T) { httphelpers.WithServer(handler, func(ts *httptest.Server) { r := newPollingRequester(sharedtest.BasicClientContext(), nil, ts.URL, filter.key) - changeSet, _, err := r.Request(context.Background(), subsystems.NoSelector()) + changeSet, _, _, err := r.Request(context.Background(), subsystems.NoSelector()) assert.NoError(t, err) assert.Equal(t, subsystems.IntentTransferFull, changeSet.IntentCode()) @@ -51,7 +51,7 @@ func TestV2RequestorImplRequestAll(t *testing.T) { httphelpers.WithServer(handler, func(ts *httptest.Server) { r := newPollingRequester(sharedtest.BasicClientContext(), nil, ts.URL, filter.key) - changeSet, _, err := r.Request(context.Background(), subsystems.NoSelector()) + changeSet, _, _, err := r.Request(context.Background(), subsystems.NoSelector()) assert.Error(t, err) if he, ok := err.(httpStatusError); assert.True(t, ok) { @@ -69,7 +69,7 @@ func TestV2RequestorImplRequestAll(t *testing.T) { }) r := newPollingRequester(sharedtest.BasicClientContext(), nil, closedServerURL, filter.key) - changeSet, _, err := r.Request(context.Background(), subsystems.NoSelector()) + changeSet, _, _, err := r.Request(context.Background(), subsystems.NoSelector()) assert.Error(t, err) assert.Nil(t, changeSet) @@ -80,7 +80,7 @@ func TestV2RequestorImplRequestAll(t *testing.T) { httphelpers.WithServer(handler, func(ts *httptest.Server) { r := newPollingRequester(sharedtest.BasicClientContext(), nil, ts.URL, filter.key) - changeSet, _, err := r.Request(context.Background(), subsystems.NoSelector()) + changeSet, _, _, err := r.Request(context.Background(), subsystems.NoSelector()) require.Error(t, err) _, ok := err.(malformedJSONError) @@ -92,7 +92,7 @@ func TestV2RequestorImplRequestAll(t *testing.T) { t.Run("malformed base URI", func(t *testing.T) { r := newPollingRequester(sharedtest.BasicClientContext(), nil, "::::", filter.key) - changeSet, _, err := r.Request(context.Background(), subsystems.NoSelector()) + changeSet, _, _, err := r.Request(context.Background(), subsystems.NoSelector()) require.Error(t, err) assert.Contains(t, err.Error(), "missing protocol scheme") @@ -112,7 +112,7 @@ func TestV2RequestorImplRequestAll(t *testing.T) { httphelpers.WithServer(handler, func(ts *httptest.Server) { r := newPollingRequester(ldcontext, nil, ts.URL, filter.key) - _, _, err := r.Request(context.Background(), subsystems.NoSelector()) + _, _, _, err := r.Request(context.Background(), subsystems.NoSelector()) assert.NoError(t, err) req := <-requestsCh @@ -132,7 +132,7 @@ func TestV2RequestorImplRequestAll(t *testing.T) { httphelpers.WithServer(handler, func(ts *httptest.Server) { r := newPollingRequester(ldcontext, nil, ts.URL, filter.key) - _, _, err := r.Request(context.Background(), subsystems.NoSelector()) + _, _, _, err := r.Request(context.Background(), subsystems.NoSelector()) assert.NoError(t, err) assert.Equal(t, []string{"Polling LaunchDarkly for feature flag updates"}, @@ -159,7 +159,7 @@ func TestV2RequestorImplCaching(t *testing.T) { httphelpers.WithServer(handler, func(ts *httptest.Server) { r := newPollingRequester(sharedtest.BasicClientContext(), nil, ts.URL, filter.key) - changeSet1, _, err1 := r.Request(context.Background(), subsystems.NoSelector()) + changeSet1, _, _, err1 := r.Request(context.Background(), subsystems.NoSelector()) assert.NoError(t, err1) assert.NotNil(t, changeSet1) @@ -171,7 +171,7 @@ func TestV2RequestorImplCaching(t *testing.T) { assert.Equal(t, "", req1.Request.Header.Get("If-None-Match")) - changeSet2, _, err2 := r.Request(context.Background(), selector) + changeSet2, _, _, err2 := r.Request(context.Background(), selector) assert.NoError(t, err2) assert.Equal(t, subsystems.IntentNone, changeSet2.IntentCode()) @@ -184,7 +184,7 @@ func TestV2RequestorImplCaching(t *testing.T) { assert.Equal(t, "", req2.Request.Header.Get("If-None-Match")) - changeSet3, _, err3 := r.Request(context.Background(), selector) + changeSet3, _, _, err3 := r.Request(context.Background(), selector) assert.NoError(t, err3) assert.Equal(t, subsystems.IntentNone, changeSet3.IntentCode()) @@ -210,7 +210,7 @@ func TestV2RequestorImplCanUseCustomHTTPClientFactory(t *testing.T) { httphelpers.WithServer(pollHandler, func(ts *httptest.Server) { r := newPollingRequester(ldcontext, nil, ts.URL, "") - _, _, _ = r.Request(context.Background(), subsystems.NoSelector()) + _, _, _, _ = r.Request(context.Background(), subsystems.NoSelector()) req := <-requestsCh @@ -226,7 +226,7 @@ func TestV2RequestorImplCanAppendsFilterParameter(t *testing.T) { httphelpers.WithServer(pollHandler, func(ts *httptest.Server) { r := newPollingRequester(sharedtest.BasicClientContext(), nil, ts.URL, filter.key) - _, _, _ = r.Request(context.Background(), subsystems.NoSelector()) + _, _, _, _ = r.Request(context.Background(), subsystems.NoSelector()) req := <-requestsCh @@ -243,7 +243,7 @@ func TestV2RequestorImplCanAppendsBasis(t *testing.T) { httphelpers.WithServer(pollHandler, func(ts *httptest.Server) { r := newPollingRequester(sharedtest.BasicClientContext(), nil, ts.URL, filter.key) - _, _, _ = r.Request(context.Background(), subsystems.NewSelector("test-state", 1)) + _, _, _, _ = r.Request(context.Background(), subsystems.NewSelector("test-state", 1)) req := <-requestsCh diff --git a/internal/datasourcev2/release_clause_values_test.go b/internal/datasourcev2/release_clause_values_test.go index e0a2aaa75..3dc938d18 100644 --- a/internal/datasourcev2/release_clause_values_test.go +++ b/internal/datasourcev2/release_clause_values_test.go @@ -76,7 +76,7 @@ func TestPollingRequesterReleaseClauseValues(t *testing.T) { httphelpers.WithServer(ldservices.ServerSidePollingV2ServiceHandler(payload), func(ts *httptest.Server) { r := newPollingRequester(makeReleaseClauseValuesContext(release), nil, ts.URL, "") - changeSet, _, err := r.Request(context.Background(), subsystems.NoSelector()) + changeSet, _, _, err := r.Request(context.Background(), subsystems.NoSelector()) require.NoError(t, err) assertChangeSetClauseValuesReleased(t, release, changeSet) @@ -95,7 +95,7 @@ func TestFDv1FallbackPollingReleaseClauseValues(t *testing.T) { p := NewFDv1PollingProcessor(makeReleaseClauseValuesContext(release), datasource.PollingConfig{BaseURI: ts.URL}) - changeSet, _, err := p.requester.Request(context.Background(), subsystems.NoSelector()) + changeSet, _, _, err := p.requester.Request(context.Background(), subsystems.NoSelector()) require.NoError(t, err) assertChangeSetClauseValuesReleased(t, release, changeSet) diff --git a/internal/datasourcev2/streaming_data_source.go b/internal/datasourcev2/streaming_data_source.go index a448ad6d3..bc72b5a38 100644 --- a/internal/datasourcev2/streaming_data_source.go +++ b/internal/datasourcev2/streaming_data_source.go @@ -109,8 +109,11 @@ func (sp *StreamProcessor) Name() string { } //nolint:revive // DataInitializer method. -func (sp *StreamProcessor) Fetch(ds subsystems.DataSelector, _ context.Context) (*subsystems.Basis, bool, error) { - return nil, false, errors.New("StreamProcessor does not implement Fetch capability") +func (sp *StreamProcessor) Fetch( + ds subsystems.DataSelector, + _ context.Context, +) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return nil, subsystems.FDv1Fallback{}, errors.New("StreamProcessor does not implement Fetch capability") } //nolint:revive // DataSynchronizer method. @@ -174,10 +177,10 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su changeSetBuilder := subsystems.NewChangeSetBuilder().WithDeserializeOptions(sp.deserializeOptions) environmentID := ldvalue.OptionalString{} - // fallbackRequested is set when the server's response headers carry x-ld-fd-fallback: true. - // We finish applying the current payload before emitting the fallback signal, so evaluations + // fallback is set when the server's response headers carry x-ld-fd-fallback: true. We + // finish applying the current payload before emitting the fallback signal, so evaluations // can serve the server-provided data while FDv1 takes over. - fallbackRequested := false + fallback := subsystems.FDv1Fallback{} for { select { @@ -200,8 +203,8 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su if eventWithHeaders, ok := event.(es.EventWithHeaders); ok { headers := eventWithHeaders.Headers() environmentID = internal.NewInitMetadataFromHeaders(headers).GetEnvironmentID() - if isFDv1FallbackRequested(headers) { - fallbackRequested = true + if headerFallback := fdv1FallbackFromHeaders(headers); headerFallback.Requested { + fallback = headerFallback } } @@ -235,9 +238,10 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su } resultChan <- subsystems.DataSynchronizerResult{ - State: interfaces.DataSourceStateValid, - EnvironmentID: environmentID, - FallbackToFDv1: fallbackRequested, + State: interfaces.DataSourceStateValid, + EnvironmentID: environmentID, + FallbackToFDv1: fallback.Requested, + FDv1FallbackTTL: fallback.TTL, } payloadApplied = true break @@ -268,6 +272,19 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su } sp.loggers.Infof("SSE server sent goodbye: %s", goodbye.Reason) + + // A goodbye with a fallback TTL tells the SDK to fall back to FDv1. Any + // payload in progress is incomplete, so it is discarded. + if goodbyeFallback := fdv1FallbackFromGoodbye(goodbye); goodbyeFallback.Requested { + resultChan <- subsystems.DataSynchronizerResult{ + State: interfaces.DataSourceStateInterrupted, + EnvironmentID: environmentID, + FallbackToFDv1: true, + FDv1FallbackTTL: goodbyeFallback.TTL, + } + stream.Close() + return + } case subsystems.EventError: var errorData subsystems.Error err := json.Unmarshal([]byte(event.Data()), &errorData) @@ -302,10 +319,11 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su } resultChan <- subsystems.DataSynchronizerResult{ - ChangeSet: changeSet, - State: interfaces.DataSourceStateValid, - EnvironmentID: environmentID, - FallbackToFDv1: fallbackRequested, + ChangeSet: changeSet, + State: interfaces.DataSourceStateValid, + EnvironmentID: environmentID, + FallbackToFDv1: fallback.Requested, + FDv1FallbackTTL: fallback.TTL, } payloadApplied = true @@ -320,8 +338,8 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su // Once a payload has been applied with a pending FDv1 fallback signal, the Valid // result emitted above carries FallbackToFDv1=true; close the stream so we stop // consuming. Events that don't complete a payload leave payloadApplied false so we - // keep consuming (fallbackRequested persists across iterations). - if fallbackRequested && payloadApplied { + // keep consuming (fallback persists across iterations). + if fallback.Requested && payloadApplied { stream.Close() return } @@ -384,12 +402,13 @@ func (sp *StreamProcessor) subscribe(ds subsystems.DataSelector, resultChan chan Time: time.Now(), } - if isFDv1FallbackRequested(se.Header) { + if fallback := fdv1FallbackFromHeaders(se.Header); fallback.Requested { resultChan <- subsystems.DataSynchronizerResult{ - State: interfaces.DataSourceStateOff, - Error: errorInfo, - FallbackToFDv1: true, - EnvironmentID: environmentID, + State: interfaces.DataSourceStateOff, + Error: errorInfo, + FallbackToFDv1: true, + FDv1FallbackTTL: fallback.TTL, + EnvironmentID: environmentID, } return es.StreamErrorHandlerResult{CloseNow: true} } diff --git a/internal/datasourcev2/streaming_data_source_test.go b/internal/datasourcev2/streaming_data_source_test.go index d9c0e41d4..e65b3f5ce 100644 --- a/internal/datasourcev2/streaming_data_source_test.go +++ b/internal/datasourcev2/streaming_data_source_test.go @@ -62,7 +62,7 @@ func TestStreamingDoesNotWorkAsInitializer(t *testing.T) { defer sp.Close() basis, fallback, err := sp.Fetch(ds, context.Background()) assert.Nil(t, basis) - assert.False(t, fallback) + assert.False(t, fallback.Requested) assert.NotNil(t, err) } @@ -164,7 +164,8 @@ func TestStreamingProcessorHandlesFallbackToFDv1(t *testing.T) { ds := mocks.NewMockDataSelector(subsystems.NoSelector()) header := http.Header{ - "X-LD-FD-Fallback": []string{"true"}, + "X-LD-FD-Fallback": []string{"true"}, + "X-LD-FD-Fallback-TTL": []string{"120"}, } handler, _ := httphelpers.RecordingHandler(httphelpers.HandlerWithResponse(500, header, nil)) httphelpers.WithServer(handler, func(ts *httptest.Server) { @@ -183,6 +184,30 @@ func TestStreamingProcessorHandlesFallbackToFDv1(t *testing.T) { assert.Equal(t, result.State, interfaces.DataSourceStateOff) assert.Equal(t, result.Error.Kind, interfaces.DataSourceErrorKindErrorResponse) assert.True(t, result.FallbackToFDv1) + assert.Equal(t, 120*time.Second, result.FDv1FallbackTTL) + }) +} + +func TestStreamingProcessorFallsBackOnGoodbyeWithFallbackTTL(t *testing.T) { + ds := mocks.NewMockDataSelector(subsystems.NoSelector()) + + protocol := ldservicesv2.NewStreamingProtocol(). + WithIntent(subsystems.ServerIntent{Payload: subsystems.Payload{ + ID: "something-id", Target: 0, Code: subsystems.IntentTransferFull, Reason: "payload-missing", + }}). + WithGoodbye(subsystems.Goodbye{Reason: "fall back", ProtocolFallbackTTL: json.RawMessage("120")}) + + streamHandler, _ := ldservices.ServerSideStreamingV2ServiceProtocolHandler(protocol) + httphelpers.WithServer(streamHandler, func(ts *httptest.Server) { + sp := NewStreamProcessor(sharedtest.BasicClientContext(), datasource.StreamConfig{URI: ts.URL}) + defer sp.Close() + + result := <-sp.Sync(ds) + + assert.Equal(t, interfaces.DataSourceStateInterrupted, result.State) + assert.Nil(t, result.ChangeSet) + assert.True(t, result.FallbackToFDv1) + assert.Equal(t, 120*time.Second, result.FDv1FallbackTTL) }) } diff --git a/internal/datasystem/fdv2_datasystem.go b/internal/datasystem/fdv2_datasystem.go index 5013234db..d54e99f5e 100644 --- a/internal/datasystem/fdv2_datasystem.go +++ b/internal/datasystem/fdv2_datasystem.go @@ -3,6 +3,7 @@ package datasystem import ( "context" "errors" + "math/rand" "sync" "time" @@ -235,17 +236,9 @@ func (f *FDv2) launchTask(task func()) { func (f *FDv2) run(ctx context.Context, closeWhenReady chan struct{}) { f.UpdateStatus(interfaces.DataSourceStateInitializing, interfaces.DataSourceErrorInfo{}) - if fallback, errorInfo := f.runInitializers(ctx, closeWhenReady); fallback { - if f.synchronizers.hasFDv1Fallback() { - f.loggers.Warn("Falling back to FDv1 protocol") - // Block every FDv2 synchronizer and make the FDv1 fallback synchronizer available. - f.synchronizers.fdv1Fallback() - } else { - f.loggers.Warn("Initializer requested FDv1 fallback but none configured") - // Block every synchronizer so that none starts. - f.synchronizers.blockAll() - f.UpdateStatus(interfaces.DataSourceStateOff, errorInfo) - } + var recovery *time.Timer + if fallback, errorInfo := f.runInitializers(ctx, closeWhenReady); fallback.Requested { + recovery = f.fallBackToFDv1(fallback.TTL, errorInfo) } if f.configuredWithDataSources && f.dataStoreStatusProvider.IsStatusMonitoringEnabled() { @@ -254,7 +247,54 @@ func (f *FDv2) run(ctx context.Context, closeWhenReady chan struct{}) { }) } - f.runSynchronizers(ctx, closeWhenReady) + f.runSynchronizers(ctx, closeWhenReady, recovery) +} + +const ( + maxFDv1FallbackTTL = time.Hour + defaultFDv1FallbackTTL = time.Hour + maxFDv1FallbackTTLJitter = 30 * time.Minute + fdv1FallbackTTLJitterRange = int64(maxFDv1FallbackTTLJitter) + 1 +) + +// fdv1FallbackDuration returns how long to stay on FDv1 before trying FDv2 again. A TTL from +// LaunchDarkly in (0, 1 hour] is used as-is. Any other TTL means the 1 hour default, minus a +// random jitter of up to 30 minutes. +func fdv1FallbackDuration(ttl time.Duration) time.Duration { + if ttl > 0 && ttl <= maxFDv1FallbackTTL { + return ttl + } + //nolint:gosec // jitter does not need a cryptographically secure source + return defaultFDv1FallbackTTL - time.Duration(rand.Int63n(fdv1FallbackTTLJitterRange)) +} + +// fallBackToFDv1 pauses every FDv2 synchronizer and makes the FDv1 fallback synchronizer +// available, if there is one. It returns a timer that fires when it is time to try FDv2 again, +// or nil if there is no synchronizer to recover to. errorInfo is reported in the status if there +// is no FDv1 fallback synchronizer. +func (f *FDv2) fallBackToFDv1(ttl time.Duration, errorInfo interfaces.DataSourceErrorInfo) *time.Timer { + if !f.synchronizers.hasFDv1Fallback() && f.synchronizers.availableCount() == 0 { + f.loggers.Warn("FDv1 fallback requested but no synchronizers are configured") + f.UpdateStatus(interfaces.DataSourceStateOff, errorInfo) + return nil + } + duration := fdv1FallbackDuration(ttl) + if f.synchronizers.hasFDv1Fallback() { + f.loggers.Warnf("Falling back to FDv1 protocol; will try FDv2 again in %s", duration) + } else { + f.loggers.Warnf("FDv1 fallback requested but none configured; will try FDv2 again in %s", duration) + f.UpdateStatus(interfaces.DataSourceStateInterrupted, errorInfo) + } + f.synchronizers.fdv1Fallback() + return time.NewTimer(duration) +} + +// recoverFDv2 ends an FDv1 fallback. The FDv1 fallback synchronizer, if any, must already be +// closed, so that FDv1 and FDv2 never write to the store at the same time. +func (f *FDv2) recoverFDv2() { + f.loggers.Info("FDv1 fallback TTL has expired, trying FDv2 again") + f.synchronizers.fdv2Recovery() + f.synchronizers.next() } func (f *FDv2) runPersistentStoreOutageRecovery(ctx context.Context, statuses <-chan interfaces.DataStoreStatus) { @@ -285,7 +325,7 @@ func (f *FDv2) runPersistentStoreOutageRecovery(ctx context.Context, statuses <- // applied to the store, even if that data had no selector. A basis that carries no data, or data // that cannot be applied, does not count; the synchronizers then decide readiness. // -// fallbackToFDv1 is true when an initializer asked the SDK to switch to FDv1. If the fallback is +// fallback.Requested is true when an initializer asked the SDK to switch to FDv1. If the fallback is // signalled alongside a basis, that basis is applied before returning, so evaluations can serve // the server-provided data while the FDv1 synchronizer spins up. If any initializer's data was // applied by then, initialization is complete before returning. @@ -294,16 +334,16 @@ func (f *FDv2) runPersistentStoreOutageRecovery(ctx context.Context, statuses <- // configured. It is empty when the fallback accompanied a successful response. func (f *FDv2) runInitializers( ctx context.Context, closeWhenReady chan struct{}, -) (fallbackToFDv1 bool, errorInfo interfaces.DataSourceErrorInfo) { +) (fallback subsystems.FDv1Fallback, errorInfo interfaces.DataSourceErrorInfo) { // Name of the last initializer whose data was applied to the store. appliedFrom := "" for _, initializer := range f.initializers { f.loggers.Infof("Attempting to initialize via %s", initializer.Name()) basis, fallback, err := initializer.Fetch(f.store, ctx) if errors.Is(err, context.Canceled) { - return false, interfaces.DataSourceErrorInfo{} + return subsystems.FDv1Fallback{}, interfaces.DataSourceErrorInfo{} } - if fallback { + if fallback.Requested { if err != nil { f.loggers.Warnf("Initializer %s requested fallback to FDv1 protocol: %v", initializer.Name(), err) errorInfo = interfaces.DataSourceErrorInfo{ @@ -321,7 +361,7 @@ func (f *FDv2) runInitializers( f.loggers.Infof("Initialized via %s before falling back to FDv1", appliedFrom) f.completeInitialization(closeWhenReady) } - return true, errorInfo + return fallback, errorInfo } if err != nil { f.loggers.Warnf("Initializer %s failed: %v", initializer.Name(), err) @@ -334,14 +374,14 @@ func (f *FDv2) runInitializers( if basis.ChangeSet.Selector().IsDefined() { f.loggers.Infof("Initialized via %s", initializer.Name()) f.completeInitialization(closeWhenReady) - return false, interfaces.DataSourceErrorInfo{} + return subsystems.FDv1Fallback{}, interfaces.DataSourceErrorInfo{} } } if appliedFrom != "" { f.loggers.Infof("Initialized via %s; the data has no selector", appliedFrom) f.completeInitialization(closeWhenReady) } - return false, interfaces.DataSourceErrorInfo{} + return subsystems.FDv1Fallback{}, interfaces.DataSourceErrorInfo{} } // applyBasis applies an initializer's basis to the store. It reports whether the store received @@ -366,9 +406,11 @@ func (f *FDv2) completeInitialization(closeWhenReady chan struct{}) { }) } -func (f *FDv2) runSynchronizers(ctx context.Context, closeWhenReady chan struct{}) { +// runSynchronizers runs the synchronizers. If recovery is not nil, an FDv1 fallback is in +// effect, and FDv2 recovers when the timer fires. +func (f *FDv2) runSynchronizers(ctx context.Context, closeWhenReady chan struct{}, recovery *time.Timer) { // If no synchronizers are available, close ready channel and return - if f.synchronizers.availableCount() == 0 { + if f.synchronizers.availableCount() == 0 && recovery == nil { f.readyOnce.Do(func() { close(closeWhenReady) }) @@ -380,14 +422,33 @@ func (f *FDv2) runSynchronizers(ctx context.Context, closeWhenReady chan struct{ defer f.readyOnce.Do(func() { close(closeWhenReady) }) + defer func() { + if recovery != nil { + recovery.Stop() + } + }() f.synchronizers.next() for { // Check if we've run out of synchronizers if f.synchronizers.availableCount() == 0 { - f.loggers.Warn("No more synchronizers available") - f.UpdateStatus(interfaces.DataSourceStateOff, f.getStatus().LastError) - return + if recovery == nil { + f.loggers.Warn("No more synchronizers available") + f.UpdateStatus(interfaces.DataSourceStateOff, f.getStatus().LastError) + return + } + // Nothing can run until FDv2 recovers, so stop waiting for initialization. + f.readyOnce.Do(func() { + close(closeWhenReady) + }) + select { + case <-ctx.Done(): + return + case <-recovery.C: + } + recovery = nil + f.recoverFDv2() + continue } // Build synchronizer @@ -401,7 +462,7 @@ func (f *FDv2) runSynchronizers(ctx context.Context, closeWhenReady chan struct{ f.loggers.Infof("Synchronizer at index %d (%s) is starting", f.synchronizers.currentIndex(), sync.Name()) resultChan := sync.Sync(f.store) - action, err := f.consumeSynchronizerResults(ctx, resultChan, closeWhenReady) + action, fallbackTTL, err := f.consumeSynchronizerResults(ctx, resultChan, closeWhenReady, timerChannel(recovery)) if err := sync.Close(); err != nil { f.loggers.Errorf("Synchronizer %s failed to close: %v", sync.Name(), err) @@ -414,16 +475,15 @@ func (f *FDv2) runSynchronizers(ctx context.Context, closeWhenReady chan struct{ // Handle action based on conditions switch action { case syncFDv1: - if f.synchronizers.hasFDv1Fallback() { - f.loggers.Warn("Falling back to FDv1 protocol") - // Block every FDv2 synchronizer and make the FDv1 fallback synchronizer available. - f.synchronizers.fdv1Fallback() - f.synchronizers.next() - continue + if recovery == nil { + recovery = f.fallBackToFDv1(fallbackTTL, f.getStatus().LastError) } - f.loggers.Warn("Synchronizer requested FDv1 fallback but none configured") - f.UpdateStatus(interfaces.DataSourceStateOff, f.getStatus().LastError) - return + f.synchronizers.next() + continue + case syncFDv2Recovery: + recovery = nil + f.recoverFDv2() + continue case syncBlock: f.loggers.Warnf("Blocking synchronizer at index %d because it failed permanently", f.synchronizers.currentIndex()) @@ -458,26 +518,38 @@ const ( syncRecover syncBlock syncFDv1 + syncFDv2Recovery ) +// timerChannel returns the timer's channel, or nil if there is no timer. +func timerChannel(timer *time.Timer) <-chan time.Time { + if timer == nil { + return nil + } + return timer.C +} + func (f *FDv2) consumeSynchronizerResults( ctx context.Context, resultChan <-chan subsystems.DataSynchronizerResult, closeWhenReady chan<- struct{}, -) (action syncAction, err error) { + recovery <-chan time.Time, +) (action syncAction, fdv1FallbackTTL time.Duration, err error) { ticker := time.NewTicker(10 * time.Second) defer ticker.Stop() for { select { case <-ctx.Done(): - return syncFallback, ctx.Err() + return syncFallback, 0, ctx.Err() + case <-recovery: + return syncFDv2Recovery, 0, nil case result, ok := <-resultChan: // The status channel being closed means that we won't be receiving // any more information from that synchronizer and we should // probably fall back. if !ok { - return syncFallback, nil + return syncFallback, 0, nil } if result.EnvironmentID.IsDefined() { @@ -502,9 +574,9 @@ func (f *FDv2) consumeSynchronizerResults( case interfaces.DataSourceStateOff: f.UpdateStatus(interfaces.DataSourceStateInterrupted, result.Error) if result.FallbackToFDv1 { - return syncFDv1, nil + return syncFDv1, result.FDv1FallbackTTL, nil } - return syncBlock, nil + return syncBlock, 0, nil } // FallbackToFDv1 may ride along on a Valid or Interrupted result too -- e.g. a @@ -512,7 +584,7 @@ func (f *FDv2) consumeSynchronizerResults( // Interrupted branches above already applied any ChangeSet and updated status; // now hand control to the FDv1 fallback synchronizer. if result.FallbackToFDv1 { - return syncFDv1, nil + return syncFDv1, result.FDv1FallbackTTL, nil } case <-ticker.C: // If there's only one available synchronizer, don't check conditions @@ -526,13 +598,13 @@ func (f *FDv2) consumeSynchronizerResults( // Check fallback condition first (things are bad) if f.fallbackCond(status) { f.loggers.Debugf("Fallback condition met") - return syncFallback, nil + return syncFallback, 0, nil } // If not at the first available synchronizer, also check recovery condition (things are good) if !f.synchronizers.isFirstAvailable() && f.recoveryCond(status) { f.loggers.Debugf("Recovery condition met") - return syncRecover, nil + return syncRecover, 0, nil } f.loggers.Debugf("No condition met, continue with current synchronizer") diff --git a/internal/datasystem/fdv2_fallback_test.go b/internal/datasystem/fdv2_fallback_test.go index 1cef3b9ef..a7c5ac798 100644 --- a/internal/datasystem/fdv2_fallback_test.go +++ b/internal/datasystem/fdv2_fallback_test.go @@ -39,8 +39,8 @@ func (f *fakeSynchronizer) Name() string { return f.name } func (f *fakeSynchronizer) Fetch( _ subsystems.DataSelector, _ context.Context, -) (*subsystems.Basis, bool, error) { - return nil, false, nil +) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return nil, subsystems.FDv1Fallback{}, nil } func (f *fakeSynchronizer) Sync(_ subsystems.DataSelector) <-chan subsystems.DataSynchronizerResult { diff --git a/internal/datasystem/fdv2_fdv1_recovery_test.go b/internal/datasystem/fdv2_fdv1_recovery_test.go new file mode 100644 index 000000000..61229b92c --- /dev/null +++ b/internal/datasystem/fdv2_fdv1_recovery_test.go @@ -0,0 +1,191 @@ +package datasystem + +import ( + "context" + "testing" + "time" + + "github.com/launchdarkly/go-sdk-common/v3/ldlog" + "github.com/launchdarkly/go-sdk-common/v3/ldlogtest" + "github.com/launchdarkly/go-server-sdk/v7/interfaces" + "github.com/launchdarkly/go-server-sdk/v7/internal" + "github.com/launchdarkly/go-server-sdk/v7/internal/sharedtest" + "github.com/launchdarkly/go-server-sdk/v7/subsystems" + th "github.com/launchdarkly/go-test-helpers/v3" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A TTL that is valid, so that it is used as-is, and short, so that recovery is quick. +const shortFallbackTTL = 10 * time.Millisecond + +// lifecycleSynchronizer is a DataSynchronizer that reports when it starts and closes. Each +// build of a lifecycleBuilder creates a new one. +type lifecycleSynchronizer struct { + name string + results chan subsystems.DataSynchronizerResult + events chan<- string +} + +func (s *lifecycleSynchronizer) Name() string { return s.name } + +func (s *lifecycleSynchronizer) Fetch( + _ subsystems.DataSelector, + _ context.Context, +) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return nil, subsystems.FDv1Fallback{}, nil +} + +func (s *lifecycleSynchronizer) Sync(_ subsystems.DataSelector) <-chan subsystems.DataSynchronizerResult { + s.events <- s.name + " started" + return s.results +} + +func (s *lifecycleSynchronizer) Close() error { + s.events <- s.name + " closed" + return nil +} + +type lifecycleBuilder struct { + name string + events chan string + built chan *lifecycleSynchronizer +} + +func newLifecycleBuilder(name string, events chan string) *lifecycleBuilder { + return &lifecycleBuilder{name: name, events: events, built: make(chan *lifecycleSynchronizer, 10)} +} + +func (b *lifecycleBuilder) build() (subsystems.DataSynchronizer, error) { + s := &lifecycleSynchronizer{ + name: b.name, + results: make(chan subsystems.DataSynchronizerResult, 1), + events: b.events, + } + b.built <- s + return s, nil +} + +// fallbackInitializer is a DataInitializer that requests an FDv1 fallback. +type fallbackInitializer struct { + ttl time.Duration +} + +func (i fallbackInitializer) Name() string { return "fallback-initializer" } + +func (i fallbackInitializer) Fetch( + _ subsystems.DataSelector, + _ context.Context, +) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return nil, subsystems.FDv1Fallback{Requested: true, TTL: i.ttl}, nil +} + +func makeRecoveryTestFDv2( + t *testing.T, + loggers ldlog.Loggers, + initializers []subsystems.DataInitializer, + fdv2 *lifecycleBuilder, + fdv1 *lifecycleBuilder, +) *FDv2 { + t.Helper() + synchronizers := subsystems.SynchronizersConfiguration{ + SynchronizerBuilders: []func() (subsystems.DataSynchronizer, error){fdv2.build}, + } + if fdv1 != nil { + synchronizers.FDv1FallbackBuilder = fdv1.build + } + + clientContext := &internal.ClientContextImpl{ + BasicClientContext: subsystems.BasicClientContext{ + SDKKey: sharedtest.TestSDKKey, + Logging: subsystems.LoggingConfiguration{Loggers: loggers}, + }, + } + + system, err := NewFDv2(false, fixedConfig{ + Initializers: initializers, + Synchronizers: synchronizers, + }, clientContext, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = system.Stop() }) + return system +} + +func requireEvents(t *testing.T, events <-chan string, want ...string) { + t.Helper() + for _, w := range want { + assert.Equal(t, w, th.RequireValue(t, events, time.Second, "timed out waiting for %q", w)) + } +} + +func TestFDv1FallbackDurationUsesValidTTLAsIs(t *testing.T) { + for _, ttl := range []time.Duration{time.Millisecond, time.Second, 120 * time.Second, time.Hour} { + assert.Equal(t, ttl, fdv1FallbackDuration(ttl), ttl.String()) + } +} + +func TestFDv1FallbackDurationUsesJitteredDefaultForInvalidTTL(t *testing.T) { + for _, ttl := range []time.Duration{0, -time.Second, time.Hour + time.Second, 24 * time.Hour} { + for i := 0; i < 100; i++ { + duration := fdv1FallbackDuration(ttl) + assert.GreaterOrEqual(t, duration, 30*time.Minute, ttl.String()) + assert.LessOrEqual(t, duration, time.Hour, ttl.String()) + } + } +} + +func TestFDv2RecoversAfterSynchronizerFallbackWithoutFDv1Synchronizer(t *testing.T) { + events := make(chan string, 10) + fdv2 := newLifecycleBuilder("fdv2", events) + + system := makeRecoveryTestFDv2(t, ldlog.NewDisabledLoggers(), nil, fdv2, nil) + system.Start(make(chan struct{})) + + first := th.RequireValue(t, fdv2.built, time.Second) + first.results <- subsystems.DataSynchronizerResult{ + State: interfaces.DataSourceStateOff, + FallbackToFDv1: true, + FDv1FallbackTTL: shortFallbackTTL, + } + + // The FDv2 synchronizer is closed and, after the TTL, started again. + requireEvents(t, events, "fdv2 started", "fdv2 closed", "fdv2 started") + assert.Equal(t, interfaces.DataSourceStateInterrupted, system.getStatus().State) +} + +func TestFDv2RecoversAfterSynchronizerFallbackToFDv1Synchronizer(t *testing.T) { + events := make(chan string, 10) + fdv2 := newLifecycleBuilder("fdv2", events) + fdv1 := newLifecycleBuilder("fdv1", events) + + system := makeRecoveryTestFDv2(t, ldlog.NewDisabledLoggers(), nil, fdv2, fdv1) + system.Start(make(chan struct{})) + + first := th.RequireValue(t, fdv2.built, time.Second) + first.results <- subsystems.DataSynchronizerResult{ + State: interfaces.DataSourceStateValid, + FallbackToFDv1: true, + FDv1FallbackTTL: shortFallbackTTL, + } + + // Each synchronizer is closed before the next one starts, so FDv1 and FDv2 never write to + // the store at the same time. + requireEvents(t, events, "fdv2 started", "fdv2 closed", "fdv1 started", "fdv1 closed", "fdv2 started") +} + +func TestFDv2RecoversAfterInitializerFallbackWithoutFDv1Synchronizer(t *testing.T) { + mockLog := ldlogtest.NewMockLog() + defer mockLog.DumpIfTestFailed(t) + + events := make(chan string, 10) + fdv2 := newLifecycleBuilder("fdv2", events) + + system := makeRecoveryTestFDv2(t, mockLog.Loggers, + []subsystems.DataInitializer{fallbackInitializer{ttl: shortFallbackTTL}}, fdv2, nil) + system.Start(make(chan struct{})) + + // The FDv2 synchronizer only starts once the TTL has expired. + requireEvents(t, events, "fdv2 started") + assert.True(t, mockLog.HasMessageMatch(ldlog.Info, "FDv1 fallback TTL has expired")) +} diff --git a/internal/datasystem/synchronizer_list.go b/internal/datasystem/synchronizer_list.go index 174e426a8..4ccac9349 100644 --- a/internal/datasystem/synchronizer_list.go +++ b/internal/datasystem/synchronizer_list.go @@ -11,17 +11,19 @@ type synchronizerBuilder = func() (subsystems.DataSynchronizer, error) var errNoCurrentSynchronizer = errors.New("no current synchronizer") // synchronizerSlot holds one synchronizer builder and its state. A blocked slot is not -// eligible for use. +// eligible for use. A slot that is paused for an FDv1 fallback is also blocked, and is +// unblocked again when FDv2 recovers. type synchronizerSlot struct { build synchronizerBuilder isFDv1Fallback bool blocked bool + pausedForFDv1 bool } // synchronizerList is the ordered list of synchronizers that the data system can use. A // synchronizer that fails permanently stays in the list and is marked as blocked. The FDv1 // fallback synchronizer, if configured, is the last slot. It is blocked until an FDv1 -// fallback occurs. +// fallback occurs, and blocked again when FDv2 recovers. // // The list is not safe for concurrent use. Only the goroutine that runs the synchronizers // uses it. @@ -113,19 +115,35 @@ func (l *synchronizerList) hasFDv1Fallback() bool { return l.hasFDv1 } -// blockAll blocks every slot and clears the current slot. -func (l *synchronizerList) blockAll() { +// fdv1Fallback pauses every available FDv2 slot and unblocks the FDv1 fallback slot, if there +// is one. It also clears the current slot, so the next call to next selects the FDv1 fallback +// slot. +func (l *synchronizerList) fdv1Fallback() { for i := range l.slots { - l.slots[i].blocked = true + slot := &l.slots[i] + if slot.isFDv1Fallback { + slot.blocked = false + } else if !slot.blocked { + slot.blocked = true + slot.pausedForFDv1 = true + } } l.current = -1 } -// fdv1Fallback blocks every FDv2 slot and unblocks the FDv1 fallback slot. It also clears -// the current slot, so the next call to next selects the FDv1 fallback slot. -func (l *synchronizerList) fdv1Fallback() { +// fdv2Recovery undoes fdv1Fallback. It unblocks every FDv2 slot that fdv1Fallback paused and +// blocks the FDv1 fallback slot. An FDv2 slot that was blocked because it failed permanently +// stays blocked. It also clears the current slot, so the next call to next selects the first +// available FDv2 slot. +func (l *synchronizerList) fdv2Recovery() { for i := range l.slots { - l.slots[i].blocked = !l.slots[i].isFDv1Fallback + slot := &l.slots[i] + if slot.isFDv1Fallback { + slot.blocked = true + } else if slot.pausedForFDv1 { + slot.blocked = false + slot.pausedForFDv1 = false + } } l.current = -1 } diff --git a/internal/datasystem/synchronizer_list_test.go b/internal/datasystem/synchronizer_list_test.go index 4dddce59a..b8f9ef103 100644 --- a/internal/datasystem/synchronizer_list_test.go +++ b/internal/datasystem/synchronizer_list_test.go @@ -109,14 +109,43 @@ func TestSynchronizerListFDv1FallbackSlotIsBlockedUntilFallback(t *testing.T) { assert.Equal(t, 1, list.availableCount()) } -func TestSynchronizerListBlockAllLeavesNoSlotAvailable(t *testing.T) { - list := newSynchronizerList([]synchronizerBuilder{noopBuilder, noopBuilder}, noopBuilder) +func TestSynchronizerListFDv1FallbackWithoutFDv1SlotLeavesNoSlotAvailable(t *testing.T) { + list := newSynchronizerList([]synchronizerBuilder{noopBuilder, noopBuilder}, nil) list.next() - // Block every slot, including the FDv1 fallback slot. - list.blockAll() + list.fdv1Fallback() // No slot remains available. list.next() assert.Equal(t, -1, list.currentIndex()) } + +func TestSynchronizerListFDv2RecoveryRestoresPausedSlots(t *testing.T) { + list := newSynchronizerList([]synchronizerBuilder{noopBuilder, noopBuilder, noopBuilder}, noopBuilder) + + // Block the second slot as if it failed permanently. + list.next() + list.next() + list.blockCurrent() + + list.fdv1Fallback() + list.fdv2Recovery() + list.next() + + // The first slot is current again, the FDv1 slot is blocked, and the failed slot stays blocked. + assert.Equal(t, 0, list.currentIndex()) + assert.Equal(t, 2, list.availableCount()) + assert.True(t, list.slots[1].blocked) + assert.True(t, list.slots[3].blocked) +} + +func TestSynchronizerListFDv2RecoveryWithoutFDv1SlotRestoresPausedSlots(t *testing.T) { + list := newSynchronizerList([]synchronizerBuilder{noopBuilder, noopBuilder}, nil) + + list.fdv1Fallback() + list.fdv2Recovery() + list.next() + + assert.Equal(t, 0, list.currentIndex()) + assert.Equal(t, 2, list.availableCount()) +} diff --git a/ldclient_end_to_end_fdv2_test.go b/ldclient_end_to_end_fdv2_test.go index df5753216..586a79a49 100644 --- a/ldclient_end_to_end_fdv2_test.go +++ b/ldclient_end_to_end_fdv2_test.go @@ -123,9 +123,9 @@ func TestFDV2CanFallBackToV1(t *testing.T) { }) } -// When an initializer requests FDv1 fallback but no FDv1 fallback is configured, the data source -// status must transition to Off rather than staying stuck at Initializing. This mirrors the -// synchronizer-triggered path when no FDv1 fallback is configured. +// When an initializer requests FDv1 fallback and no synchronizer of any kind is configured, there +// is nothing to recover to, so the data source status must transition to Off rather than staying +// stuck at Initializing. func TestFDV2InitializerFallbackWithoutFDv1FallbackTransitionsToOff(t *testing.T) { header := http.Header{ "X-LD-FD-Fallback": []string{"true"}, @@ -167,15 +167,17 @@ func TestFDV2InitializerFallbackWithoutFDv1FallbackTransitionsToOff(t *testing.T assert.Equal(t, initializationFailedErrorMessage, err.Error()) assert.Contains(t, logCapture.GetOutput(ldlog.Warn), - "Initializer requested FDv1 fallback but none configured") + "FDv1 fallback requested but no synchronizers are configured") }) } // When a synchronizer requests FDv1 fallback but no FDv1 fallback is configured, the data -// source status must transition to Off. No other FDv2 synchronizer may start. -func TestFDV2SynchronizerFallbackWithoutFDv1FallbackTransitionsToOff(t *testing.T) { +// source status is Interrupted rather than Off, and the first FDv2 synchronizer starts again once +// the fallback TTL expires. +func TestFDV2SynchronizerFallbackWithoutFDv1FallbackRecoversAfterTTL(t *testing.T) { header := http.Header{ - "X-LD-FD-Fallback": []string{"true"}, + "X-LD-FD-Fallback": []string{"true"}, + "X-LD-FD-Fallback-TTL": []string{"1"}, } handler, requestsCh := httphelpers.RecordingHandler(httphelpers.HandlerWithResponse(500, header, nil)) @@ -198,15 +200,16 @@ func TestFDV2SynchronizerFallbackWithoutFDv1FallbackTransitionsToOff(t *testing. require.NotNil(t, client) defer client.Close() - // The data system is off, and only the first synchronizer made a request. assert.Equal(t, initializationFailedErrorMessage, err.Error()) assert.Equal(t, - string(interfaces.DataSourceStateOff), + string(interfaces.DataSourceStateInterrupted), string(client.GetDataSourceStatusProvider().GetStatus().State)) - <-requestsCh - assertNoMoreRequests(t, requestsCh) assert.Contains(t, logCapture.GetOutput(ldlog.Warn), - "Synchronizer requested FDv1 fallback but none configured") + "FDv1 fallback requested but none configured; will try FDv2 again in 1s") + + // The first synchronizer makes a request, and makes another once the TTL expires. + testHelpers.RequireValue(t, requestsCh, time.Second*5) + testHelpers.RequireValue(t, requestsCh, time.Second*5) }) } @@ -908,18 +911,18 @@ func healthyStreamHandler() http.Handler { // fetchInitializer is a DataInitializer with a canned Fetch result. type fetchInitializer struct { name string - fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, bool, error) + fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) } func (i *fetchInitializer) Name() string { return i.name } -func (i *fetchInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, bool, error) { +func (i *fetchInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { return i.fetch(ds, ctx) } func initializerReturning( name string, - fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, bool, error), + fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error), ) subsystems.ComponentConfigurer[subsystems.DataInitializer] { return mocks.SingleComponentConfigurer[subsystems.DataInitializer]{ Instance: &fetchInitializer{name: name, fetch: fetch}, @@ -944,8 +947,8 @@ func malformedBasis(t *testing.T) *subsystems.Basis { // neither complete nor fail on that result; the synchronizer decides. func TestFDV2InitializerWithNoDataDefersToSynchronizer(t *testing.T) { noData := initializerReturning("no-data", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, bool, error) { - return noDataBasis(), false, nil + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return noDataBasis(), subsystems.FDv1Fallback{}, nil }) httphelpers.WithServer(healthyStreamHandler(), func(server *httptest.Server) { @@ -972,8 +975,8 @@ func TestFDV2InitializerWithNoDataDefersToSynchronizer(t *testing.T) { // An initializer whose data cannot be applied has not provided data. The synchronizer decides. func TestFDV2InitializerWithMalformedDataDefersToSynchronizer(t *testing.T) { malformed := initializerReturning("malformed", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, bool, error) { - return malformedBasis(t), false, nil + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return malformedBasis(t), subsystems.FDv1Fallback{}, nil }) httphelpers.WithServer(healthyStreamHandler(), func(server *httptest.Server) { @@ -1035,8 +1038,8 @@ func TestFDV2FallbackDirectiveWithDataCompletesInitialization(t *testing.T) { func TestFDV2FallbackDirectiveWithNoDataDefersToFDv1Synchronizer(t *testing.T) { dataV1 := ldservices.NewServerSDKData().Flags(alwaysFalseFlag) fallbackWithNoData := initializerReturning("fallback-no-data", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, bool, error) { - return noDataBasis(), true, nil + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return noDataBasis(), subsystems.FDv1Fallback{Requested: true}, nil }) pollV1Handler, pollV1Requests := httphelpers.RecordingHandler(ldservices.ServerSidePollingServiceHandler(dataV1)) @@ -1116,8 +1119,8 @@ func TestFDV2SelectorlessSynchronizerCompletesInitialization(t *testing.T) { // synchronizer. func TestFDV2FallbackDirectiveAfterEarlierInitializerDataCompletesInitialization(t *testing.T) { fallbackWithoutData := initializerReturning("fallback-without-data", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, bool, error) { - return nil, true, errors.New("server requested fallback") + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return nil, subsystems.FDv1Fallback{Requested: true}, errors.New("server requested fallback") }) testHelpers.WithTempFileData(fileDataWithOneFlag, func(filename string) { diff --git a/ldclient_override_vectors_test.go b/ldclient_override_vectors_test.go index a72f4055d..47d9c504e 100644 --- a/ldclient_override_vectors_test.go +++ b/ldclient_override_vectors_test.go @@ -72,8 +72,8 @@ type vectorInitializer struct { func (v *vectorInitializer) Name() string { return "VectorInitializer" } -func (v *vectorInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, bool, error) { - return &subsystems.Basis{ChangeSet: *v.changeSet, Persist: false}, false, nil +func (v *vectorInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return &subsystems.Basis{ChangeSet: *v.changeSet, Persist: false}, subsystems.FDv1Fallback{}, nil } func (v *vectorInitializer) Build(subsystems.ClientContext) (subsystems.DataInitializer, error) { diff --git a/ldclient_overrides_test.go b/ldclient_overrides_test.go index 0b8215927..f83c0ac20 100644 --- a/ldclient_overrides_test.go +++ b/ldclient_overrides_test.go @@ -42,8 +42,8 @@ func newHangingSynchronizer() *hangingSynchronizer { func (h *hangingSynchronizer) Name() string { return "HangingSynchronizer" } -func (h *hangingSynchronizer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, bool, error) { - return nil, false, errors.New("no data available") +func (h *hangingSynchronizer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return nil, subsystems.FDv1Fallback{}, errors.New("no data available") } func (h *hangingSynchronizer) Sync(ds subsystems.DataSelector) <-chan subsystems.DataSynchronizerResult { @@ -284,8 +284,8 @@ func (s *staticBasisInitializer) Name() string { return "StaticBasisInitializer" func (s *staticBasisInitializer) Fetch( ds subsystems.DataSelector, ctx context.Context, -) (*subsystems.Basis, bool, error) { - return &subsystems.Basis{ChangeSet: *s.changeSet, Persist: false}, false, nil +) (*subsystems.Basis, subsystems.FDv1Fallback, error) { + return &subsystems.Basis{ChangeSet: *s.changeSet, Persist: false}, subsystems.FDv1Fallback{}, nil } func (s *staticBasisInitializer) Build(subsystems.ClientContext) (subsystems.DataInitializer, error) { diff --git a/ldfiledatav2/file_data_source_impl.go b/ldfiledatav2/file_data_source_impl.go index 772ec8e9e..ac9ff817e 100644 --- a/ldfiledatav2/file_data_source_impl.go +++ b/ldfiledatav2/file_data_source_impl.go @@ -163,28 +163,31 @@ func (fs *fileDataSource) Sync(ds subsystems.DataSelector) <-chan subsystems.Dat return resultChan } -func (fs *fileDataSource) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, bool, error) { +func (fs *fileDataSource) Fetch( + ds subsystems.DataSelector, + ctx context.Context, +) (*subsystems.Basis, subsystems.FDv1Fallback, error) { docs := make([]filedata.Document, 0, len(fs.absFilePaths)) for _, path := range fs.absFilePaths { doc, err := filedata.ReadFile(path) if err != nil { - return nil, false, &filedata.ReadError{Err: err, Path: path} + return nil, subsystems.FDv1Fallback{}, &filedata.ReadError{Err: err, Path: path} } docs = append(docs, doc) } merged, err := filedata.Merge( filedata.DuplicateKeysHandling(fs.duplicateKeysHandling), filedata.MakeOffFlagWithValue, docs...) if err != nil { - return nil, false, err + return nil, subsystems.FDv1Fallback{}, err } changeSet, err := fs.makeChangeSet(merged) if err != nil { - return nil, false, err + return nil, subsystems.FDv1Fallback{}, err } return &subsystems.Basis{ ChangeSet: *changeSet, Persist: false, - }, false, nil + }, subsystems.FDv1Fallback{}, nil } func (fs *fileDataSource) applyData(merged filedata.MergeResult) { diff --git a/ldfiledatav2/file_data_source_test.go b/ldfiledatav2/file_data_source_test.go index e73550158..c451759ce 100644 --- a/ldfiledatav2/file_data_source_test.go +++ b/ldfiledatav2/file_data_source_test.go @@ -308,7 +308,7 @@ func TestSuccessfullyLoadsJsonFlagsAsInitializer(t *testing.T) { basis, fallback, err := initializer.Fetch(mocks.NewMockDataSelector(subsystems.NoSelector()), context.Background()) assert.NoError(t, err) - assert.False(t, fallback) + assert.False(t, fallback.Requested) assert.NotNil(t, basis) assert.Len(t, basis.ChangeSet.Changes(), 2) diff --git a/subsystems/data_source.go b/subsystems/data_source.go index e6bcc1028..90c168a62 100644 --- a/subsystems/data_source.go +++ b/subsystems/data_source.go @@ -3,6 +3,7 @@ package subsystems import ( "context" "io" + "time" "github.com/launchdarkly/go-sdk-common/v3/ldvalue" "github.com/launchdarkly/go-server-sdk/v7/interfaces" @@ -47,10 +48,20 @@ type DataInitializer interface { Name() string // Fetch returns a Basis, or an error if the Basis could not be retrieved. If the context has // expired, it returns the context's error. If the LaunchDarkly server has instructed the SDK - // to fall back to the FDv1 protocol (via the x-ld-fd-fallback response header), fallbackToFDv1 - // is true; in that case the Basis is nil and err may be nil or carry the underlying HTTP - // error for logging. Callers should branch on fallbackToFDv1 before err. - Fetch(ds DataSelector, ctx context.Context) (basis *Basis, fallbackToFDv1 bool, err error) + // to fall back to the FDv1 protocol, fdv1Fallback.Requested is true; in that case the Basis + // may still carry data from the same response, and err may be nil or carry the underlying + // HTTP error for logging. Callers should branch on fdv1Fallback before err. + Fetch(ds DataSelector, ctx context.Context) (basis *Basis, fdv1Fallback FDv1Fallback, err error) +} + +// FDv1Fallback is an instruction from LaunchDarkly to stop using the FDv2 protocol and fall back +// to FDv1 for a while. The zero value means that no fallback was requested. +type FDv1Fallback struct { + // Requested is true if LaunchDarkly asked the SDK to fall back to FDv1. + Requested bool + // TTL is how long LaunchDarkly asked the SDK to stay on FDv1 before trying FDv2 again. Zero + // means that LaunchDarkly did not supply a usable TTL, so the SDK uses its default. + TTL time.Duration } // DataSynchronizerResult represents the results of a Synchronizer's ongoing Sync method. @@ -59,7 +70,11 @@ type DataSynchronizerResult struct { State interfaces.DataSourceState Error interfaces.DataSourceErrorInfo FallbackToFDv1 bool - EnvironmentID ldvalue.OptionalString + // FDv1FallbackTTL is how long to stay on FDv1 before trying FDv2 again, when FallbackToFDv1 + // is true. Zero means that LaunchDarkly did not supply a usable TTL, so the SDK uses its + // default. + FDv1FallbackTTL time.Duration + EnvironmentID ldvalue.OptionalString } // DataSynchronizer represents a component capable of obtaining a Basis and subsequent delta updates asynchronously. diff --git a/subsystems/datasystem_configuration.go b/subsystems/datasystem_configuration.go index 793e4ac5f..b4f6c78cc 100644 --- a/subsystems/datasystem_configuration.go +++ b/subsystems/datasystem_configuration.go @@ -8,8 +8,8 @@ type SynchronizersConfiguration struct { SynchronizerBuilders []func() (DataSynchronizer, error) // FDv1FallbackBuilder is a special fallback used only when a synchronizer - // returns FallbackToFDv1=true. When activated, the system abandons the synchronizer list - // and switches to FDv1-only mode. + // returns FallbackToFDv1=true. When activated, the system pauses the synchronizer list + // and switches to FDv1-only mode until the fallback TTL expires. FDv1FallbackBuilder func() (DataSynchronizer, error) } diff --git a/subsystems/events.go b/subsystems/events.go index 9f3318f39..ed90c1b69 100644 --- a/subsystems/events.go +++ b/subsystems/events.go @@ -154,6 +154,9 @@ func (e Error) Name() EventName { // Goodbye represents a goodbye event. type Goodbye struct { Reason string `json:"reason"` + // ProtocolFallbackTTL, if present, is an instruction to fall back to FDv1 for this many + // seconds. It is kept as raw JSON so that a malformed value still counts as an instruction. + ProtocolFallbackTTL json.RawMessage `json:"protocolFallbackTTL,omitempty"` } //nolint:revive // Event method. diff --git a/testhelpers/ldtestdatav2/test_data_source.go b/testhelpers/ldtestdatav2/test_data_source.go index c22b56df3..cd4277c7a 100644 --- a/testhelpers/ldtestdatav2/test_data_source.go +++ b/testhelpers/ldtestdatav2/test_data_source.go @@ -316,9 +316,12 @@ func (d *testDataSourceImpl) Name() string { return "TestDataSynchronizer" } -func (d *testDataSourceImpl) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, bool, error) { +func (d *testDataSourceImpl) Fetch( + ds subsystems.DataSelector, + ctx context.Context, +) (*subsystems.Basis, subsystems.FDv1Fallback, error) { basis, err := d.owner.makeBasis() - return basis, false, err + return basis, subsystems.FDv1Fallback{}, err } func (d *testDataSourceImpl) Sync(ds subsystems.DataSelector) <-chan subsystems.DataSynchronizerResult { From 8ef78088f2a35df9091c515f9e7ed8a1a853cde9 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:09:26 +0000 Subject: [PATCH 2/5] fix: Return a nil *FDv1Fallback when no fallback is requested Co-Authored-By: Bee Klimt --- .../datasourcev2/fdv1_polling_data_source.go | 6 ++-- internal/datasourcev2/helpers.go | 24 +++++++++----- internal/datasourcev2/helpers_test.go | 30 ++++++++--------- internal/datasourcev2/polling_data_source.go | 8 ++--- .../datasourcev2/polling_data_source_test.go | 6 ++-- internal/datasourcev2/polling_http_request.go | 32 +++++++++---------- .../datasourcev2/streaming_data_source.go | 24 +++++++------- .../streaming_data_source_test.go | 2 +- internal/datasystem/fdv2_datasystem.go | 14 ++++---- internal/datasystem/fdv2_fallback_test.go | 4 +-- .../datasystem/fdv2_fdv1_recovery_test.go | 8 ++--- ldclient_end_to_end_fdv2_test.go | 22 ++++++------- ldclient_override_vectors_test.go | 4 +-- ldclient_overrides_test.go | 8 ++--- ldfiledatav2/file_data_source_impl.go | 10 +++--- ldfiledatav2/file_data_source_test.go | 2 +- subsystems/data_source.go | 12 +++---- testhelpers/ldtestdatav2/test_data_source.go | 4 +-- 18 files changed, 113 insertions(+), 107 deletions(-) diff --git a/internal/datasourcev2/fdv1_polling_data_source.go b/internal/datasourcev2/fdv1_polling_data_source.go index 95e066c05..54c28b14b 100644 --- a/internal/datasourcev2/fdv1_polling_data_source.go +++ b/internal/datasourcev2/fdv1_polling_data_source.go @@ -17,10 +17,10 @@ type fdv1ToFDv2Requester struct { func (r *fdv1ToFDv2Requester) Request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, subsystems.FDv1Fallback, error) { +) (*subsystems.ChangeSet, http.Header, *subsystems.FDv1Fallback, error) { data, cached, headers, err := r.requester.Request() if err != nil { - return nil, headers, subsystems.FDv1Fallback{}, err + return nil, headers, nil, err } code := subsystems.IntentTransferFull @@ -47,7 +47,7 @@ func (r *fdv1ToFDv2Requester) Request( subsystems.NewSelector("state", 1), data, ) - return changeset, headers, subsystems.FDv1Fallback{}, err + return changeset, headers, nil, err } func (r *fdv1ToFDv2Requester) BaseURI() string { diff --git a/internal/datasourcev2/helpers.go b/internal/datasourcev2/helpers.go index 6e35f6f95..affefd3e7 100644 --- a/internal/datasourcev2/helpers.go +++ b/internal/datasourcev2/helpers.go @@ -32,28 +32,36 @@ func isFDv1FallbackRequested(h http.Header) bool { const fdv1FallbackTTLHeader = "X-LD-FD-Fallback-TTL" // fdv1FallbackFromHeaders returns the FDv1 fallback instruction carried by the response headers. -func fdv1FallbackFromHeaders(h http.Header) subsystems.FDv1Fallback { +func fdv1FallbackFromHeaders(h http.Header) *subsystems.FDv1Fallback { if !isFDv1FallbackRequested(h) { - return subsystems.FDv1Fallback{} + return nil } seconds, err := strconv.ParseInt(h.Get(fdv1FallbackTTLHeader), 10, 64) if err != nil { - return subsystems.FDv1Fallback{Requested: true} + return &subsystems.FDv1Fallback{} } - return subsystems.FDv1Fallback{Requested: true, TTL: fdv1FallbackTTLFromSeconds(seconds)} + return &subsystems.FDv1Fallback{TTL: fdv1FallbackTTLFromSeconds(seconds)} } // fdv1FallbackFromGoodbye returns the FDv1 fallback instruction carried by a goodbye event. A // goodbye event is an instruction to fall back only if it has a protocolFallbackTTL. -func fdv1FallbackFromGoodbye(goodbye subsystems.Goodbye) subsystems.FDv1Fallback { +func fdv1FallbackFromGoodbye(goodbye subsystems.Goodbye) *subsystems.FDv1Fallback { if len(goodbye.ProtocolFallbackTTL) == 0 || string(goodbye.ProtocolFallbackTTL) == "null" { - return subsystems.FDv1Fallback{} + return nil } var seconds int64 if err := json.Unmarshal(goodbye.ProtocolFallbackTTL, &seconds); err != nil { - return subsystems.FDv1Fallback{Requested: true} + return &subsystems.FDv1Fallback{} } - return subsystems.FDv1Fallback{Requested: true, TTL: fdv1FallbackTTLFromSeconds(seconds)} + return &subsystems.FDv1Fallback{TTL: fdv1FallbackTTLFromSeconds(seconds)} +} + +// fdv1FallbackTTL returns the TTL of a fallback instruction, or zero if there is none. +func fdv1FallbackTTL(fallback *subsystems.FDv1Fallback) time.Duration { + if fallback == nil { + return 0 + } + return fallback.TTL } // fdv1FallbackTTLFromSeconds converts a TTL in seconds to a duration. A value that is too large to diff --git a/internal/datasourcev2/helpers_test.go b/internal/datasourcev2/helpers_test.go index 7c198140c..dc4d5edb2 100644 --- a/internal/datasourcev2/helpers_test.go +++ b/internal/datasourcev2/helpers_test.go @@ -63,15 +63,15 @@ func TestFDv1FallbackFromHeaders(t *testing.T) { name string fallback string ttl string - want subsystems.FDv1Fallback + want *subsystems.FDv1Fallback }{ - {name: "no fallback header", ttl: "120", want: subsystems.FDv1Fallback{}}, - {name: "no TTL header", fallback: "true", want: subsystems.FDv1Fallback{Requested: true}}, - {name: "valid TTL", fallback: "true", ttl: "120", want: subsystems.FDv1Fallback{Requested: true, TTL: 120 * time.Second}}, - {name: "malformed TTL", fallback: "true", ttl: "soon", want: subsystems.FDv1Fallback{Requested: true}}, - {name: "fractional TTL", fallback: "true", ttl: "1.5", want: subsystems.FDv1Fallback{Requested: true}}, - {name: "negative TTL", fallback: "true", ttl: "-5", want: subsystems.FDv1Fallback{Requested: true, TTL: -5 * time.Second}}, - {name: "huge TTL", fallback: "true", ttl: "9223372036854775807", want: subsystems.FDv1Fallback{Requested: true}}, + {name: "no fallback header", ttl: "120", want: nil}, + {name: "no TTL header", fallback: "true", want: &subsystems.FDv1Fallback{}}, + {name: "valid TTL", fallback: "true", ttl: "120", want: &subsystems.FDv1Fallback{TTL: 120 * time.Second}}, + {name: "malformed TTL", fallback: "true", ttl: "soon", want: &subsystems.FDv1Fallback{}}, + {name: "fractional TTL", fallback: "true", ttl: "1.5", want: &subsystems.FDv1Fallback{}}, + {name: "negative TTL", fallback: "true", ttl: "-5", want: &subsystems.FDv1Fallback{TTL: -5 * time.Second}}, + {name: "huge TTL", fallback: "true", ttl: "9223372036854775807", want: &subsystems.FDv1Fallback{}}, } { t.Run(tc.name, func(t *testing.T) { h := http.Header{} @@ -90,13 +90,13 @@ func TestFDv1FallbackFromGoodbye(t *testing.T) { for _, tc := range []struct { name string ttl string - want subsystems.FDv1Fallback + want *subsystems.FDv1Fallback }{ - {name: "no TTL", want: subsystems.FDv1Fallback{}}, - {name: "null TTL", ttl: "null", want: subsystems.FDv1Fallback{}}, - {name: "valid TTL", ttl: "120", want: subsystems.FDv1Fallback{Requested: true, TTL: 120 * time.Second}}, - {name: "zero TTL", ttl: "0", want: subsystems.FDv1Fallback{Requested: true}}, - {name: "malformed TTL", ttl: `"soon"`, want: subsystems.FDv1Fallback{Requested: true}}, + {name: "no TTL", want: nil}, + {name: "null TTL", ttl: "null", want: nil}, + {name: "valid TTL", ttl: "120", want: &subsystems.FDv1Fallback{TTL: 120 * time.Second}}, + {name: "zero TTL", ttl: "0", want: &subsystems.FDv1Fallback{}}, + {name: "malformed TTL", ttl: `"soon"`, want: &subsystems.FDv1Fallback{}}, } { t.Run(tc.name, func(t *testing.T) { goodbye := subsystems.Goodbye{Reason: "test", ProtocolFallbackTTL: json.RawMessage(tc.ttl)} @@ -109,5 +109,5 @@ func TestGoodbyeUnmarshalsProtocolFallbackTTL(t *testing.T) { var goodbye subsystems.Goodbye require.NoError(t, json.Unmarshal([]byte(`{"reason":"r","protocolFallbackTTL":120}`), &goodbye)) - assert.Equal(t, subsystems.FDv1Fallback{Requested: true, TTL: 120 * time.Second}, fdv1FallbackFromGoodbye(goodbye)) + assert.Equal(t, &subsystems.FDv1Fallback{TTL: 120 * time.Second}, fdv1FallbackFromGoodbye(goodbye)) } diff --git a/internal/datasourcev2/polling_data_source.go b/internal/datasourcev2/polling_data_source.go index 3e33458e3..83f2b340d 100644 --- a/internal/datasourcev2/polling_data_source.go +++ b/internal/datasourcev2/polling_data_source.go @@ -21,7 +21,7 @@ const ( // PollingRequester allows PollingProcessor to delegate fetching data to another component. // This is useful for testing the PollingProcessor without needing to set up a test HTTP server. type PollingRequester interface { - Request(context.Context, subsystems.Selector) (*subsystems.ChangeSet, http.Header, subsystems.FDv1Fallback, error) + Request(context.Context, subsystems.Selector) (*subsystems.ChangeSet, http.Header, *subsystems.FDv1Fallback, error) BaseURI() string FilterKey() string } @@ -71,7 +71,7 @@ func (pp *PollingProcessor) Name() string { func (pp *PollingProcessor) Fetch( ds subsystems.DataSelector, ctx context.Context, -) (*subsystems.Basis, subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { changeSet, headers, fallback, err := pp.requester.Request(ctx, ds.Selector()) if err != nil { return nil, fallback, err @@ -169,8 +169,8 @@ func (pp *PollingProcessor) poll( changeSet, headers, fallback, err := pp.requester.Request(ctx, ds.Selector()) result := subsystems.DataSynchronizerResult{ EnvironmentID: internal.NewInitMetadataFromHeaders(headers).GetEnvironmentID(), - FallbackToFDv1: fallback.Requested, - FDv1FallbackTTL: fallback.TTL, + FallbackToFDv1: fallback != nil, + FDv1FallbackTTL: fdv1FallbackTTL(fallback), } if err == nil { diff --git a/internal/datasourcev2/polling_data_source_test.go b/internal/datasourcev2/polling_data_source_test.go index e9db01823..20ef964cb 100644 --- a/internal/datasourcev2/polling_data_source_test.go +++ b/internal/datasourcev2/polling_data_source_test.go @@ -381,7 +381,7 @@ func TestPollingProcessorInitializerHandlesFallbackOnSuccessfulResponse(t *testi defer processor.Close() basis, fallback, err := processor.Fetch(ds, context.Background()) - assert.True(t, fallback.Requested) + assert.NotNil(t, fallback) assert.NoError(t, err) // Even when the server signals fallback, a valid payload in the same response // should still be surfaced so the caller can apply it before switching protocols. @@ -410,7 +410,7 @@ func TestPollingProcessorInitializerHandlesFallbackOnErrorResponse(t *testing.T) basis, fallback, err := processor.Fetch(ds, context.Background()) assert.Nil(t, basis) - assert.True(t, fallback.Requested) + assert.NotNil(t, fallback) assert.Error(t, err, "underlying HTTP error should still be surfaced alongside the fallback signal") }) } @@ -431,7 +431,7 @@ func TestPollingProcessorInitializerErrorWithoutFallbackHeaderReturnsRegularErro basis, fallback, err := processor.Fetch(ds, context.Background()) assert.Nil(t, basis) - assert.False(t, fallback.Requested) + assert.Nil(t, fallback) assert.Error(t, err) }) } diff --git a/internal/datasourcev2/polling_http_request.go b/internal/datasourcev2/polling_http_request.go index 144c295b1..4f6cf0324 100644 --- a/internal/datasourcev2/polling_http_request.go +++ b/internal/datasourcev2/polling_http_request.go @@ -86,10 +86,10 @@ func (r *pollingRequester) FilterKey() string { func (r *pollingRequester) Request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, subsystems.FDv1Fallback, error) { +) (*subsystems.ChangeSet, http.Header, *subsystems.FDv1Fallback, error) { changeSet, headers, goodbyeFallback, err := r.request(ctx, selector) fallback := fdv1FallbackFromHeaders(headers) - if goodbyeFallback.Requested { + if goodbyeFallback != nil { fallback = goodbyeFallback } return changeSet, headers, fallback, err @@ -98,22 +98,22 @@ func (r *pollingRequester) Request( func (r *pollingRequester) request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, subsystems.FDv1Fallback, error) { +) (*subsystems.ChangeSet, http.Header, *subsystems.FDv1Fallback, error) { if r.loggers.IsDebugEnabled() { r.loggers.Debug("Polling LaunchDarkly for feature flag updates") } body, cached, headers, err := r.makeRequest(ctx, endpoints.PollingRequestV2Path, selector) if err != nil { - return nil, headers, subsystems.FDv1Fallback{}, err + return nil, headers, nil, err } if cached { - return subsystems.NewChangeSetBuilder().NoChanges(), headers, subsystems.FDv1Fallback{}, nil + return subsystems.NewChangeSetBuilder().NoChanges(), headers, nil, nil } var payload subsystems.PollingPayload if err = json.Unmarshal(body, &payload); err != nil { - return nil, headers, subsystems.FDv1Fallback{}, malformedJSONError{err} + return nil, headers, nil, malformedJSONError{err} } changeSet := subsystems.NewChangeSetBuilder().WithDeserializeOptions(r.deserializeOptions) @@ -121,52 +121,52 @@ func (r *pollingRequester) request( for _, event := range payload.Events { select { case <-ctx.Done(): - return nil, headers, subsystems.FDv1Fallback{}, ctx.Err() + return nil, headers, nil, ctx.Err() default: switch event.Name { case subsystems.EventServerIntent: var serverIntent subsystems.ServerIntent err := json.Unmarshal(event.Data, &serverIntent) if err != nil { - return nil, headers, subsystems.FDv1Fallback{}, err + return nil, headers, nil, err } if serverIntent.Payload.Code == subsystems.IntentNone { - return changeSet.NoChanges(), headers, subsystems.FDv1Fallback{}, nil + return changeSet.NoChanges(), headers, nil, nil } changeSet.Start(serverIntent) case subsystems.EventPutObject: var put subsystems.PutObject if err := json.Unmarshal(event.Data, &put); err != nil { - return nil, headers, subsystems.FDv1Fallback{}, err + return nil, headers, nil, err } changeSet.AddPut(put.Kind, put.Key, put.Version, put.Object) case subsystems.EventDeleteObject: var deleteObject subsystems.DeleteObject if err := json.Unmarshal(event.Data, &deleteObject); err != nil { - return nil, headers, subsystems.FDv1Fallback{}, err + return nil, headers, nil, err } changeSet.AddDelete(deleteObject.Kind, deleteObject.Key, deleteObject.Version) case subsystems.EventGoodbye: var goodbye subsystems.Goodbye if err := json.Unmarshal(event.Data, &goodbye); err != nil { - return nil, headers, subsystems.FDv1Fallback{}, err + return nil, headers, nil, err } - if fallback := fdv1FallbackFromGoodbye(goodbye); fallback.Requested { + if fallback := fdv1FallbackFromGoodbye(goodbye); fallback != nil { return nil, headers, fallback, goodbyeError{reason: goodbye.Reason} } case subsystems.EventPayloadTransferred: var selector subsystems.Selector if err := json.Unmarshal(event.Data, &selector); err != nil { - return nil, headers, subsystems.FDv1Fallback{}, err + return nil, headers, nil, err } changeset, err := changeSet.Finish(selector) - return changeset, headers, subsystems.FDv1Fallback{}, err + return changeset, headers, nil, err } } } err = fmt.Errorf("didn't receive any known protocol events in polling payload") - return nil, headers, subsystems.FDv1Fallback{}, err + return nil, headers, nil, err } func (r *pollingRequester) makeRequest( diff --git a/internal/datasourcev2/streaming_data_source.go b/internal/datasourcev2/streaming_data_source.go index bc72b5a38..367f3a1bd 100644 --- a/internal/datasourcev2/streaming_data_source.go +++ b/internal/datasourcev2/streaming_data_source.go @@ -112,8 +112,8 @@ func (sp *StreamProcessor) Name() string { func (sp *StreamProcessor) Fetch( ds subsystems.DataSelector, _ context.Context, -) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return nil, subsystems.FDv1Fallback{}, errors.New("StreamProcessor does not implement Fetch capability") +) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return nil, nil, errors.New("StreamProcessor does not implement Fetch capability") } //nolint:revive // DataSynchronizer method. @@ -180,7 +180,7 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su // fallback is set when the server's response headers carry x-ld-fd-fallback: true. We // finish applying the current payload before emitting the fallback signal, so evaluations // can serve the server-provided data while FDv1 takes over. - fallback := subsystems.FDv1Fallback{} + var fallback *subsystems.FDv1Fallback for { select { @@ -203,7 +203,7 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su if eventWithHeaders, ok := event.(es.EventWithHeaders); ok { headers := eventWithHeaders.Headers() environmentID = internal.NewInitMetadataFromHeaders(headers).GetEnvironmentID() - if headerFallback := fdv1FallbackFromHeaders(headers); headerFallback.Requested { + if headerFallback := fdv1FallbackFromHeaders(headers); headerFallback != nil { fallback = headerFallback } } @@ -240,8 +240,8 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su resultChan <- subsystems.DataSynchronizerResult{ State: interfaces.DataSourceStateValid, EnvironmentID: environmentID, - FallbackToFDv1: fallback.Requested, - FDv1FallbackTTL: fallback.TTL, + FallbackToFDv1: fallback != nil, + FDv1FallbackTTL: fdv1FallbackTTL(fallback), } payloadApplied = true break @@ -275,7 +275,7 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su // A goodbye with a fallback TTL tells the SDK to fall back to FDv1. Any // payload in progress is incomplete, so it is discarded. - if goodbyeFallback := fdv1FallbackFromGoodbye(goodbye); goodbyeFallback.Requested { + if goodbyeFallback := fdv1FallbackFromGoodbye(goodbye); goodbyeFallback != nil { resultChan <- subsystems.DataSynchronizerResult{ State: interfaces.DataSourceStateInterrupted, EnvironmentID: environmentID, @@ -322,8 +322,8 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su ChangeSet: changeSet, State: interfaces.DataSourceStateValid, EnvironmentID: environmentID, - FallbackToFDv1: fallback.Requested, - FDv1FallbackTTL: fallback.TTL, + FallbackToFDv1: fallback != nil, + FDv1FallbackTTL: fdv1FallbackTTL(fallback), } payloadApplied = true @@ -339,7 +339,7 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su // result emitted above carries FallbackToFDv1=true; close the stream so we stop // consuming. Events that don't complete a payload leave payloadApplied false so we // keep consuming (fallback persists across iterations). - if fallback.Requested && payloadApplied { + if fallback != nil && payloadApplied { stream.Close() return } @@ -402,12 +402,12 @@ func (sp *StreamProcessor) subscribe(ds subsystems.DataSelector, resultChan chan Time: time.Now(), } - if fallback := fdv1FallbackFromHeaders(se.Header); fallback.Requested { + if fallback := fdv1FallbackFromHeaders(se.Header); fallback != nil { resultChan <- subsystems.DataSynchronizerResult{ State: interfaces.DataSourceStateOff, Error: errorInfo, FallbackToFDv1: true, - FDv1FallbackTTL: fallback.TTL, + FDv1FallbackTTL: fdv1FallbackTTL(fallback), EnvironmentID: environmentID, } return es.StreamErrorHandlerResult{CloseNow: true} diff --git a/internal/datasourcev2/streaming_data_source_test.go b/internal/datasourcev2/streaming_data_source_test.go index e65b3f5ce..2a8dd3d73 100644 --- a/internal/datasourcev2/streaming_data_source_test.go +++ b/internal/datasourcev2/streaming_data_source_test.go @@ -62,7 +62,7 @@ func TestStreamingDoesNotWorkAsInitializer(t *testing.T) { defer sp.Close() basis, fallback, err := sp.Fetch(ds, context.Background()) assert.Nil(t, basis) - assert.False(t, fallback.Requested) + assert.Nil(t, fallback) assert.NotNil(t, err) } diff --git a/internal/datasystem/fdv2_datasystem.go b/internal/datasystem/fdv2_datasystem.go index d54e99f5e..90cbb4846 100644 --- a/internal/datasystem/fdv2_datasystem.go +++ b/internal/datasystem/fdv2_datasystem.go @@ -237,7 +237,7 @@ func (f *FDv2) run(ctx context.Context, closeWhenReady chan struct{}) { f.UpdateStatus(interfaces.DataSourceStateInitializing, interfaces.DataSourceErrorInfo{}) var recovery *time.Timer - if fallback, errorInfo := f.runInitializers(ctx, closeWhenReady); fallback.Requested { + if fallback, errorInfo := f.runInitializers(ctx, closeWhenReady); fallback != nil { recovery = f.fallBackToFDv1(fallback.TTL, errorInfo) } @@ -325,7 +325,7 @@ func (f *FDv2) runPersistentStoreOutageRecovery(ctx context.Context, statuses <- // applied to the store, even if that data had no selector. A basis that carries no data, or data // that cannot be applied, does not count; the synchronizers then decide readiness. // -// fallback.Requested is true when an initializer asked the SDK to switch to FDv1. If the fallback is +// fallback is not nil when an initializer asked the SDK to switch to FDv1. If the fallback is // signalled alongside a basis, that basis is applied before returning, so evaluations can serve // the server-provided data while the FDv1 synchronizer spins up. If any initializer's data was // applied by then, initialization is complete before returning. @@ -334,16 +334,16 @@ func (f *FDv2) runPersistentStoreOutageRecovery(ctx context.Context, statuses <- // configured. It is empty when the fallback accompanied a successful response. func (f *FDv2) runInitializers( ctx context.Context, closeWhenReady chan struct{}, -) (fallback subsystems.FDv1Fallback, errorInfo interfaces.DataSourceErrorInfo) { +) (fallback *subsystems.FDv1Fallback, errorInfo interfaces.DataSourceErrorInfo) { // Name of the last initializer whose data was applied to the store. appliedFrom := "" for _, initializer := range f.initializers { f.loggers.Infof("Attempting to initialize via %s", initializer.Name()) basis, fallback, err := initializer.Fetch(f.store, ctx) if errors.Is(err, context.Canceled) { - return subsystems.FDv1Fallback{}, interfaces.DataSourceErrorInfo{} + return nil, interfaces.DataSourceErrorInfo{} } - if fallback.Requested { + if fallback != nil { if err != nil { f.loggers.Warnf("Initializer %s requested fallback to FDv1 protocol: %v", initializer.Name(), err) errorInfo = interfaces.DataSourceErrorInfo{ @@ -374,14 +374,14 @@ func (f *FDv2) runInitializers( if basis.ChangeSet.Selector().IsDefined() { f.loggers.Infof("Initialized via %s", initializer.Name()) f.completeInitialization(closeWhenReady) - return subsystems.FDv1Fallback{}, interfaces.DataSourceErrorInfo{} + return nil, interfaces.DataSourceErrorInfo{} } } if appliedFrom != "" { f.loggers.Infof("Initialized via %s; the data has no selector", appliedFrom) f.completeInitialization(closeWhenReady) } - return subsystems.FDv1Fallback{}, interfaces.DataSourceErrorInfo{} + return nil, interfaces.DataSourceErrorInfo{} } // applyBasis applies an initializer's basis to the store. It reports whether the store received diff --git a/internal/datasystem/fdv2_fallback_test.go b/internal/datasystem/fdv2_fallback_test.go index a7c5ac798..61ed7a229 100644 --- a/internal/datasystem/fdv2_fallback_test.go +++ b/internal/datasystem/fdv2_fallback_test.go @@ -39,8 +39,8 @@ func (f *fakeSynchronizer) Name() string { return f.name } func (f *fakeSynchronizer) Fetch( _ subsystems.DataSelector, _ context.Context, -) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return nil, subsystems.FDv1Fallback{}, nil +) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return nil, nil, nil } func (f *fakeSynchronizer) Sync(_ subsystems.DataSelector) <-chan subsystems.DataSynchronizerResult { diff --git a/internal/datasystem/fdv2_fdv1_recovery_test.go b/internal/datasystem/fdv2_fdv1_recovery_test.go index 61229b92c..b2d64b2bf 100644 --- a/internal/datasystem/fdv2_fdv1_recovery_test.go +++ b/internal/datasystem/fdv2_fdv1_recovery_test.go @@ -33,8 +33,8 @@ func (s *lifecycleSynchronizer) Name() string { return s.name } func (s *lifecycleSynchronizer) Fetch( _ subsystems.DataSelector, _ context.Context, -) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return nil, subsystems.FDv1Fallback{}, nil +) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return nil, nil, nil } func (s *lifecycleSynchronizer) Sync(_ subsystems.DataSelector) <-chan subsystems.DataSynchronizerResult { @@ -77,8 +77,8 @@ func (i fallbackInitializer) Name() string { return "fallback-initializer" } func (i fallbackInitializer) Fetch( _ subsystems.DataSelector, _ context.Context, -) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return nil, subsystems.FDv1Fallback{Requested: true, TTL: i.ttl}, nil +) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return nil, &subsystems.FDv1Fallback{TTL: i.ttl}, nil } func makeRecoveryTestFDv2( diff --git a/ldclient_end_to_end_fdv2_test.go b/ldclient_end_to_end_fdv2_test.go index 586a79a49..3ec11898e 100644 --- a/ldclient_end_to_end_fdv2_test.go +++ b/ldclient_end_to_end_fdv2_test.go @@ -911,18 +911,18 @@ func healthyStreamHandler() http.Handler { // fetchInitializer is a DataInitializer with a canned Fetch result. type fetchInitializer struct { name string - fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) + fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) } func (i *fetchInitializer) Name() string { return i.name } -func (i *fetchInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { +func (i *fetchInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { return i.fetch(ds, ctx) } func initializerReturning( name string, - fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error), + fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error), ) subsystems.ComponentConfigurer[subsystems.DataInitializer] { return mocks.SingleComponentConfigurer[subsystems.DataInitializer]{ Instance: &fetchInitializer{name: name, fetch: fetch}, @@ -947,8 +947,8 @@ func malformedBasis(t *testing.T) *subsystems.Basis { // neither complete nor fail on that result; the synchronizer decides. func TestFDV2InitializerWithNoDataDefersToSynchronizer(t *testing.T) { noData := initializerReturning("no-data", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return noDataBasis(), subsystems.FDv1Fallback{}, nil + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return noDataBasis(), nil, nil }) httphelpers.WithServer(healthyStreamHandler(), func(server *httptest.Server) { @@ -975,8 +975,8 @@ func TestFDV2InitializerWithNoDataDefersToSynchronizer(t *testing.T) { // An initializer whose data cannot be applied has not provided data. The synchronizer decides. func TestFDV2InitializerWithMalformedDataDefersToSynchronizer(t *testing.T) { malformed := initializerReturning("malformed", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return malformedBasis(t), subsystems.FDv1Fallback{}, nil + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return malformedBasis(t), nil, nil }) httphelpers.WithServer(healthyStreamHandler(), func(server *httptest.Server) { @@ -1038,8 +1038,8 @@ func TestFDV2FallbackDirectiveWithDataCompletesInitialization(t *testing.T) { func TestFDV2FallbackDirectiveWithNoDataDefersToFDv1Synchronizer(t *testing.T) { dataV1 := ldservices.NewServerSDKData().Flags(alwaysFalseFlag) fallbackWithNoData := initializerReturning("fallback-no-data", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return noDataBasis(), subsystems.FDv1Fallback{Requested: true}, nil + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return noDataBasis(), &subsystems.FDv1Fallback{}, nil }) pollV1Handler, pollV1Requests := httphelpers.RecordingHandler(ldservices.ServerSidePollingServiceHandler(dataV1)) @@ -1119,8 +1119,8 @@ func TestFDV2SelectorlessSynchronizerCompletesInitialization(t *testing.T) { // synchronizer. func TestFDV2FallbackDirectiveAfterEarlierInitializerDataCompletesInitialization(t *testing.T) { fallbackWithoutData := initializerReturning("fallback-without-data", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return nil, subsystems.FDv1Fallback{Requested: true}, errors.New("server requested fallback") + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return nil, &subsystems.FDv1Fallback{}, errors.New("server requested fallback") }) testHelpers.WithTempFileData(fileDataWithOneFlag, func(filename string) { diff --git a/ldclient_override_vectors_test.go b/ldclient_override_vectors_test.go index 47d9c504e..9a322a4f4 100644 --- a/ldclient_override_vectors_test.go +++ b/ldclient_override_vectors_test.go @@ -72,8 +72,8 @@ type vectorInitializer struct { func (v *vectorInitializer) Name() string { return "VectorInitializer" } -func (v *vectorInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return &subsystems.Basis{ChangeSet: *v.changeSet, Persist: false}, subsystems.FDv1Fallback{}, nil +func (v *vectorInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return &subsystems.Basis{ChangeSet: *v.changeSet, Persist: false}, nil, nil } func (v *vectorInitializer) Build(subsystems.ClientContext) (subsystems.DataInitializer, error) { diff --git a/ldclient_overrides_test.go b/ldclient_overrides_test.go index f83c0ac20..62988f620 100644 --- a/ldclient_overrides_test.go +++ b/ldclient_overrides_test.go @@ -42,8 +42,8 @@ func newHangingSynchronizer() *hangingSynchronizer { func (h *hangingSynchronizer) Name() string { return "HangingSynchronizer" } -func (h *hangingSynchronizer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return nil, subsystems.FDv1Fallback{}, errors.New("no data available") +func (h *hangingSynchronizer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return nil, nil, errors.New("no data available") } func (h *hangingSynchronizer) Sync(ds subsystems.DataSelector) <-chan subsystems.DataSynchronizerResult { @@ -284,8 +284,8 @@ func (s *staticBasisInitializer) Name() string { return "StaticBasisInitializer" func (s *staticBasisInitializer) Fetch( ds subsystems.DataSelector, ctx context.Context, -) (*subsystems.Basis, subsystems.FDv1Fallback, error) { - return &subsystems.Basis{ChangeSet: *s.changeSet, Persist: false}, subsystems.FDv1Fallback{}, nil +) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + return &subsystems.Basis{ChangeSet: *s.changeSet, Persist: false}, nil, nil } func (s *staticBasisInitializer) Build(subsystems.ClientContext) (subsystems.DataInitializer, error) { diff --git a/ldfiledatav2/file_data_source_impl.go b/ldfiledatav2/file_data_source_impl.go index ac9ff817e..b6dfc88c0 100644 --- a/ldfiledatav2/file_data_source_impl.go +++ b/ldfiledatav2/file_data_source_impl.go @@ -166,28 +166,28 @@ func (fs *fileDataSource) Sync(ds subsystems.DataSelector) <-chan subsystems.Dat func (fs *fileDataSource) Fetch( ds subsystems.DataSelector, ctx context.Context, -) (*subsystems.Basis, subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { docs := make([]filedata.Document, 0, len(fs.absFilePaths)) for _, path := range fs.absFilePaths { doc, err := filedata.ReadFile(path) if err != nil { - return nil, subsystems.FDv1Fallback{}, &filedata.ReadError{Err: err, Path: path} + return nil, nil, &filedata.ReadError{Err: err, Path: path} } docs = append(docs, doc) } merged, err := filedata.Merge( filedata.DuplicateKeysHandling(fs.duplicateKeysHandling), filedata.MakeOffFlagWithValue, docs...) if err != nil { - return nil, subsystems.FDv1Fallback{}, err + return nil, nil, err } changeSet, err := fs.makeChangeSet(merged) if err != nil { - return nil, subsystems.FDv1Fallback{}, err + return nil, nil, err } return &subsystems.Basis{ ChangeSet: *changeSet, Persist: false, - }, subsystems.FDv1Fallback{}, nil + }, nil, nil } func (fs *fileDataSource) applyData(merged filedata.MergeResult) { diff --git a/ldfiledatav2/file_data_source_test.go b/ldfiledatav2/file_data_source_test.go index c451759ce..0f3b2472e 100644 --- a/ldfiledatav2/file_data_source_test.go +++ b/ldfiledatav2/file_data_source_test.go @@ -308,7 +308,7 @@ func TestSuccessfullyLoadsJsonFlagsAsInitializer(t *testing.T) { basis, fallback, err := initializer.Fetch(mocks.NewMockDataSelector(subsystems.NoSelector()), context.Background()) assert.NoError(t, err) - assert.False(t, fallback.Requested) + assert.Nil(t, fallback) assert.NotNil(t, basis) assert.Len(t, basis.ChangeSet.Changes(), 2) diff --git a/subsystems/data_source.go b/subsystems/data_source.go index 90c168a62..459e58a70 100644 --- a/subsystems/data_source.go +++ b/subsystems/data_source.go @@ -48,17 +48,15 @@ type DataInitializer interface { Name() string // Fetch returns a Basis, or an error if the Basis could not be retrieved. If the context has // expired, it returns the context's error. If the LaunchDarkly server has instructed the SDK - // to fall back to the FDv1 protocol, fdv1Fallback.Requested is true; in that case the Basis - // may still carry data from the same response, and err may be nil or carry the underlying - // HTTP error for logging. Callers should branch on fdv1Fallback before err. - Fetch(ds DataSelector, ctx context.Context) (basis *Basis, fdv1Fallback FDv1Fallback, err error) + // to fall back to the FDv1 protocol, fdv1Fallback is not nil; in that case the Basis may + // still carry data from the same response, and err may be nil or carry the underlying HTTP + // error for logging. Callers should branch on fdv1Fallback before err. + Fetch(ds DataSelector, ctx context.Context) (basis *Basis, fdv1Fallback *FDv1Fallback, err error) } // FDv1Fallback is an instruction from LaunchDarkly to stop using the FDv2 protocol and fall back -// to FDv1 for a while. The zero value means that no fallback was requested. +// to FDv1 for a while. type FDv1Fallback struct { - // Requested is true if LaunchDarkly asked the SDK to fall back to FDv1. - Requested bool // TTL is how long LaunchDarkly asked the SDK to stay on FDv1 before trying FDv2 again. Zero // means that LaunchDarkly did not supply a usable TTL, so the SDK uses its default. TTL time.Duration diff --git a/testhelpers/ldtestdatav2/test_data_source.go b/testhelpers/ldtestdatav2/test_data_source.go index cd4277c7a..c4867c207 100644 --- a/testhelpers/ldtestdatav2/test_data_source.go +++ b/testhelpers/ldtestdatav2/test_data_source.go @@ -319,9 +319,9 @@ func (d *testDataSourceImpl) Name() string { func (d *testDataSourceImpl) Fetch( ds subsystems.DataSelector, ctx context.Context, -) (*subsystems.Basis, subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { basis, err := d.owner.makeBasis() - return basis, subsystems.FDv1Fallback{}, err + return basis, nil, err } func (d *testDataSourceImpl) Sync(ds subsystems.DataSelector) <-chan subsystems.DataSynchronizerResult { From 0ffa24f7f57854975a415346b1ae1fb4df317223 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:26:18 +0000 Subject: [PATCH 3/5] fix: Represent the FDv1 fallback TTL as a nil-able *time.Duration Co-Authored-By: Bee Klimt --- .../datasourcev2/fdv1_polling_data_source.go | 3 +- internal/datasourcev2/helpers.go | 20 +++++------- internal/datasourcev2/helpers_test.go | 24 +++++++------- internal/datasourcev2/polling_data_source.go | 13 ++++---- .../datasourcev2/polling_data_source_test.go | 14 ++++---- internal/datasourcev2/polling_http_request.go | 5 +-- .../datasourcev2/streaming_data_source.go | 18 ++++------- .../streaming_data_source_test.go | 13 ++++---- internal/datasystem/fdv2_datasystem.go | 14 ++++---- internal/datasystem/fdv2_fallback_test.go | 2 +- .../datasystem/fdv2_fdv1_recovery_test.go | 16 ++++++---- ldclient_end_to_end_fdv2_test.go | 20 ++++++------ ldclient_override_vectors_test.go | 2 +- ldclient_overrides_test.go | 4 +-- ldfiledatav2/file_data_source_impl.go | 3 +- subsystems/data_source.go | 32 ++++++++----------- subsystems/datasystem_configuration.go | 2 +- testhelpers/ldtestdatav2/test_data_source.go | 6 ++-- 18 files changed, 100 insertions(+), 111 deletions(-) diff --git a/internal/datasourcev2/fdv1_polling_data_source.go b/internal/datasourcev2/fdv1_polling_data_source.go index 54c28b14b..b145837a4 100644 --- a/internal/datasourcev2/fdv1_polling_data_source.go +++ b/internal/datasourcev2/fdv1_polling_data_source.go @@ -3,6 +3,7 @@ package datasourcev2 import ( "context" "net/http" + "time" "github.com/launchdarkly/go-sdk-common/v3/ldlog" "github.com/launchdarkly/go-server-sdk/v7/internal/datasource" @@ -17,7 +18,7 @@ type fdv1ToFDv2Requester struct { func (r *fdv1ToFDv2Requester) Request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, *subsystems.FDv1Fallback, error) { +) (*subsystems.ChangeSet, http.Header, *time.Duration, error) { data, cached, headers, err := r.requester.Request() if err != nil { return nil, headers, nil, err diff --git a/internal/datasourcev2/helpers.go b/internal/datasourcev2/helpers.go index affefd3e7..2f3667df2 100644 --- a/internal/datasourcev2/helpers.go +++ b/internal/datasourcev2/helpers.go @@ -32,36 +32,32 @@ func isFDv1FallbackRequested(h http.Header) bool { const fdv1FallbackTTLHeader = "X-LD-FD-Fallback-TTL" // fdv1FallbackFromHeaders returns the FDv1 fallback instruction carried by the response headers. -func fdv1FallbackFromHeaders(h http.Header) *subsystems.FDv1Fallback { +func fdv1FallbackFromHeaders(h http.Header) *time.Duration { if !isFDv1FallbackRequested(h) { return nil } seconds, err := strconv.ParseInt(h.Get(fdv1FallbackTTLHeader), 10, 64) if err != nil { - return &subsystems.FDv1Fallback{} + return new(time.Duration) } - return &subsystems.FDv1Fallback{TTL: fdv1FallbackTTLFromSeconds(seconds)} + return ttlPointer(fdv1FallbackTTLFromSeconds(seconds)) } // fdv1FallbackFromGoodbye returns the FDv1 fallback instruction carried by a goodbye event. A // goodbye event is an instruction to fall back only if it has a protocolFallbackTTL. -func fdv1FallbackFromGoodbye(goodbye subsystems.Goodbye) *subsystems.FDv1Fallback { +func fdv1FallbackFromGoodbye(goodbye subsystems.Goodbye) *time.Duration { if len(goodbye.ProtocolFallbackTTL) == 0 || string(goodbye.ProtocolFallbackTTL) == "null" { return nil } var seconds int64 if err := json.Unmarshal(goodbye.ProtocolFallbackTTL, &seconds); err != nil { - return &subsystems.FDv1Fallback{} + return new(time.Duration) } - return &subsystems.FDv1Fallback{TTL: fdv1FallbackTTLFromSeconds(seconds)} + return ttlPointer(fdv1FallbackTTLFromSeconds(seconds)) } -// fdv1FallbackTTL returns the TTL of a fallback instruction, or zero if there is none. -func fdv1FallbackTTL(fallback *subsystems.FDv1Fallback) time.Duration { - if fallback == nil { - return 0 - } - return fallback.TTL +func ttlPointer(ttl time.Duration) *time.Duration { + return &ttl } // fdv1FallbackTTLFromSeconds converts a TTL in seconds to a duration. A value that is too large to diff --git a/internal/datasourcev2/helpers_test.go b/internal/datasourcev2/helpers_test.go index dc4d5edb2..e6eae5a03 100644 --- a/internal/datasourcev2/helpers_test.go +++ b/internal/datasourcev2/helpers_test.go @@ -63,15 +63,15 @@ func TestFDv1FallbackFromHeaders(t *testing.T) { name string fallback string ttl string - want *subsystems.FDv1Fallback + want *time.Duration }{ {name: "no fallback header", ttl: "120", want: nil}, - {name: "no TTL header", fallback: "true", want: &subsystems.FDv1Fallback{}}, - {name: "valid TTL", fallback: "true", ttl: "120", want: &subsystems.FDv1Fallback{TTL: 120 * time.Second}}, - {name: "malformed TTL", fallback: "true", ttl: "soon", want: &subsystems.FDv1Fallback{}}, - {name: "fractional TTL", fallback: "true", ttl: "1.5", want: &subsystems.FDv1Fallback{}}, - {name: "negative TTL", fallback: "true", ttl: "-5", want: &subsystems.FDv1Fallback{TTL: -5 * time.Second}}, - {name: "huge TTL", fallback: "true", ttl: "9223372036854775807", want: &subsystems.FDv1Fallback{}}, + {name: "no TTL header", fallback: "true", want: new(time.Duration)}, + {name: "valid TTL", fallback: "true", ttl: "120", want: ttlPointer(120 * time.Second)}, + {name: "malformed TTL", fallback: "true", ttl: "soon", want: new(time.Duration)}, + {name: "fractional TTL", fallback: "true", ttl: "1.5", want: new(time.Duration)}, + {name: "negative TTL", fallback: "true", ttl: "-5", want: ttlPointer(-5 * time.Second)}, + {name: "huge TTL", fallback: "true", ttl: "9223372036854775807", want: new(time.Duration)}, } { t.Run(tc.name, func(t *testing.T) { h := http.Header{} @@ -90,13 +90,13 @@ func TestFDv1FallbackFromGoodbye(t *testing.T) { for _, tc := range []struct { name string ttl string - want *subsystems.FDv1Fallback + want *time.Duration }{ {name: "no TTL", want: nil}, {name: "null TTL", ttl: "null", want: nil}, - {name: "valid TTL", ttl: "120", want: &subsystems.FDv1Fallback{TTL: 120 * time.Second}}, - {name: "zero TTL", ttl: "0", want: &subsystems.FDv1Fallback{}}, - {name: "malformed TTL", ttl: `"soon"`, want: &subsystems.FDv1Fallback{}}, + {name: "valid TTL", ttl: "120", want: ttlPointer(120 * time.Second)}, + {name: "zero TTL", ttl: "0", want: new(time.Duration)}, + {name: "malformed TTL", ttl: `"soon"`, want: new(time.Duration)}, } { t.Run(tc.name, func(t *testing.T) { goodbye := subsystems.Goodbye{Reason: "test", ProtocolFallbackTTL: json.RawMessage(tc.ttl)} @@ -109,5 +109,5 @@ func TestGoodbyeUnmarshalsProtocolFallbackTTL(t *testing.T) { var goodbye subsystems.Goodbye require.NoError(t, json.Unmarshal([]byte(`{"reason":"r","protocolFallbackTTL":120}`), &goodbye)) - assert.Equal(t, &subsystems.FDv1Fallback{TTL: 120 * time.Second}, fdv1FallbackFromGoodbye(goodbye)) + assert.Equal(t, ttlPointer(120*time.Second), fdv1FallbackFromGoodbye(goodbye)) } diff --git a/internal/datasourcev2/polling_data_source.go b/internal/datasourcev2/polling_data_source.go index 83f2b340d..2c5365f01 100644 --- a/internal/datasourcev2/polling_data_source.go +++ b/internal/datasourcev2/polling_data_source.go @@ -21,7 +21,7 @@ const ( // PollingRequester allows PollingProcessor to delegate fetching data to another component. // This is useful for testing the PollingProcessor without needing to set up a test HTTP server. type PollingRequester interface { - Request(context.Context, subsystems.Selector) (*subsystems.ChangeSet, http.Header, *subsystems.FDv1Fallback, error) + Request(context.Context, subsystems.Selector) (*subsystems.ChangeSet, http.Header, *time.Duration, error) BaseURI() string FilterKey() string } @@ -71,7 +71,7 @@ func (pp *PollingProcessor) Name() string { func (pp *PollingProcessor) Fetch( ds subsystems.DataSelector, ctx context.Context, -) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *time.Duration, error) { changeSet, headers, fallback, err := pp.requester.Request(ctx, ds.Selector()) if err != nil { return nil, fallback, err @@ -112,8 +112,8 @@ func (pp *PollingProcessor) Sync(ds subsystems.DataSelector) <-chan subsystems.D result, err := pp.poll(ctx, ds) // When the server requested FDv1 fallback, dispatch the result as-is -- poll has - // already populated State (Valid on success, Off on error) and FallbackToFDv1=true. - if result.FallbackToFDv1 { + // already populated State (Valid on success, Off on error) and FDv1FallbackTTL. + if result.FDv1FallbackTTL != nil { resultChan <- result return } @@ -152,7 +152,7 @@ func (pp *PollingProcessor) Sync(ds subsystems.DataSelector) <-chan subsystems.D } // poll performs a single polling request and builds a DataSynchronizerResult describing the -// outcome. The result's FallbackToFDv1 flag and FDv1FallbackTTL are always populated from the +// outcome. The result's FDv1FallbackTTL is always populated from the // fallback instruction in the response, whether or not the request succeeded -- a 500 or a // malformed-JSON body can still carry the fallback signal. // @@ -169,8 +169,7 @@ func (pp *PollingProcessor) poll( changeSet, headers, fallback, err := pp.requester.Request(ctx, ds.Selector()) result := subsystems.DataSynchronizerResult{ EnvironmentID: internal.NewInitMetadataFromHeaders(headers).GetEnvironmentID(), - FallbackToFDv1: fallback != nil, - FDv1FallbackTTL: fdv1FallbackTTL(fallback), + FDv1FallbackTTL: fallback, } if err == nil { diff --git a/internal/datasourcev2/polling_data_source_test.go b/internal/datasourcev2/polling_data_source_test.go index 20ef964cb..8d3bfa730 100644 --- a/internal/datasourcev2/polling_data_source_test.go +++ b/internal/datasourcev2/polling_data_source_test.go @@ -260,13 +260,13 @@ func TestPollingProcessorSynchronizerHandlesFallbackOnSuccessfulResponse(t *test resultChan := processor.Sync(ds) - // A single Valid result carries both the payload and the FallbackToFDv1 signal -- the + // A single Valid result carries both the payload and the FDv1FallbackTTL signal -- the // consumer applies the ChangeSet first, then switches to the FDv1 synchronizer. result := <-resultChan assert.Equal(t, interfaces.DataSourceStateValid, result.State) require.NotNil(t, result.ChangeSet) assert.Len(t, result.ChangeSet.Changes(), 1) - assert.True(t, result.FallbackToFDv1) + require.NotNil(t, *result.FDv1FallbackTTL) }) } @@ -294,7 +294,7 @@ func TestPollingProcessorSynchronizerHandlesFallbackOnMalformedBody(t *testing.T assert.Equal(t, interfaces.DataSourceStateOff, result.State) assert.Equal(t, interfaces.DataSourceErrorKindInvalidData, result.Error.Kind) - assert.True(t, result.FallbackToFDv1) + require.NotNil(t, *result.FDv1FallbackTTL) }) } @@ -321,8 +321,8 @@ func TestPollingProcessorSynchronizerHandlesFallbackToFDv2(t *testing.T) { assert.Equal(t, result.State, interfaces.DataSourceStateOff) assert.Equal(t, result.Error.Kind, interfaces.DataSourceErrorKindErrorResponse) - assert.True(t, result.FallbackToFDv1) - assert.Equal(t, 120*time.Second, result.FDv1FallbackTTL) + require.NotNil(t, *result.FDv1FallbackTTL) + assert.Equal(t, 120*time.Second, *result.FDv1FallbackTTL) }) } @@ -354,8 +354,8 @@ func TestPollingProcessorSynchronizerFallsBackOnGoodbyeWithFallbackTTL(t *testin assert.Equal(t, interfaces.DataSourceStateOff, result.State) assert.Equal(t, interfaces.DataSourceErrorKindUnknown, result.Error.Kind) - assert.True(t, result.FallbackToFDv1) - assert.Equal(t, 120*time.Second, result.FDv1FallbackTTL) + require.NotNil(t, *result.FDv1FallbackTTL) + assert.Equal(t, 120*time.Second, *result.FDv1FallbackTTL) }) } diff --git a/internal/datasourcev2/polling_http_request.go b/internal/datasourcev2/polling_http_request.go index 4f6cf0324..18c94d99b 100644 --- a/internal/datasourcev2/polling_http_request.go +++ b/internal/datasourcev2/polling_http_request.go @@ -8,6 +8,7 @@ import ( "maps" "net/http" "net/url" + "time" "github.com/gregjones/httpcache" @@ -86,7 +87,7 @@ func (r *pollingRequester) FilterKey() string { func (r *pollingRequester) Request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, *subsystems.FDv1Fallback, error) { +) (*subsystems.ChangeSet, http.Header, *time.Duration, error) { changeSet, headers, goodbyeFallback, err := r.request(ctx, selector) fallback := fdv1FallbackFromHeaders(headers) if goodbyeFallback != nil { @@ -98,7 +99,7 @@ func (r *pollingRequester) Request( func (r *pollingRequester) request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, *subsystems.FDv1Fallback, error) { +) (*subsystems.ChangeSet, http.Header, *time.Duration, error) { if r.loggers.IsDebugEnabled() { r.loggers.Debug("Polling LaunchDarkly for feature flag updates") } diff --git a/internal/datasourcev2/streaming_data_source.go b/internal/datasourcev2/streaming_data_source.go index 367f3a1bd..d74fd401e 100644 --- a/internal/datasourcev2/streaming_data_source.go +++ b/internal/datasourcev2/streaming_data_source.go @@ -112,7 +112,7 @@ func (sp *StreamProcessor) Name() string { func (sp *StreamProcessor) Fetch( ds subsystems.DataSelector, _ context.Context, -) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *time.Duration, error) { return nil, nil, errors.New("StreamProcessor does not implement Fetch capability") } @@ -180,7 +180,7 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su // fallback is set when the server's response headers carry x-ld-fd-fallback: true. We // finish applying the current payload before emitting the fallback signal, so evaluations // can serve the server-provided data while FDv1 takes over. - var fallback *subsystems.FDv1Fallback + var fallback *time.Duration for { select { @@ -240,8 +240,7 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su resultChan <- subsystems.DataSynchronizerResult{ State: interfaces.DataSourceStateValid, EnvironmentID: environmentID, - FallbackToFDv1: fallback != nil, - FDv1FallbackTTL: fdv1FallbackTTL(fallback), + FDv1FallbackTTL: fallback, } payloadApplied = true break @@ -279,8 +278,7 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su resultChan <- subsystems.DataSynchronizerResult{ State: interfaces.DataSourceStateInterrupted, EnvironmentID: environmentID, - FallbackToFDv1: true, - FDv1FallbackTTL: goodbyeFallback.TTL, + FDv1FallbackTTL: goodbyeFallback, } stream.Close() return @@ -322,8 +320,7 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su ChangeSet: changeSet, State: interfaces.DataSourceStateValid, EnvironmentID: environmentID, - FallbackToFDv1: fallback != nil, - FDv1FallbackTTL: fdv1FallbackTTL(fallback), + FDv1FallbackTTL: fallback, } payloadApplied = true @@ -336,7 +333,7 @@ func (sp *StreamProcessor) consumeStream(stream *es.Stream, resultChan chan<- su } // Once a payload has been applied with a pending FDv1 fallback signal, the Valid - // result emitted above carries FallbackToFDv1=true; close the stream so we stop + // result emitted above carries FDv1FallbackTTL; close the stream so we stop // consuming. Events that don't complete a payload leave payloadApplied false so we // keep consuming (fallback persists across iterations). if fallback != nil && payloadApplied { @@ -406,8 +403,7 @@ func (sp *StreamProcessor) subscribe(ds subsystems.DataSelector, resultChan chan resultChan <- subsystems.DataSynchronizerResult{ State: interfaces.DataSourceStateOff, Error: errorInfo, - FallbackToFDv1: true, - FDv1FallbackTTL: fdv1FallbackTTL(fallback), + FDv1FallbackTTL: fallback, EnvironmentID: environmentID, } return es.StreamErrorHandlerResult{CloseNow: true} diff --git a/internal/datasourcev2/streaming_data_source_test.go b/internal/datasourcev2/streaming_data_source_test.go index 2a8dd3d73..6e48b4d06 100644 --- a/internal/datasourcev2/streaming_data_source_test.go +++ b/internal/datasourcev2/streaming_data_source_test.go @@ -27,6 +27,7 @@ import ( th "github.com/launchdarkly/go-test-helpers/v3" "github.com/launchdarkly/go-test-helpers/v3/httphelpers" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -183,8 +184,8 @@ func TestStreamingProcessorHandlesFallbackToFDv1(t *testing.T) { assert.Equal(t, result.State, interfaces.DataSourceStateOff) assert.Equal(t, result.Error.Kind, interfaces.DataSourceErrorKindErrorResponse) - assert.True(t, result.FallbackToFDv1) - assert.Equal(t, 120*time.Second, result.FDv1FallbackTTL) + require.NotNil(t, *result.FDv1FallbackTTL) + assert.Equal(t, 120*time.Second, *result.FDv1FallbackTTL) }) } @@ -206,8 +207,8 @@ func TestStreamingProcessorFallsBackOnGoodbyeWithFallbackTTL(t *testing.T) { assert.Equal(t, interfaces.DataSourceStateInterrupted, result.State) assert.Nil(t, result.ChangeSet) - assert.True(t, result.FallbackToFDv1) - assert.Equal(t, 120*time.Second, result.FDv1FallbackTTL) + require.NotNil(t, *result.FDv1FallbackTTL) + assert.Equal(t, 120*time.Second, *result.FDv1FallbackTTL) }) } @@ -240,13 +241,13 @@ func TestStreamingProcessorHandlesFallbackOnSuccessfulResponse(t *testing.T) { defer sp.Close() resultChan := sp.Sync(ds) - // A single Valid result carries both the payload and the FallbackToFDv1 signal -- the + // A single Valid result carries both the payload and the FDv1FallbackTTL signal -- the // consumer applies the ChangeSet first, then switches to the FDv1 synchronizer. result := <-resultChan assert.Equal(t, interfaces.DataSourceStateValid, result.State) assert.NotNil(t, result.ChangeSet) assert.Len(t, result.ChangeSet.Changes(), 1) - assert.True(t, result.FallbackToFDv1) + require.NotNil(t, *result.FDv1FallbackTTL) }) } diff --git a/internal/datasystem/fdv2_datasystem.go b/internal/datasystem/fdv2_datasystem.go index 90cbb4846..2f0d8394b 100644 --- a/internal/datasystem/fdv2_datasystem.go +++ b/internal/datasystem/fdv2_datasystem.go @@ -238,7 +238,7 @@ func (f *FDv2) run(ctx context.Context, closeWhenReady chan struct{}) { var recovery *time.Timer if fallback, errorInfo := f.runInitializers(ctx, closeWhenReady); fallback != nil { - recovery = f.fallBackToFDv1(fallback.TTL, errorInfo) + recovery = f.fallBackToFDv1(*fallback, errorInfo) } if f.configuredWithDataSources && f.dataStoreStatusProvider.IsStatusMonitoringEnabled() { @@ -334,7 +334,7 @@ func (f *FDv2) runPersistentStoreOutageRecovery(ctx context.Context, statuses <- // configured. It is empty when the fallback accompanied a successful response. func (f *FDv2) runInitializers( ctx context.Context, closeWhenReady chan struct{}, -) (fallback *subsystems.FDv1Fallback, errorInfo interfaces.DataSourceErrorInfo) { +) (fallback *time.Duration, errorInfo interfaces.DataSourceErrorInfo) { // Name of the last initializer whose data was applied to the store. appliedFrom := "" for _, initializer := range f.initializers { @@ -573,18 +573,18 @@ func (f *FDv2) consumeSynchronizerResults( f.UpdateStatus(result.State, result.Error) case interfaces.DataSourceStateOff: f.UpdateStatus(interfaces.DataSourceStateInterrupted, result.Error) - if result.FallbackToFDv1 { - return syncFDv1, result.FDv1FallbackTTL, nil + if result.FDv1FallbackTTL != nil { + return syncFDv1, *result.FDv1FallbackTTL, nil } return syncBlock, 0, nil } - // FallbackToFDv1 may ride along on a Valid or Interrupted result too -- e.g. a + // FDv1FallbackTTL may ride along on a Valid or Interrupted result too -- e.g. a // successful response whose headers also requested the fallback. The Valid/ // Interrupted branches above already applied any ChangeSet and updated status; // now hand control to the FDv1 fallback synchronizer. - if result.FallbackToFDv1 { - return syncFDv1, result.FDv1FallbackTTL, nil + if result.FDv1FallbackTTL != nil { + return syncFDv1, *result.FDv1FallbackTTL, nil } case <-ticker.C: // If there's only one available synchronizer, don't check conditions diff --git a/internal/datasystem/fdv2_fallback_test.go b/internal/datasystem/fdv2_fallback_test.go index 61ed7a229..d5833aabe 100644 --- a/internal/datasystem/fdv2_fallback_test.go +++ b/internal/datasystem/fdv2_fallback_test.go @@ -39,7 +39,7 @@ func (f *fakeSynchronizer) Name() string { return f.name } func (f *fakeSynchronizer) Fetch( _ subsystems.DataSelector, _ context.Context, -) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *time.Duration, error) { return nil, nil, nil } diff --git a/internal/datasystem/fdv2_fdv1_recovery_test.go b/internal/datasystem/fdv2_fdv1_recovery_test.go index b2d64b2bf..08c80341d 100644 --- a/internal/datasystem/fdv2_fdv1_recovery_test.go +++ b/internal/datasystem/fdv2_fdv1_recovery_test.go @@ -20,6 +20,10 @@ import ( // A TTL that is valid, so that it is used as-is, and short, so that recovery is quick. const shortFallbackTTL = 10 * time.Millisecond +func ttlPointer(ttl time.Duration) *time.Duration { + return &ttl +} + // lifecycleSynchronizer is a DataSynchronizer that reports when it starts and closes. Each // build of a lifecycleBuilder creates a new one. type lifecycleSynchronizer struct { @@ -33,7 +37,7 @@ func (s *lifecycleSynchronizer) Name() string { return s.name } func (s *lifecycleSynchronizer) Fetch( _ subsystems.DataSelector, _ context.Context, -) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *time.Duration, error) { return nil, nil, nil } @@ -77,8 +81,8 @@ func (i fallbackInitializer) Name() string { return "fallback-initializer" } func (i fallbackInitializer) Fetch( _ subsystems.DataSelector, _ context.Context, -) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { - return nil, &subsystems.FDv1Fallback{TTL: i.ttl}, nil +) (*subsystems.Basis, *time.Duration, error) { + return nil, ttlPointer(i.ttl), nil } func makeRecoveryTestFDv2( @@ -145,8 +149,7 @@ func TestFDv2RecoversAfterSynchronizerFallbackWithoutFDv1Synchronizer(t *testing first := th.RequireValue(t, fdv2.built, time.Second) first.results <- subsystems.DataSynchronizerResult{ State: interfaces.DataSourceStateOff, - FallbackToFDv1: true, - FDv1FallbackTTL: shortFallbackTTL, + FDv1FallbackTTL: ttlPointer(shortFallbackTTL), } // The FDv2 synchronizer is closed and, after the TTL, started again. @@ -165,8 +168,7 @@ func TestFDv2RecoversAfterSynchronizerFallbackToFDv1Synchronizer(t *testing.T) { first := th.RequireValue(t, fdv2.built, time.Second) first.results <- subsystems.DataSynchronizerResult{ State: interfaces.DataSourceStateValid, - FallbackToFDv1: true, - FDv1FallbackTTL: shortFallbackTTL, + FDv1FallbackTTL: ttlPointer(shortFallbackTTL), } // Each synchronizer is closed before the next one starts, so FDv1 and FDv2 never write to diff --git a/ldclient_end_to_end_fdv2_test.go b/ldclient_end_to_end_fdv2_test.go index 3ec11898e..a2bd1cd7d 100644 --- a/ldclient_end_to_end_fdv2_test.go +++ b/ldclient_end_to_end_fdv2_test.go @@ -259,7 +259,7 @@ func TestFDV2CanFallBackToV1FromStreamingSuccess(t *testing.T) { reached := client.GetDataSourceStatusProvider().WaitFor(interfaces.DataSourceStateValid, time.Second*5) require.True(t, reached, "timed out waiting for data source to reach VALID state") - // Status becomes Valid as soon as the FDv2 stream applies its payload (with FallbackToFDv1 + // Status becomes Valid as soon as the FDv2 stream applies its payload (with FDv1FallbackTTL // riding along on the same result), which happens before FDv1 has fetched its own data. // Poll until the flag value reflects FDv1 data to verify the handoff completed. assert.Eventually(t, func() bool { @@ -911,18 +911,18 @@ func healthyStreamHandler() http.Handler { // fetchInitializer is a DataInitializer with a canned Fetch result. type fetchInitializer struct { name string - fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) + fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *time.Duration, error) } func (i *fetchInitializer) Name() string { return i.name } -func (i *fetchInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +func (i *fetchInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *time.Duration, error) { return i.fetch(ds, ctx) } func initializerReturning( name string, - fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error), + fetch func(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *time.Duration, error), ) subsystems.ComponentConfigurer[subsystems.DataInitializer] { return mocks.SingleComponentConfigurer[subsystems.DataInitializer]{ Instance: &fetchInitializer{name: name, fetch: fetch}, @@ -947,7 +947,7 @@ func malformedBasis(t *testing.T) *subsystems.Basis { // neither complete nor fail on that result; the synchronizer decides. func TestFDV2InitializerWithNoDataDefersToSynchronizer(t *testing.T) { noData := initializerReturning("no-data", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *time.Duration, error) { return noDataBasis(), nil, nil }) @@ -975,7 +975,7 @@ func TestFDV2InitializerWithNoDataDefersToSynchronizer(t *testing.T) { // An initializer whose data cannot be applied has not provided data. The synchronizer decides. func TestFDV2InitializerWithMalformedDataDefersToSynchronizer(t *testing.T) { malformed := initializerReturning("malformed", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *time.Duration, error) { return malformedBasis(t), nil, nil }) @@ -1038,8 +1038,8 @@ func TestFDV2FallbackDirectiveWithDataCompletesInitialization(t *testing.T) { func TestFDV2FallbackDirectiveWithNoDataDefersToFDv1Synchronizer(t *testing.T) { dataV1 := ldservices.NewServerSDKData().Flags(alwaysFalseFlag) fallbackWithNoData := initializerReturning("fallback-no-data", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { - return noDataBasis(), &subsystems.FDv1Fallback{}, nil + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *time.Duration, error) { + return noDataBasis(), new(time.Duration), nil }) pollV1Handler, pollV1Requests := httphelpers.RecordingHandler(ldservices.ServerSidePollingServiceHandler(dataV1)) @@ -1119,8 +1119,8 @@ func TestFDV2SelectorlessSynchronizerCompletesInitialization(t *testing.T) { // synchronizer. func TestFDV2FallbackDirectiveAfterEarlierInitializerDataCompletesInitialization(t *testing.T) { fallbackWithoutData := initializerReturning("fallback-without-data", - func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { - return nil, &subsystems.FDv1Fallback{}, errors.New("server requested fallback") + func(subsystems.DataSelector, context.Context) (*subsystems.Basis, *time.Duration, error) { + return nil, new(time.Duration), errors.New("server requested fallback") }) testHelpers.WithTempFileData(fileDataWithOneFlag, func(filename string) { diff --git a/ldclient_override_vectors_test.go b/ldclient_override_vectors_test.go index 9a322a4f4..8c8d76bf0 100644 --- a/ldclient_override_vectors_test.go +++ b/ldclient_override_vectors_test.go @@ -72,7 +72,7 @@ type vectorInitializer struct { func (v *vectorInitializer) Name() string { return "VectorInitializer" } -func (v *vectorInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +func (v *vectorInitializer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *time.Duration, error) { return &subsystems.Basis{ChangeSet: *v.changeSet, Persist: false}, nil, nil } diff --git a/ldclient_overrides_test.go b/ldclient_overrides_test.go index 62988f620..8412af159 100644 --- a/ldclient_overrides_test.go +++ b/ldclient_overrides_test.go @@ -42,7 +42,7 @@ func newHangingSynchronizer() *hangingSynchronizer { func (h *hangingSynchronizer) Name() string { return "HangingSynchronizer" } -func (h *hangingSynchronizer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +func (h *hangingSynchronizer) Fetch(ds subsystems.DataSelector, ctx context.Context) (*subsystems.Basis, *time.Duration, error) { return nil, nil, errors.New("no data available") } @@ -284,7 +284,7 @@ func (s *staticBasisInitializer) Name() string { return "StaticBasisInitializer" func (s *staticBasisInitializer) Fetch( ds subsystems.DataSelector, ctx context.Context, -) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *time.Duration, error) { return &subsystems.Basis{ChangeSet: *s.changeSet, Persist: false}, nil, nil } diff --git a/ldfiledatav2/file_data_source_impl.go b/ldfiledatav2/file_data_source_impl.go index b6dfc88c0..53e0bb9e1 100644 --- a/ldfiledatav2/file_data_source_impl.go +++ b/ldfiledatav2/file_data_source_impl.go @@ -166,7 +166,7 @@ func (fs *fileDataSource) Sync(ds subsystems.DataSelector) <-chan subsystems.Dat func (fs *fileDataSource) Fetch( ds subsystems.DataSelector, ctx context.Context, -) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *time.Duration, error) { docs := make([]filedata.Document, 0, len(fs.absFilePaths)) for _, path := range fs.absFilePaths { doc, err := filedata.ReadFile(path) @@ -213,7 +213,6 @@ func (fs *fileDataSource) handleError(err error) { Message: err.Error(), Time: time.Time{}, }, - FallbackToFDv1: false, }) } diff --git a/subsystems/data_source.go b/subsystems/data_source.go index 459e58a70..3465213cc 100644 --- a/subsystems/data_source.go +++ b/subsystems/data_source.go @@ -48,30 +48,24 @@ type DataInitializer interface { Name() string // Fetch returns a Basis, or an error if the Basis could not be retrieved. If the context has // expired, it returns the context's error. If the LaunchDarkly server has instructed the SDK - // to fall back to the FDv1 protocol, fdv1Fallback is not nil; in that case the Basis may + // to fall back to the FDv1 protocol, fdv1FallbackTTL is not nil; in that case the Basis may // still carry data from the same response, and err may be nil or carry the underlying HTTP - // error for logging. Callers should branch on fdv1Fallback before err. - Fetch(ds DataSelector, ctx context.Context) (basis *Basis, fdv1Fallback *FDv1Fallback, err error) -} - -// FDv1Fallback is an instruction from LaunchDarkly to stop using the FDv2 protocol and fall back -// to FDv1 for a while. -type FDv1Fallback struct { - // TTL is how long LaunchDarkly asked the SDK to stay on FDv1 before trying FDv2 again. Zero - // means that LaunchDarkly did not supply a usable TTL, so the SDK uses its default. - TTL time.Duration + // error for logging. Callers should branch on fdv1FallbackTTL before err. + // + // fdv1FallbackTTL is how long LaunchDarkly asked the SDK to stay on FDv1 before trying FDv2 + // again. Zero means that LaunchDarkly did not supply a usable TTL, so the SDK uses its default. + Fetch(ds DataSelector, ctx context.Context) (basis *Basis, fdv1FallbackTTL *time.Duration, err error) } // DataSynchronizerResult represents the results of a Synchronizer's ongoing Sync method. type DataSynchronizerResult struct { - ChangeSet *ChangeSet - State interfaces.DataSourceState - Error interfaces.DataSourceErrorInfo - FallbackToFDv1 bool - // FDv1FallbackTTL is how long to stay on FDv1 before trying FDv2 again, when FallbackToFDv1 - // is true. Zero means that LaunchDarkly did not supply a usable TTL, so the SDK uses its - // default. - FDv1FallbackTTL time.Duration + ChangeSet *ChangeSet + State interfaces.DataSourceState + Error interfaces.DataSourceErrorInfo + // FDv1FallbackTTL is not nil if LaunchDarkly instructed the SDK to fall back to the FDv1 + // protocol. It is how long to stay on FDv1 before trying FDv2 again. Zero means that + // LaunchDarkly did not supply a usable TTL, so the SDK uses its default. + FDv1FallbackTTL *time.Duration EnvironmentID ldvalue.OptionalString } diff --git a/subsystems/datasystem_configuration.go b/subsystems/datasystem_configuration.go index b4f6c78cc..b0835ea55 100644 --- a/subsystems/datasystem_configuration.go +++ b/subsystems/datasystem_configuration.go @@ -8,7 +8,7 @@ type SynchronizersConfiguration struct { SynchronizerBuilders []func() (DataSynchronizer, error) // FDv1FallbackBuilder is a special fallback used only when a synchronizer - // returns FallbackToFDv1=true. When activated, the system pauses the synchronizer list + // returns a non-nil FDv1FallbackTTL. When activated, the system pauses the synchronizer list // and switches to FDv1-only mode until the fallback TTL expires. FDv1FallbackBuilder func() (DataSynchronizer, error) } diff --git a/testhelpers/ldtestdatav2/test_data_source.go b/testhelpers/ldtestdatav2/test_data_source.go index c4867c207..7f235f434 100644 --- a/testhelpers/ldtestdatav2/test_data_source.go +++ b/testhelpers/ldtestdatav2/test_data_source.go @@ -5,6 +5,7 @@ import ( "encoding/json" "strconv" "sync" + "time" "github.com/launchdarkly/go-server-sdk-evaluation/v3/ldmodel" "github.com/launchdarkly/go-server-sdk/v7/interfaces" @@ -319,7 +320,7 @@ func (d *testDataSourceImpl) Name() string { func (d *testDataSourceImpl) Fetch( ds subsystems.DataSelector, ctx context.Context, -) (*subsystems.Basis, *subsystems.FDv1Fallback, error) { +) (*subsystems.Basis, *time.Duration, error) { basis, err := d.owner.makeBasis() return basis, nil, err } @@ -331,8 +332,7 @@ func (d *testDataSourceImpl) Sync(ds subsystems.DataSelector) <-chan subsystems. statusChan := d.owner.statusBroadcaster.AddListener() result := subsystems.DataSynchronizerResult{ - State: interfaces.DataSourceStateInitializing, - FallbackToFDv1: false, + State: interfaces.DataSourceStateInitializing, } go func() { From 0227d1bf186aeac0460571ec4cbc9864e56002c6 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 05:05:05 +0000 Subject: [PATCH 4/5] refactor: Name the results of PollingRequester.Request Co-Authored-By: Bee Klimt --- internal/datasourcev2/fdv1_polling_data_source.go | 2 +- internal/datasourcev2/polling_data_source.go | 9 ++++++++- internal/datasourcev2/polling_http_request.go | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/datasourcev2/fdv1_polling_data_source.go b/internal/datasourcev2/fdv1_polling_data_source.go index b145837a4..7fbcae739 100644 --- a/internal/datasourcev2/fdv1_polling_data_source.go +++ b/internal/datasourcev2/fdv1_polling_data_source.go @@ -18,7 +18,7 @@ type fdv1ToFDv2Requester struct { func (r *fdv1ToFDv2Requester) Request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, *time.Duration, error) { +) (changeSet *subsystems.ChangeSet, headers http.Header, fdv1FallbackTTL *time.Duration, err error) { data, cached, headers, err := r.requester.Request() if err != nil { return nil, headers, nil, err diff --git a/internal/datasourcev2/polling_data_source.go b/internal/datasourcev2/polling_data_source.go index 2c5365f01..1b2f1afe8 100644 --- a/internal/datasourcev2/polling_data_source.go +++ b/internal/datasourcev2/polling_data_source.go @@ -21,7 +21,14 @@ const ( // PollingRequester allows PollingProcessor to delegate fetching data to another component. // This is useful for testing the PollingProcessor without needing to set up a test HTTP server. type PollingRequester interface { - Request(context.Context, subsystems.Selector) (*subsystems.ChangeSet, http.Header, *time.Duration, error) + // Request fetches a change set. If LaunchDarkly instructed the SDK to fall back to FDv1, + // fdv1FallbackTTL is not nil; zero means that no usable TTL was supplied. + Request(ctx context.Context, selector subsystems.Selector) ( + changeSet *subsystems.ChangeSet, + headers http.Header, + fdv1FallbackTTL *time.Duration, + err error, + ) BaseURI() string FilterKey() string } diff --git a/internal/datasourcev2/polling_http_request.go b/internal/datasourcev2/polling_http_request.go index 18c94d99b..102891fd7 100644 --- a/internal/datasourcev2/polling_http_request.go +++ b/internal/datasourcev2/polling_http_request.go @@ -87,7 +87,7 @@ func (r *pollingRequester) FilterKey() string { func (r *pollingRequester) Request( ctx context.Context, selector subsystems.Selector, -) (*subsystems.ChangeSet, http.Header, *time.Duration, error) { +) (changeSet *subsystems.ChangeSet, headers http.Header, fdv1FallbackTTL *time.Duration, err error) { changeSet, headers, goodbyeFallback, err := r.request(ctx, selector) fallback := fdv1FallbackFromHeaders(headers) if goodbyeFallback != nil { From 49a07eb5b139847952682b40f3995086784dfc54 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 05:15:12 +0000 Subject: [PATCH 5/5] fix: Log polling goodbyes without a fallback TTL and keep reading the payload Co-Authored-By: Bee Klimt --- .../datasourcev2/polling_data_source_test.go | 8 +++---- internal/datasourcev2/polling_http_request.go | 2 ++ .../datasourcev2/polling_http_request_test.go | 22 +++++++++++++++++++ .../streaming_data_source_test.go | 6 ++--- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/internal/datasourcev2/polling_data_source_test.go b/internal/datasourcev2/polling_data_source_test.go index 8d3bfa730..10a4c2655 100644 --- a/internal/datasourcev2/polling_data_source_test.go +++ b/internal/datasourcev2/polling_data_source_test.go @@ -266,7 +266,7 @@ func TestPollingProcessorSynchronizerHandlesFallbackOnSuccessfulResponse(t *test assert.Equal(t, interfaces.DataSourceStateValid, result.State) require.NotNil(t, result.ChangeSet) assert.Len(t, result.ChangeSet.Changes(), 1) - require.NotNil(t, *result.FDv1FallbackTTL) + require.NotNil(t, result.FDv1FallbackTTL) }) } @@ -294,7 +294,7 @@ func TestPollingProcessorSynchronizerHandlesFallbackOnMalformedBody(t *testing.T assert.Equal(t, interfaces.DataSourceStateOff, result.State) assert.Equal(t, interfaces.DataSourceErrorKindInvalidData, result.Error.Kind) - require.NotNil(t, *result.FDv1FallbackTTL) + require.NotNil(t, result.FDv1FallbackTTL) }) } @@ -321,7 +321,7 @@ func TestPollingProcessorSynchronizerHandlesFallbackToFDv2(t *testing.T) { assert.Equal(t, result.State, interfaces.DataSourceStateOff) assert.Equal(t, result.Error.Kind, interfaces.DataSourceErrorKindErrorResponse) - require.NotNil(t, *result.FDv1FallbackTTL) + require.NotNil(t, result.FDv1FallbackTTL) assert.Equal(t, 120*time.Second, *result.FDv1FallbackTTL) }) } @@ -354,7 +354,7 @@ func TestPollingProcessorSynchronizerFallsBackOnGoodbyeWithFallbackTTL(t *testin assert.Equal(t, interfaces.DataSourceStateOff, result.State) assert.Equal(t, interfaces.DataSourceErrorKindUnknown, result.Error.Kind) - require.NotNil(t, *result.FDv1FallbackTTL) + require.NotNil(t, result.FDv1FallbackTTL) assert.Equal(t, 120*time.Second, *result.FDv1FallbackTTL) }) } diff --git a/internal/datasourcev2/polling_http_request.go b/internal/datasourcev2/polling_http_request.go index 102891fd7..f8199112c 100644 --- a/internal/datasourcev2/polling_http_request.go +++ b/internal/datasourcev2/polling_http_request.go @@ -152,6 +152,8 @@ func (r *pollingRequester) request( if err := json.Unmarshal(event.Data, &goodbye); err != nil { return nil, headers, nil, err } + r.loggers.Infof("Polling payload contained goodbye: %s", goodbye.Reason) + // A goodbye without a fallback TTL is informational, so keep reading the payload. if fallback := fdv1FallbackFromGoodbye(goodbye); fallback != nil { return nil, headers, fallback, goodbyeError{reason: goodbye.Reason} } diff --git a/internal/datasourcev2/polling_http_request_test.go b/internal/datasourcev2/polling_http_request_test.go index 1714de803..aa7fe3476 100644 --- a/internal/datasourcev2/polling_http_request_test.go +++ b/internal/datasourcev2/polling_http_request_test.go @@ -2,6 +2,7 @@ package datasourcev2 import ( "context" + "encoding/json" "net/http" "net/http/httptest" "testing" @@ -46,6 +47,27 @@ func TestV2RequestorImplRequestAll(t *testing.T) { }) }) + t.Run("goodbye without fallback TTL", func(t *testing.T) { + flag := ldbuilders.NewFlagBuilder("flagkey").Version(1).Build() + payload := ldservicesv2.NewServerSDKData().Flags(flag).ToInitializerPayload(subsystems.NewSelector("test-state", 1)) + goodbye := subsystems.RawEvent{ + Name: subsystems.EventGoodbye, + Data: json.RawMessage(`{"reason":"just saying"}`), + } + payload.Events = append([]subsystems.RawEvent{payload.Events[0], goodbye}, payload.Events[1:]...) + + httphelpers.WithServer(ldservices.ServerSidePollingV2ServiceHandler(payload), func(ts *httptest.Server) { + r := newPollingRequester(sharedtest.BasicClientContext(), nil, ts.URL, filter.key) + + changeSet, _, fallback, err := r.Request(context.Background(), subsystems.NoSelector()) + + require.NoError(t, err) + assert.Nil(t, fallback) + assert.Equal(t, subsystems.IntentTransferFull, changeSet.IntentCode()) + assert.Equal(t, "test-state", changeSet.Selector().State()) + }) + }) + t.Run("HTTP error response", func(t *testing.T) { handler := httphelpers.HandlerWithStatus(500) httphelpers.WithServer(handler, func(ts *httptest.Server) { diff --git a/internal/datasourcev2/streaming_data_source_test.go b/internal/datasourcev2/streaming_data_source_test.go index 6e48b4d06..759d2af8d 100644 --- a/internal/datasourcev2/streaming_data_source_test.go +++ b/internal/datasourcev2/streaming_data_source_test.go @@ -184,7 +184,7 @@ func TestStreamingProcessorHandlesFallbackToFDv1(t *testing.T) { assert.Equal(t, result.State, interfaces.DataSourceStateOff) assert.Equal(t, result.Error.Kind, interfaces.DataSourceErrorKindErrorResponse) - require.NotNil(t, *result.FDv1FallbackTTL) + require.NotNil(t, result.FDv1FallbackTTL) assert.Equal(t, 120*time.Second, *result.FDv1FallbackTTL) }) } @@ -207,7 +207,7 @@ func TestStreamingProcessorFallsBackOnGoodbyeWithFallbackTTL(t *testing.T) { assert.Equal(t, interfaces.DataSourceStateInterrupted, result.State) assert.Nil(t, result.ChangeSet) - require.NotNil(t, *result.FDv1FallbackTTL) + require.NotNil(t, result.FDv1FallbackTTL) assert.Equal(t, 120*time.Second, *result.FDv1FallbackTTL) }) } @@ -247,7 +247,7 @@ func TestStreamingProcessorHandlesFallbackOnSuccessfulResponse(t *testing.T) { assert.Equal(t, interfaces.DataSourceStateValid, result.State) assert.NotNil(t, result.ChangeSet) assert.Len(t, result.ChangeSet.Changes(), 1) - require.NotNil(t, *result.FDv1FallbackTTL) + require.NotNil(t, result.FDv1FallbackTTL) }) }