fix: Retry after partial file reads.

pkgs/sdk/server/src/Integrations/FileDataSourceBuilder.cs

@@ -111,7 +111,8 @@ public FileDataSourceBuilder FileReader(FileDataTypes.IFileReader fileReader)
         /// Whenever possible, you should update a file's entire contents in one atomic operation; in Unix-like OSes,
         /// that can be done by creating a temporary file, writing to it, and then renaming it to replace the original
         /// file. In Windows, that is not always possible, so FileDataSource might detect an update before the file has
-        /// been fully written; in that case it will retry until it succeeds.
+        /// been fully written; in that case it will retry several times until the file can be parsed, giving up after
+        /// repeated failures until another change to the file is detected.
         /// </para>
         /// <para>
         /// Note that auto-updating may not work if any of the files you specified has an invalid directory path.

pkgs/sdk/server/src/Internal/DataSources/FileDataSource.cs

@@ -22,12 +22,38 @@ internal sealed class FileDataSource : IDataSource
         private readonly FlagFileDataMerger _dataMerger;
         private readonly FileDataTypes.IFileReader _fileReader;
         private readonly bool _skipMissingPaths;
+        private readonly bool _autoUpdate;
         private readonly Logger _logger;
         private volatile bool _started;
         private volatile bool _loadedValidData;
+        private volatile bool _disposed;
         private volatile int _lastVersion;
         private object _updateLock = new object();
 
+        private const int MaxLoadAttempts = 5;
+        private static readonly TimeSpan RetryDelay = TimeSpan.FromMilliseconds(600);
+        // Consecutive load failures (parse or read) per path within the current failure episode.
+        // An externally triggered load (Start or a file-change notification) starts a new episode
+        // and clears this, so the retry budget is per-episode, not per-lifetime. Only touched
+        // inside _updateLock.
+        private readonly Dictionary<string, int> _loadFailureCounts = new Dictionary<string, int>();
+        // Whether a delayed retry is already scheduled; at most one retry chain exists at a time,
+        // since each retry re-reads every path anyway. Only touched inside _updateLock.
+        private bool _retryPending;
+
+        /// <summary>
+        /// Constructs a file data source that loads flag and segment data from local files.
+        /// </summary>
+        /// <param name="dataSourceUpdates">receives the data set produced by each successful load</param>
+        /// <param name="fileReader">reads file contents; injectable for testing</param>
+        /// <param name="paths">the file paths to load, in order</param>
+        /// <param name="autoUpdate">true to watch the files and reload on changes; also enables the
+        /// bounded retry of loads that fail while a file is being written</param>
+        /// <param name="alternateParser">optional parser for non-JSON content (for example YAML);
+        /// null to parse JSON only</param>
+        /// <param name="skipMissingPaths">true to skip missing files instead of failing the load</param>
+        /// <param name="duplicateKeysHandling">how to handle a key that appears in more than one file</param>
+        /// <param name="logger">the destination for log output</param>
         public FileDataSource(IDataSourceUpdates dataSourceUpdates, FileDataTypes.IFileReader fileReader,
             List<string> paths, bool autoUpdate, Func<string, object> alternateParser, bool skipMissingPaths,
             FileDataTypes.DuplicateKeysHandling duplicateKeysHandling,
@@ -40,6 +66,7 @@ public FileDataSource(IDataSourceUpdates dataSourceUpdates, FileDataTypes.IFileR
             _dataMerger = new FlagFileDataMerger(duplicateKeysHandling);
             _fileReader = fileReader;
             _skipMissingPaths = skipMissingPaths;
+            _autoUpdate = autoUpdate;
             _lastVersion = 0;
             if (autoUpdate)
             {
@@ -62,7 +89,7 @@ public FileDataSource(IDataSourceUpdates dataSourceUpdates, FileDataTypes.IFileR
         public Task<bool> Start()
         {
             _started = true;
-            LoadAll();
+            LoadAll(isRetry: false);
 
             // We always complete the start task regardless of whether we successfully loaded data or not;
             // if the data files were bad, they're unlikely to become good within the short interval that
@@ -83,14 +110,38 @@ private void Dispose(bool disposing)
         {
             if (disposing)
             {
+                _disposed = true;
                 _reloader?.Dispose();
             }
         }
 
-        private void LoadAll()
+        private void LoadAll(bool isRetry)
         {
             lock (_updateLock)
             {
+                if (_disposed)
+                {
+                    return;
+                }
+                if (!isRetry)
+                {
+                    // An externally triggered load starts a new failure episode: failures
+                    // observed from here on get a fresh retry budget, and any state left over
+                    // from a previous episode is discarded.
+                    _loadFailureCounts.Clear();
+                }
+                else
+                {
+                    _retryPending = false;
+                    if (_loadFailureCounts.Count == 0)
+                    {
+                        // The failure state was cleared in the meantime (an externally triggered
+                        // load succeeded, or the chain gave up) — a reload would be redundant and
+                        // would re-Init identical data at bumped versions, firing spurious change
+                        // events.
+                        return;
+                    }
+                }
                 var version = Interlocked.Increment(ref _lastVersion);
                 var flags = new Dictionary<string, ItemDescriptor>();
                 var segments = new Dictionary<string, ItemDescriptor>();
@@ -100,7 +151,21 @@ private void LoadAll()
                     {
                         var content = _fileReader.ReadAllText(path);
                         _logger.Debug("file data: {0}", content);
-                        var data = _parser.Parse(content, version);
+                        FullDataSet<ItemDescriptor> data;
+                        try
+                        {
+                            data = _parser.Parse(content, version);
+                        }
+                        catch (Exception e)
+                        {
+                            // A file-change notification can fire while the file is mid-write, so a parse
+                            // failure may just mean we read an empty or partially written file. This applies
+                            // to any configured parser (JSON or alternate), so we treat every failure of
+                            // Parse — as opposed to reading the file — as potentially transient.
+                            HandleParseFailure(path, e);
+                            return;
+                        }
+                        _loadFailureCounts.Remove(path);
                         _dataMerger.AddToData(data, flags, segments);
                     }
                     catch (FileNotFoundException) when (_skipMissingPaths)
@@ -109,7 +174,20 @@ private void LoadAll()
                     }
                     catch (Exception e)
                     {
-                        LogHelpers.LogException(_logger, "Failed to load " + path, e);
+                        if (isRetry)
+                        {
+                            // A transient read error (for example, a writer replacing the file)
+                            // must not end a retry episode early: paths that were promised
+                            // retries would keep stale data with budget remaining, and another
+                            // file-change notification is not guaranteed. Charge the failure to
+                            // the same per-path budget and continue the chain; it logs a Warn
+                            // while retrying and an Error only on give-up.
+                            HandleRetryLoadFailure(path, e);
+                        }
+                        else
+                        {
+                            LogHelpers.LogException(_logger, "Failed to load " + path, e);
+                        }
                         return;
                     }
                 }
@@ -124,12 +202,106 @@ private void LoadAll()
             }
         }
 
+        // Called under _updateLock when parsing a path's content fails. Since an externally
+        // triggered load clears _loadFailureCounts before reading, any existing count for the
+        // path belongs to the current episode.
+        private void HandleParseFailure(string path, Exception e)
+        {
+            if (!_autoUpdate)
+            {
+                // With auto-update off, files are documented to be loaded only once, so we don't
+                // retry in the background — Start()'s result stays final.
+                LogHelpers.LogException(_logger, "Failed to parse " + path, e);
+                return;
+            }
+
+            _loadFailureCounts.TryGetValue(path, out var previousAttempts);
+            var attempts = previousAttempts + 1;
+            _loadFailureCounts[path] = attempts;
+
+            if (attempts < MaxLoadAttempts)
+            {
+                _logger.Warn("{0}: Failed to parse file ({1}); will retry in {2} ms in case it was incompletely written",
+                    path, LogValues.ExceptionSummary(e), RetryDelay.TotalMilliseconds);
+                _logger.Debug("{0}", LogValues.ExceptionTrace(e));
+                ScheduleRetry();
+            }
+            else
+            {
+                EndEpisode(path);
+                LogHelpers.LogException(_logger,
+                    string.Format("{0}: Failed to parse file after {1} attempts", path, MaxLoadAttempts), e);
+            }
+        }
+
+        // Called under _updateLock when a retry attempt fails before parsing (for example, a
+        // transient read error). Charges the failure to the path's per-episode budget and
+        // continues or ends the retry chain.
+        private void HandleRetryLoadFailure(string path, Exception e)
+        {
+            _loadFailureCounts.TryGetValue(path, out var previousAttempts);
+            var attempts = previousAttempts + 1;
+            _loadFailureCounts[path] = attempts;
+
+            if (attempts < MaxLoadAttempts)
+            {
+                _logger.Warn("{0}: Failed to read file on a retry ({1}); will retry again in {2} ms",
+                    path, LogValues.ExceptionSummary(e), RetryDelay.TotalMilliseconds);
+                _logger.Debug("{0}", LogValues.ExceptionTrace(e));
+                ScheduleRetry();
+            }
+            else
+            {
+                EndEpisode(path);
+                LogHelpers.LogException(_logger,
+                    string.Format("{0}: Failed to load file after {1} attempts; will not retry until the next detected file change",
+                        path, MaxLoadAttempts), e);
+            }
+        }
+
+        // Called under _updateLock. Ends the current failure episode for every path: the chain
+        // stopped at failedPath on every attempt, so any other paths with recorded failures were
+        // never re-attempted and their promised retries cannot happen.
+        private void EndEpisode(string failedPath)
+        {
+            _loadFailureCounts.Remove(failedPath);
+            foreach (var abandoned in _loadFailureCounts.Keys)
+            {
+                _logger.Error("{0}: Will not be retried because {1} repeatedly failed to load; both will be re-read on the next detected file change",
+                    abandoned, failedPath);
+            }
+            _loadFailureCounts.Clear();
+        }
+
+        // Called under _updateLock.
+        private void ScheduleRetry()
+        {
+            if (_retryPending)
+            {
+                return; // the already-scheduled retry will re-read every path
+            }
+            _retryPending = true;
+            Task.Run(async () =>
+            {
+                await Task.Delay(RetryDelay).ConfigureAwait(false);
+                try
+                {
+                    LoadAll(isRetry: true);
+                }
+                catch (Exception e)
+                {
+                    // Nothing observes this task, so any escaping exception would otherwise vanish.
+                    LogHelpers.LogException(_logger, "Unexpected error while retrying file data load", e);
+                }
+            });
+        }
+
         private void TriggerReload()
         {
             if (_started)
             {
                 _logger.Info("detected file modification, reloading");
-                LoadAll();
+                LoadAll(isRetry: false);
             }
         }
     }