fix: Honor x-ld-fd-fallback header in FDv2 initializer phase and on successful responses (#251)

Author: keelerm84

.github/actions/contract-tests/action.yml

@@ -12,7 +12,7 @@ inputs:
     required: false
     default: ''
   run_fdv2_tests:
-    description: 'Whether to run contract tests from the v3.0.0-alpha.3 tag'
+    description: 'Whether to run contract tests from the v3.0.0-alpha.6 tag'
     required: false
     default: 'false'
 
@@ -64,16 +64,25 @@ runs:
       shell: bash
       run: dotnet ${{ inputs.service_dll_file }} > test-service.log 2>&1 & disown
 
-    - name: Clone and run contract tests from v3.0.0-alpha.3 tag
+    - name: Clone and run contract tests from v3.0.0-alpha.6 tag
       if: inputs.run_fdv2_tests == 'true'
       shell: bash
       run: |
         mkdir -p /tmp/sdk-test-harness
         git clone https://github.com/launchdarkly/sdk-test-harness.git /tmp/sdk-test-harness
         cp $(dirname ./${{ inputs.service_project_file }})/test-supressions-fdv2.txt /tmp/sdk-test-harness/testharness-suppressions-fdv2.txt
         cd /tmp/sdk-test-harness
-        git checkout v3.0.0-alpha.3
+        git checkout v3.0.0-alpha.6
         go build -o test-harness .
         ./test-harness -url http://localhost:8000 -debug -status-timeout=360 --skip-from=testharness-suppressions-fdv2.txt --stop-service-at-end
       env:
         GITHUB_TOKEN: ${{ inputs.token }}
+
+    - name: Upload test service log on failure
+      if: failure() && inputs.run_fdv2_tests == 'true'
+      uses: actions/upload-artifact@v4
+      with:
+        name: contract-test-service-log-${{ runner.os }}-${{ runner.arch }}
+        path: test-service.log
+        if-no-files-found: warn
+        retention-days: 7

pkgs/sdk/server/contract-tests/Representations.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Text.Json.Serialization;
 using LaunchDarkly.Sdk;
 
 // Note, in order for System.Text.Json serialization/deserialization to work correctly, the members of
@@ -153,6 +154,12 @@ public class SdkConfigDataSystemParams
         public int? StoreMode { get; set; }
         public SdkConfigDataInitializerParams[] Initializers { get; set; }
         public SdkConfigDataSynchronizerParams[] Synchronizers { get; set; }
+        // FDv1Fallback configures the SDK's FDv1 Fallback Synchronizer, which is engaged only when
+        // the LaunchDarkly server returns the FDv1 fallback directive. It is distinct from the FDv2
+        // Primary/Fallback Synchronizers above. The harness sends this as "fdv1Fallback" (lowercase
+        // 'd'); we override the default CamelCase mapping (which would produce "fDv1Fallback").
+        [JsonPropertyName("fdv1Fallback")]
+        public SdkConfigPollingParams FDv1Fallback { get; set; }
         public string PayloadFilter { get; set; }
     }
 

pkgs/sdk/server/contract-tests/SdkClientEntity.cs

@@ -503,47 +503,29 @@ private static Configuration BuildSdkConfig(SdkConfigParams sdkParams, ILogAdapt
                     if (synchronizers.Count > 0)
                     {
                         dataSystemBuilder.Synchronizers(synchronizers.ToArray());
-                        
-                        // Find the best synchronizer to use for FDv1 fallback configuration
-                        // Prefer polling synchronizers since FDv1 fallback is polling-based
-                        SdkConfigDataSynchronizerParams synchronizerForFallback = null;
-                        
-                        // First, try to find a polling synchronizer
-                        foreach (var syncParams in sdkParams.DataSystem.Synchronizers)
-                        {
-                            if (syncParams.Polling != null)
-                            {
-                                synchronizerForFallback = syncParams;
-                                break;
-                            }
-                        }
-                        
-                        // If no polling synchronizer found, use the first synchronizer (could be streaming)
-                        if (synchronizerForFallback == null && sdkParams.DataSystem.Synchronizers.Length > 0)
-                        {
-                            synchronizerForFallback = sdkParams.DataSystem.Synchronizers[0];
-                        }
-                        
-                        if (synchronizerForFallback != null)
-                        {
-                            // Only configure global polling endpoints if we have a polling synchronizer with a custom base URI
-                            // This ensures the FDv1 fallback synchronizer uses the same base URI without overwriting
-                            // existing polling endpoint configuration
-                            if (synchronizerForFallback.Polling != null && 
-                                synchronizerForFallback.Polling.BaseUri != null)
-                            {
-                                endpoints.Polling(synchronizerForFallback.Polling.BaseUri);
-                            }
-                            
-                            var fdv1Fallback = CreateFDv1FallbackSynchronizer(synchronizerForFallback);
-                            if (fdv1Fallback != null)
-                            {
-                                dataSystemBuilder.FDv1FallbackSynchronizer(fdv1Fallback);
-                            }
-                        }
                     }
                 }
 
+                // Configure the FDv1 Fallback Synchronizer directly from dataSystem.fdv1Fallback,
+                // separate from the FDv2 Primary/Fallback synchronizer chain. This is engaged only
+                // in response to a server-directed FDv1 Fallback Directive.
+                if (sdkParams.DataSystem.FDv1Fallback != null)
+                {
+                    if (sdkParams.DataSystem.FDv1Fallback.BaseUri != null)
+                    {
+                        endpoints.Polling(sdkParams.DataSystem.FDv1Fallback.BaseUri);
+                    }
+
+                    var fdv1FallbackBuilder = DataSystemComponents.FDv1Polling();
+                    if (sdkParams.DataSystem.FDv1Fallback.PollIntervalMs.HasValue)
+                    {
+                        fdv1FallbackBuilder.PollInterval(
+                            TimeSpan.FromMilliseconds(sdkParams.DataSystem.FDv1Fallback.PollIntervalMs.Value));
+                    }
+
+                    dataSystemBuilder.FDv1FallbackSynchronizer(fdv1FallbackBuilder);
+                }
+
                 builder.DataSystem(dataSystemBuilder);
             }
 
@@ -595,33 +577,6 @@ private static IComponentConfigurer<IDataSource> CreateSynchronizer(
             return null;
         }
 
-        private static IComponentConfigurer<IDataSource> CreateFDv1FallbackSynchronizer(
-            SdkConfigDataSynchronizerParams synchronizer)
-        {
-            // FDv1 fallback synchronizer is always polling-based
-            var fdv1PollingBuilder = DataSystemComponents.FDv1Polling();
-            
-            // Configure polling interval if the synchronizer has polling configuration
-            if (synchronizer.Polling != null)
-            {
-                if (synchronizer.Polling.PollIntervalMs.HasValue)
-                {
-                    fdv1PollingBuilder.PollInterval(TimeSpan.FromMilliseconds(synchronizer.Polling.PollIntervalMs.Value));
-                }
-                // Note: FDv1 polling doesn't support ServiceEndpointsOverride, so base URI
-                // will use the global service endpoints configuration
-            }
-            else if (synchronizer.Streaming != null)
-            {
-                // For streaming synchronizers, we still create a polling fallback
-                // Use default polling interval since streaming doesn't have a poll interval
-                // Note: FDv1 polling doesn't support ServiceEndpointsOverride, so base URI
-                // will use the global service endpoints configuration
-            }
-            
-            return fdv1PollingBuilder;
-        }
-
         private MigrationVariationResponse DoMigrationVariation(MigrationVariationParams migrationVariation)
         {
             var defaultStage = MigrationStageExtensions.FromDataModelString(migrationVariation.DefaultStage);

pkgs/sdk/server/contract-tests/TestService.cs

@@ -38,7 +38,8 @@ public class Webapp
             "inline-context-all",
             "anonymous-redaction",
             "evaluation-hooks",
-            "client-prereq-events"
+            "client-prereq-events",
+            "fdv1-fallback"
         };
 
         public readonly Handler Handler;

pkgs/sdk/server/src/Internal/DataSources/CompositeDataSource/CompositeEntryKind.cs

@@ -0,0 +1,23 @@
+namespace LaunchDarkly.Sdk.Server.Internal.DataSources
+{
+    /// <summary>
+    /// Categorizes an entry in a composite source's list. Appliers use this to express
+    /// "block every FDv2 entry" via <see cref="ICompositeSourceActionable.BlockAll"/>
+    /// without having to count positions or know which phase they were attached at.
+    /// </summary>
+    internal enum CompositeEntryKind
+    {
+        /// <summary>
+        /// Default kind. Entries that participate in the FDv2 protocol -- initializers and
+        /// FDv2 synchronizers in the outer composite, and any entry that is not the
+        /// FDv1 fallback synchronizer.
+        /// </summary>
+        FDv2,
+
+        /// <summary>
+        /// The FDv1 fallback synchronizer entry. Used by the FDv1 fallback applier to
+        /// distinguish the fallback target from the FDv2 entries it should block.
+        /// </summary>
+        FDv1Fallback
+    }
+}

pkgs/sdk/server/src/Internal/DataSources/CompositeDataSource/CompositeSource.cs

@@ -30,27 +30,31 @@ internal sealed class CompositeSource : IDataSource, ICompositeSourceActionable
 
         private readonly IDataSourceUpdatesV2 _originalUpdateSink;
         private readonly IDataSourceUpdatesV2 _sanitizedUpdateSink;
-        private readonly SourcesList<(SourceFactory Factory, ActionApplierFactory ActionApplierFactory)> _sourcesList;
+        private readonly SourcesList<(SourceFactory Factory, ActionApplierFactory ActionApplierFactory, CompositeEntryKind Kind)> _sourcesList;
         private readonly DisableableDataSourceUpdatesTracker _disableableTracker;
 
         // Tracks the entry from the sources list that was used to create the current
         // data source instance. This allows operations such as blacklist to remove
         // the correct factory/action-applier-factory tuple from the list.
-        private (SourceFactory Factory, ActionApplierFactory ActionApplierFactory) _currentEntry;
+        private (SourceFactory Factory, ActionApplierFactory ActionApplierFactory, CompositeEntryKind Kind) _currentEntry;
         private IDataSource _currentDataSource;
 
         /// <summary>
-        /// Creates a new <see cref="CompositeSource"/>.
+        /// Creates a new <see cref="CompositeSource"/>. Each entry carries an explicit
+        /// <see cref="CompositeEntryKind"/> so appliers can express "block every FDv2 entry"
+        /// via <see cref="ICompositeSourceActionable.BlockAll"/> without needing to know list
+        /// positions. For inner sub-composites whose entries never participate in cross-kind
+        /// blocking, callers should pass <see cref="CompositeEntryKind.FDv2"/> uniformly.
         /// </summary>
         /// <param name="compositeDescription">description of the composite source for logging purposes</param>
         /// <param name="updatesSink">the sink that receives updates from the active source</param>
-        /// <param name="factoryTuples">the ordered list of source factories and their associated action applier factories</param>
+        /// <param name="factoryTuples">the ordered list of source factories, action applier factories, and entry kinds</param>
         /// <param name="logger">the logger instance to use</param>
         /// <param name="circular">whether to loop off the end of the list back to the start when fallback occurs</param>
         public CompositeSource(
             string compositeDescription,
             IDataSourceUpdatesV2 updatesSink,
-            IList<(SourceFactory Factory, ActionApplierFactory ActionApplierFactory)> factoryTuples,
+            IList<(SourceFactory Factory, ActionApplierFactory ActionApplierFactory, CompositeEntryKind Kind)> factoryTuples,
             Logger logger,
             bool circular = true)
         {
@@ -72,7 +76,7 @@ public CompositeSource(
             // this tracker is used to disconnect the current source from the updates sink when it is no longer needed.
             _disableableTracker = new DisableableDataSourceUpdatesTracker();
 
-            _sourcesList = new SourcesList<(SourceFactory SourceFactory, ActionApplierFactory ActionApplierFactory)>(
+            _sourcesList = new SourcesList<(SourceFactory Factory, ActionApplierFactory ActionApplierFactory, CompositeEntryKind Kind)>(
                 circular: circular,
                 initialList: factoryTuples
             );
@@ -451,6 +455,38 @@ public void BlockCurrent()
             });
         }
 
+        public void BlockAll(Predicate<CompositeEntryKind> kindMatches)
+        {
+            if (kindMatches is null) throw new ArgumentNullException(nameof(kindMatches));
+            if (_disposed)
+            {
+                return;
+            }
+
+            EnqueueAction(() =>
+            {
+                lock (_lock)
+                {
+                    var removed = _sourcesList.RemoveAll(entry => kindMatches(entry.Kind));
+                    if (removed == 0)
+                    {
+                        return;
+                    }
+
+                    // If the current entry was removed, clear the reference so a subsequent
+                    // BlockCurrent doesn't accidentally remove a remaining entry. The current
+                    // data source is left running -- callers are expected to follow BlockAll
+                    // with DisposeCurrent / GoToNext when they want it torn down.
+                    if (_currentEntry != default && kindMatches(_currentEntry.Kind))
+                    {
+                        _currentEntry = default;
+                    }
+
+                    _log.Debug("{0} blocked {1} entries by kind predicate.", _compositeDescription, removed);
+                }
+            });
+        }
+
         private void logTransition(String previousDescription, String currentDescription) {
             if (previousDescription != null && currentDescription != null) {
                 _log.Debug("{0} transitioned from {1} to {2}.", _compositeDescription, previousDescription, currentDescription);

pkgs/sdk/server/src/Internal/DataSources/CompositeDataSource/ICompositeSourceActionable.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Threading.Tasks;
 
 namespace LaunchDarkly.Sdk.Server.Internal.DataSources
@@ -34,11 +35,20 @@ internal interface ICompositeSourceActionable
         bool IsAtFirst();
 
         /// <summary>
-        /// Blocks the the current source's factory. This prevents the current source's factory from being used again. 
+        /// Blocks the the current source's factory. This prevents the current source's factory from being used again.
         /// Note that this does not tear down the current data source, it just prevents its factory from being used again.
         /// </summary>
         void BlockCurrent();
+
+        /// <summary>
+        /// Removes every entry whose kind matches the predicate from the source list, regardless
+        /// of its position. The current data source is not disposed -- callers that want the
+        /// current source torn down should follow this with <see cref="DisposeCurrent"/> or
+        /// <see cref="GoToNext"/>. If the current entry was removed, the internal "current entry"
+        /// reference is cleared so subsequent block operations don't accidentally remove a
+        /// remaining entry.
+        /// </summary>
+        /// <param name="kindMatches">predicate selecting which kinds to remove</param>
+        void BlockAll(Predicate<CompositeEntryKind> kindMatches);
     }
 }
-
-

pkgs/sdk/server/src/Internal/DataSources/CompositeDataSource/SourcesList.cs

@@ -112,6 +112,38 @@ public int IndexOf(T element)
             return _list.IndexOf(element);
         }
 
+        /// <summary>
+        /// Removes every element that matches the predicate, adjusting the head position so it
+        /// continues to point at whichever element followed the previous head. Returns the number
+        /// of elements removed.
+        /// </summary>
+        public int RemoveAll(Predicate<T> match)
+        {
+            if (match is null) return 0;
+            // Walk back-to-front so a removal never shifts an unvisited index. Adjust _pos as we go
+            // by mirroring SourcesList.Remove's "if removed index is before head, head moves left".
+            var removed = 0;
+            for (var i = _list.Count - 1; i >= 0; i--)
+            {
+                if (!match(_list[i])) continue;
+                _list.RemoveAt(i);
+                removed++;
+                if (i < _pos)
+                {
+                    _pos -= 1;
+                }
+            }
+            if (_list.Count == 0)
+            {
+                _pos = 0;
+            }
+            else if (_circular && _pos > _list.Count - 1)
+            {
+                _pos = 0;
+            }
+            return removed;
+        }
+
         /// <summary>
         /// Reset the head position to the start of the list.
         /// </summary>

pkgs/sdk/server/src/Internal/DataSystem/FDv2DataSystem.cs

@@ -79,14 +79,20 @@ public static FDv2DataSystem Create(Logger logger, Configuration configuration,
 
             var contextWithSelectorSource =
                 clientContext.WithSelectorSource(new SelectorSourceFacade(writeThroughStore));
+            // FDv1 fallback synchronizer is optional; only build a list entry when one is
+            // configured. An always-present list entry that captured a null configurer would
+            // throw NRE the moment the action applier advanced to the FDv1 fallback entry.
+            var fdv1FallbackFactories = dataSystemConfiguration.FDv1FallbackSynchronizer == null
+                ? new List<SourceFactory>()
+                : new List<SourceFactory>
+                {
+                    FactoryWithContext(clientContext)(dataSystemConfiguration.FDv1FallbackSynchronizer)
+                };
             var compositeDataSource = configuration.Offline ? Components.ExternalUpdatesOnly.Build(contextWithSelectorSource) : FDv2DataSource.CreateFDv2DataSource(
                 dataSourceUpdates,
                 dataSystemConfiguration.Initializers.Select(FactoryWithContext(contextWithSelectorSource)).ToList(),
                 dataSystemConfiguration.Synchronizers.Select(FactoryWithContext(contextWithSelectorSource)).ToList(),
-                new List<SourceFactory>
-                {
-                    FactoryWithContext(clientContext)(dataSystemConfiguration.FDv1FallbackSynchronizer)
-                },
+                fdv1FallbackFactories,
                 logger
             );
 

pkgs/sdk/server/src/Internal/FDv2DataSources/FDv2ChangeSetTranslator.cs

@@ -18,12 +18,14 @@ internal static class FDv2ChangeSetTranslator
         /// <param name="changeset">The FDv2 changeset to convert.</param>
         /// <param name="log">Logger for diagnostic messages.</param>
         /// <param name="environmentId">The environment ID to include in the changeset.</param>
+        /// <param name="fdv1Fallback">Whether to mark the changeset as carrying the FDv1 fallback directive.</param>
         /// <returns>A DataStoreTypes.ChangeSet containing the converted data.</returns>
         /// <exception cref="ArgumentOutOfRangeException">Thrown when the changeset type is unknown.</exception>
         public static DataStoreTypes.ChangeSet<DataStoreTypes.ItemDescriptor> ToChangeSet(
             FDv2ChangeSet changeset,
             Logger log,
-            string environmentId = null)
+            string environmentId = null,
+            bool fdv1Fallback = false)
         {
             DataStoreTypes.ChangeSetType changeSetType;
             switch (changeset.Type)
@@ -102,7 +104,8 @@ internal static class FDv2ChangeSetTranslator
                 changeSetType,
                 changeset.Selector,
                 dataBuilder.ToImmutable(),
-                environmentId);
+                environmentId,
+                fdv1Fallback);
         }
 
         /// <summary>