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,24 @@ 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 MaxParseAttempts = 5;
+        private static readonly TimeSpan RetryDelay = TimeSpan.FromMilliseconds(600);
+        // Consecutive parse failures 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> _parseFailureCounts = 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;
+
         public FileDataSource(IDataSourceUpdates dataSourceUpdates, FileDataTypes.IFileReader fileReader,
             List<string> paths, bool autoUpdate, Func<string, object> alternateParser, bool skipMissingPaths,
             FileDataTypes.DuplicateKeysHandling duplicateKeysHandling,
@@ -40,6 +52,7 @@ public FileDataSource(IDataSourceUpdates dataSourceUpdates, FileDataTypes.IFileR
             _dataMerger = new FlagFileDataMerger(duplicateKeysHandling);
             _fileReader = fileReader;
             _skipMissingPaths = skipMissingPaths;
+            _autoUpdate = autoUpdate;
             _lastVersion = 0;
             if (autoUpdate)
             {
@@ -62,7 +75,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 +96,26 @@ 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: parse failures
+                    // observed from here on get a fresh retry budget, and any state left over
+                    // from a previous episode is discarded.
+                    _parseFailureCounts.Clear();
+                }
                 var version = Interlocked.Increment(ref _lastVersion);
                 var flags = new Dictionary<string, ItemDescriptor>();
                 var segments = new Dictionary<string, ItemDescriptor>();
@@ -100,7 +125,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;
+                        }
+                        _parseFailureCounts.Remove(path);
                         _dataMerger.AddToData(data, flags, segments);
                     }
                     catch (FileNotFoundException) when (_skipMissingPaths)
@@ -124,12 +163,94 @@ private void LoadAll()
             }
         }
 
+        // Called under _updateLock when parsing a path's content fails. Since an externally
+        // triggered load clears _parseFailureCounts 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;
+            }
+
+            var attempts = _parseFailureCounts.TryGetValue(path, out var previousAttempts)
+                ? previousAttempts + 1
+                : 1;
+
+            if (attempts < MaxParseAttempts)
+            {
+                _parseFailureCounts[path] = attempts;
+                _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
+            {
+                // This path kept failing, so the retry chain ends until the next external trigger.
+                // Every attempt stopped at this path, so any other paths with recorded failures
+                // were never re-attempted; their episodes end here too.
+                _parseFailureCounts.Remove(path);
+                foreach (var abandoned in _parseFailureCounts.Keys)
+                {
+                    _logger.Error("{0}: Will not be retried because {1} repeatedly failed to parse; both will be re-read on the next detected file change",
+                        abandoned, path);
+                }
+                _parseFailureCounts.Clear();
+                LogHelpers.LogException(_logger,
+                    string.Format("{0}: Failed to parse file after {1} attempts", path, MaxParseAttempts), e);
+            }
+        }
+
+        // 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
+                {
+                    RetryLoadAll();
+                }
+                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 RetryLoadAll()
+        {
+            lock (_updateLock)
+            {
+                _retryPending = false;
+                if (_disposed || _parseFailureCounts.Count == 0)
+                {
+                    // Disposed, or 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;
+                }
+                LoadAll(isRetry: true);
+            }
+        }
+
         private void TriggerReload()
         {
             if (_started)
             {
                 _logger.Info("detected file modification, reloading");
-                LoadAll();
+                LoadAll(isRetry: false);
             }
         }
     }