Skip to content

Commit 0edfb9d

Browse files
committed
fix: Attach FDv1 fallback applier unconditionally so the directive halts the data system without a fallback
The previous attempt to guard FDv2DataSystem.Create against a null FDv1FallbackSynchronizer (commit dec9450) regressed Requirement 1.6.3(4): when no FDv1 fallback is configured and the directive arrives, the SDK must halt -- it must not keep retrying the failing synchronizer. Skipping the FDv1 fallback factory list also stripped the fdv1FallbackApplier from the synchronizer entry's outer-level observer, so the directive was no longer observed: nothing disposed the streaming source, and the EventSource library kept reconnecting after every 500. Attach the fdv1FallbackApplier (and the initializer variant) to the outer composite unconditionally. When an FDv1 fallback entry is in the outer list the applier advances to it; when one is not, the applier's BlockCurrent + DisposeCurrent + GoToNext sequence exhausts the outer list -- which disposes the current synchronizer (stopping reconnects) and transitions the outer composite to Off via InternalDispose. Add an end-to-end test (SynchronizerFDv1FallbackWithoutFallbackConfiguredHaltsDataSystem) that mirrors the harness "directive without FDv1 fallback configured halts the data system" scenario: a synchronizer reports Interrupted+FDv1Fallback (500 + directive), no FDv1 fallback configured, the underlying source must be disposed, and the data system must reach Off.
1 parent 4b3559e commit 0edfb9d

2 files changed

Lines changed: 101 additions & 19 deletions

File tree

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

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -89,19 +89,15 @@ public static IDataSource CreateFDv2DataSource(
8989
},
9090
(actionable) =>
9191
{
92-
// Honor the FDv1 fallback directive in the initializer phase too. When the
93-
// server returns x-ld-fd-fallback during init, skip the FDv2 synchronizer
94-
// chain entirely and switch directly to the FDv1 fallback synchronizer.
95-
if (fdv1Synchronizers != null && fdv1Synchronizers.Count > 0)
96-
{
97-
return new CompositeObserver(
98-
initializationObserver,
99-
blacklistWhenSuccessOrOff(actionable),
100-
initializerFdv1FallbackApplierFactory(actionable));
101-
}
102-
92+
// Honor the FDv1 fallback directive in the initializer phase too. The
93+
// applier is attached unconditionally: when an FDv1 fallback entry is
94+
// configured the applier advances to it; when one is not, the applier's
95+
// BlockCurrent + DisposeCurrent + GoToNext sequence exhausts the outer
96+
// list and halts the data system per Requirement 1.6.3(4).
10397
return new CompositeObserver(
104-
initializationObserver, blacklistWhenSuccessOrOff(actionable));
98+
initializationObserver,
99+
blacklistWhenSuccessOrOff(actionable),
100+
initializerFdv1FallbackApplierFactory(actionable));
105101
}
106102
));
107103
}
@@ -125,13 +121,13 @@ public static IDataSource CreateFDv2DataSource(
125121
},
126122
(actionable) =>
127123
{
128-
// Only attach FDv1 fallback applier if FDv1 synchronizers are actually provided
129-
if (fdv1Synchronizers != null && fdv1Synchronizers.Count > 0)
130-
{
131-
return new CompositeObserver(synchronizationObserver, fdv1FallbackApplierFactory(actionable));
132-
}
133-
134-
return synchronizationObserver;
124+
// The FDv1 fallback applier is attached unconditionally: when an FDv1
125+
// fallback entry exists in the outer list the applier advances to it;
126+
// when one does not, BlockCurrent + DisposeCurrent + GoToNext exhausts
127+
// the outer list and halts the data system per Requirement 1.6.3(4).
128+
// Disposing the current synchronizer is what stops the streaming source
129+
// from reconnecting.
130+
return new CompositeObserver(synchronizationObserver, fdv1FallbackApplierFactory(actionable));
135131
}
136132
));
137133
}

pkgs/sdk/server/test/Internal/FDv2DataSources/FDv2DataSourceTest.cs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1777,6 +1777,92 @@ public async Task InitializerOffWithFDv1FallbackErrorSkipsSynchronizersAndEngage
17771777
}
17781778
}
17791779

1780+
// End-to-end test: synchronizer reports the FDv1 fallback directive but no FDv1
1781+
// fallback is configured. The SDK must HALT (Requirement 1.6.3(4)) -- which in
1782+
// practice means the synchronizer's underlying data source must be disposed so it
1783+
// stops trying to reconnect. The data system reaches Off via outer composite
1784+
// exhaustion. Mirrors the harness suite "directive without FDv1 fallback configured
1785+
// halts the data system".
1786+
[Fact]
1787+
public void SynchronizerFDv1FallbackWithoutFallbackConfiguredHaltsDataSystem()
1788+
{
1789+
var capturingSink = new CapturingDataSourceUpdatesWithHeaders();
1790+
1791+
var synchronizerDisposed = false;
1792+
SourceFactory synchronizerFactory = (updatesSink) =>
1793+
new HaltMockDataSource(
1794+
onStart: async () =>
1795+
{
1796+
// 500 + directive: a recoverable status that would normally drive retries.
1797+
// The directive must take precedence and halt those retries.
1798+
updatesSink.UpdateStatus(
1799+
DataSourceState.Interrupted,
1800+
new DataSourceStatus.ErrorInfo
1801+
{
1802+
Kind = DataSourceStatus.ErrorKind.ErrorResponse,
1803+
StatusCode = 500,
1804+
FDv1Fallback = true,
1805+
Recoverable = true,
1806+
Time = DateTime.Now
1807+
});
1808+
await Task.Yield();
1809+
},
1810+
onDispose: () => synchronizerDisposed = true);
1811+
1812+
// No FDv1 fallback configured (empty list).
1813+
var dataSource = FDv2DataSource.CreateFDv2DataSource(
1814+
capturingSink,
1815+
new List<SourceFactory>(),
1816+
new List<SourceFactory> { synchronizerFactory },
1817+
new List<SourceFactory>(),
1818+
TestLogger);
1819+
1820+
try
1821+
{
1822+
_ = dataSource.Start();
1823+
1824+
// The data system must transition to Off via outer composite exhaustion. Wait a
1825+
// few status updates -- intermediate Interrupted statuses come first.
1826+
var statuses = capturingSink.WaitForStatusUpdates(2, TimeSpan.FromSeconds(5));
1827+
Assert.Contains(statuses, s => s.State == DataSourceState.Off);
1828+
1829+
// Critically, the synchronizer must have been disposed -- this is what stops the
1830+
// streaming source from reconnecting in the harness scenario.
1831+
Assert.True(synchronizerDisposed,
1832+
"synchronizer source must be disposed so the streaming source stops reconnecting");
1833+
}
1834+
finally
1835+
{
1836+
dataSource.Dispose();
1837+
}
1838+
}
1839+
1840+
// Mock data source that reports its disposal -- used to verify that the FDv1 fallback
1841+
// applier disposes the current synchronizer when no fallback is configured.
1842+
private class HaltMockDataSource : IDataSource
1843+
{
1844+
private readonly Func<Task> _onStart;
1845+
private readonly Action _onDispose;
1846+
private bool _initialized;
1847+
1848+
public HaltMockDataSource(Func<Task> onStart, Action onDispose)
1849+
{
1850+
_onStart = onStart;
1851+
_onDispose = onDispose;
1852+
}
1853+
1854+
public async Task<bool> Start()
1855+
{
1856+
await _onStart();
1857+
_initialized = true;
1858+
return true;
1859+
}
1860+
1861+
public bool Initialized => _initialized;
1862+
1863+
public void Dispose() => _onDispose();
1864+
}
1865+
17801866
// End-to-end test using the REAL FDv2StreamingDataSource (with a mock IEventSource so we
17811867
// can drive synthetic SSE events). This catches harness-style regressions in the
17821868
// streaming source's interaction with the FDv2DataSource composite that the simpler

0 commit comments

Comments
 (0)