From 7602b64f5960e3cfe6b64ef3cb7bb3247d368aaa Mon Sep 17 00:00:00 2001 From: MichalZ <188900745+mrek-msft@users.noreply.github.com> Date: Mon, 27 Jul 2026 18:55:02 +0200 Subject: [PATCH] FileConfigurationProvider: Handle and forward IO exceptions to OnLoadException (#126093) Current behavior of FileConfigurationProvider on various types of failures at various times: | | First Load | OnChange Reload | |---|---|---| | data parsing failure | `OnLoadException` callback is called
`AddJsonFile` throws

| `OnLoadException` callback is called
program continues
no reload happen | | IO failure on file open | `AddJsonFile` throws | Silent
program continues
no reload happen
possibly raises `UnobservedTaskException` later | This PR unifies it in a way that both types of failure behave same as _data parsing failure_. This brings some behavioral changes - `OnLoadException` callback is newly triggered if IO error happen on both types of events (first load, reload) - `UnobservedTaskException` can no longer be observed in scenario when user registers `OnLoadException` callback. It can however happen when no callback is registered, so XML comment on `OnLoadException` was updated. - `OnLoadException` callbacks now receives not only InvalidDataException, but also IOException (or any other exception which (possibly custom) implementation of IFileProvider can throw). If user cast to InvalidDataException, he may miss other exceptions or get cast exceptions. Based on follow up Copilot code reviews, I did few more behavior changes at edge cases, they do not relate directly to issue and can be reverted independently if not desired. - `OnReload()` now fires only when Data actually changed. Previously on first load, if a parse or IO error occurred and `OnLoadException` set `Ignore = true`, `OnReload()` was fired unconditionally even though Data was never modified. The new code fires `OnReload()` only when Data was successfully loaded or explicitly cleared (e.g., on reload or optional missing file). For consecutive reloads with parse errors, Data is cleared and `OnReload()` still fires. This change is independent on original fix and can be reverted independently if not desirable. Fix #113964 --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../src/FileConfigurationProvider.cs | 127 ++++++--- .../src/FileConfigurationSource.cs | 5 + .../src/FileLoadExceptionContext.cs | 2 +- .../FunctionalTests/ConfigurationTests.cs | 243 ++++++++++++++++-- .../FunctionalTests/DisposableFileSystem.cs | 23 +- 5 files changed, 338 insertions(+), 62 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs index fa9f59683c9427..46bf44862d08fd 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs @@ -69,58 +69,107 @@ private void Load(bool reload) IFileInfo? file = Source.FileProvider?.GetFileInfo(Source.Path ?? string.Empty); if (file == null || !file.Exists) { - if (Source.Optional || reload) // Always optional on reload - { - Data = new Dictionary(StringComparer.OrdinalIgnoreCase); - } - else + HandleLoadingNonExisting(reload, file); + return; + } + + static Stream OpenRead(IFileInfo fileInfo) + { + if (fileInfo.PhysicalPath != null) { - var error = new StringBuilder(SR.Format(SR.Error_FileNotFound, Source.Path)); - if (!string.IsNullOrEmpty(file?.PhysicalPath)) - { - error.Append(SR.Format(SR.Error_ExpectedPhysicalPath, file.PhysicalPath)); - } - HandleException(ExceptionDispatchInfo.Capture(new FileNotFoundException(error.ToString()))); + // The default physical file info assumes asynchronous IO which results in unnecessary overhead + // especially since the configuration system is synchronous. This uses the same settings + // and disables async IO. + return new FileStream( + fileInfo.PhysicalPath, + FileMode.Open, + FileAccess.Read, + FileShare.ReadWrite, + bufferSize: 1, + FileOptions.SequentialScan); } + + return fileInfo.CreateReadStream(); } - else + + Stream stream; + try { - static Stream OpenRead(IFileInfo fileInfo) - { - if (fileInfo.PhysicalPath != null) - { - // The default physical file info assumes asynchronous IO which results in unnecessary overhead - // especially since the configuration system is synchronous. This uses the same settings - // and disables async IO. - return new FileStream( - fileInfo.PhysicalPath, - FileMode.Open, - FileAccess.Read, - FileShare.ReadWrite, - bufferSize: 1, - FileOptions.SequentialScan); - } + stream = OpenRead(file); + } + catch (Exception ex) when (ex is FileNotFoundException or DirectoryNotFoundException) + { + // assuming file was deleted in meantime, we already checked existence at the beginning once + HandleLoadingNonExisting(reload, file, ex is DirectoryNotFoundException); + return; + } + catch (Exception ex) + { + // IO error on file open, preserve existing Data + HandleException(ExceptionDispatchInfo.Capture(ex)); + return; + } - return fileInfo.CreateReadStream(); - } + bool updated = false; - using Stream stream = OpenRead(file); + using (stream) + { try { Load(stream); + updated = true; } catch (Exception ex) { if (reload) { - Data = new Dictionary(StringComparer.OrdinalIgnoreCase); + ClearData(); + updated = true; } - var exception = new InvalidDataException(SR.Format(SR.Error_FailedToLoad, file.PhysicalPath), ex); - HandleException(ExceptionDispatchInfo.Capture(exception)); + string filePath = file.PhysicalPath ?? Source.Path ?? file.Name; + var wrapped = new InvalidDataException(SR.Format(SR.Error_FailedToLoad, filePath), ex); + HandleException(ExceptionDispatchInfo.Capture(wrapped)); + } + } + + if (updated) + { + OnReload(); + } + } + + /// + /// Handles a missing configuration file during the initial load or a reload. + /// + /// when the provider is reloading after a change notification; + /// when loading for first time. + /// The file information returned by the , + /// or if no file information is available. + /// Determines if or + /// is thrown on error. + private void HandleLoadingNonExisting(bool reload, IFileInfo? file, bool directoryException = false) + { + if (Source.Optional || reload) // Always optional on reload + { + ClearData(); + OnReload(); + } + else + { + var error = new StringBuilder(SR.Format(SR.Error_FileNotFound, Source.Path ?? file?.Name)); + if (!string.IsNullOrEmpty(file?.PhysicalPath)) + { + error.Append(SR.Format(SR.Error_ExpectedPhysicalPath, file.PhysicalPath)); + } + if (!directoryException) + { + HandleException(ExceptionDispatchInfo.Capture(new FileNotFoundException(error.ToString()))); + } + else + { + HandleException(ExceptionDispatchInfo.Capture(new DirectoryNotFoundException(error.ToString()))); } } - // REVIEW: Should we raise this in the base as well / instead? - OnReload(); } /// @@ -133,6 +182,9 @@ static Stream OpenRead(IFileInfo fileInfo) /// An exception was thrown by the concrete implementation of the /// method. Use the source callback /// if you need more control over the exception. + /// An I/O error occurred when opening the file. Other exceptions from the + /// underlying file provider may also be thrown. Use the source + /// callback if you need more control over the exception. public override void Load() { Load(reload: false); @@ -163,6 +215,11 @@ private void HandleException(ExceptionDispatchInfo info) } } + private void ClearData() + { + Data = new Dictionary(StringComparer.OrdinalIgnoreCase); + } + /// public void Dispose() => Dispose(true); diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs index 1e0794d0b93ab3..a039c661d19c2b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs @@ -48,6 +48,11 @@ public abstract class FileConfigurationSource : IConfigurationSource /// /// Gets or sets the action that's called if an uncaught exception occurs in FileConfigurationProvider.Load. /// + /// + /// When is enabled, this callback is also invoked on background reload failures. + /// If the callback is not set or does not set to , + /// exceptions from background reloads will propagate unhandled on the thread pool. + /// public Action? OnLoadException { get; set; } /// diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileLoadExceptionContext.cs b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileLoadExceptionContext.cs index b8e393a397110d..8fa077cb25a0a6 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileLoadExceptionContext.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileLoadExceptionContext.cs @@ -16,7 +16,7 @@ public class FileLoadExceptionContext public FileConfigurationProvider Provider { get; set; } = null!; /// - /// Gets or sets the exception that occurred in Load. + /// Gets or sets the exception that occurred during file loading. /// public Exception Exception { get; set; } = null!; diff --git a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs index f6bbb5adc056e5..76f83fec94ecd4 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs @@ -100,7 +100,7 @@ public void ThrowsOnFileNotFoundWhenNotIgnored() Assert.Throws(() => configurationBuilder.Build()); } - + [Fact] public void CanHandleExceptionIfFileNotFound() { @@ -441,29 +441,6 @@ public void OnLoadErrorWillBeCalledOnIniLoadError() Assert.NotNull(provider); } - [Fact] - public void OnLoadErrorCanIgnoreErrors() - { - _fileSystem.WriteFile("error.json", @"{""JsonKey1"": "); - - FileConfigurationProvider provider = null; - Action jsonLoadError = c => - { - provider = c.Provider; - c.Ignore = true; - }; - - CreateBuilder() - .AddJsonFile(s => - { - s.Path = "error.json"; - s.OnLoadException = jsonLoadError; - }) - .Build(); - - Assert.NotNull(provider); - } - [Fact] [ActiveIssue("File watching is flaky (particularly on non windows. https://github.com/dotnet/runtime/issues/42036")] public void CanSetValuesAndReloadValues() @@ -538,6 +515,222 @@ await WaitForChange( Assert.Equal("JsonValue2", config["JsonKey1"]); } + [Fact] + public void BuildThrowsOnInvalidData() + { + const string FileName = $"{nameof(BuildThrowsOnInvalidData)}.json"; + + _fileSystem.WriteFile(FileName, @"{""JsonKey1"": "); + + bool callbackCalled = false; + Exception failureException = null; + + Assert.Throws(() => CreateBuilder() + .AddJsonFile(s => + { + s.Path = FileName; + s.OnLoadException = c => + { + callbackCalled = true; + failureException = c.Exception; + // c.Ignore stays false. Exception propagates from Build() + }; + }) + .Build()); + + Assert.True(callbackCalled); + Assert.IsType(failureException); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void BuildThrowsOnIoError() + { + const string FileName = $"{nameof(BuildThrowsOnIoError)}.json"; + + _fileSystem.WriteFile(FileName, @"{""JsonKey1"": ""JsonValue1"" }"); + + using (_fileSystem.LockFileReading(FileName)) + { + bool callbackCalled = false; + Exception failureException = null; + + Assert.Throws(() => CreateBuilder() + .AddJsonFile(s => + { + s.Path = FileName; + s.OnLoadException = c => + { + callbackCalled = true; + failureException = c.Exception; + // c.Ignore stays false. Exception propagates from Build() + }; + }) + .Build()); + + Assert.True(callbackCalled); + Assert.IsType(failureException); + } + } + + [Fact] + public void LoadDataErrorRaisesOnLoadException() + { + const string FileName = $"{nameof(LoadDataErrorRaisesOnLoadException)}.json"; + + _fileSystem.WriteFile(FileName, @"{""JsonKey1"": "); + + FileConfigurationProvider failingProvider = null; + Exception failureException = null; + Action jsonLoadError = c => + { + failureException = c.Exception; + failingProvider = c.Provider; + c.Ignore = true; + }; + + IConfigurationRoot cfgRoot = CreateBuilder() + .AddJsonFile(s => + { + s.Path = FileName; + s.OnLoadException = jsonLoadError; + }) + .Build(); + using IDisposable cfgRootDisposable = cfgRoot as IDisposable; + + Assert.NotNull(failingProvider); + Assert.IsType(failureException); + Assert.Null(cfgRoot["JsonKey1"]); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void LoadIoErrorRaisesOnLoadException() + { + const string FileName = $"{nameof(LoadIoErrorRaisesOnLoadException)}.json"; + + _fileSystem.WriteFile(FileName, @"{""JsonKey1"": ""JsonValue1"" }"); + + using (_fileSystem.LockFileReading(FileName)) + { + FileConfigurationProvider failingProvider = null; + Exception failureException = null; + Action jsonLoadError = c => + { + failureException = c.Exception; + failingProvider = c.Provider; + c.Ignore = true; + }; + + IConfigurationRoot cfgRoot = CreateBuilder() + .AddJsonFile(s => + { + s.Path = FileName; + s.OnLoadException = jsonLoadError; + }) + .Build(); + using IDisposable cfgRootDisposable = cfgRoot as IDisposable; + + Assert.NotNull(failingProvider); + Assert.IsType(failureException); + Assert.Null(cfgRoot["JsonKey1"]); + } + } + + [Fact] + [ActiveIssue("File watching is flaky (particularly on non windows. https://github.com/dotnet/runtime/issues/42036")] + public async Task ReloadDataErrorRaisesOnLoadException() + { + const string FileName = $"{nameof(ReloadDataErrorRaisesOnLoadException)}.json"; + + _fileSystem.WriteFile(FileName, @"{""JsonKey1"": ""JsonValue1"" }"); + + FileConfigurationProvider failingProvider = null; + Exception failureException = null; + Action jsonLoadError = c => + { + failureException = c.Exception; + failingProvider = c.Provider; + c.Ignore = true; + }; + + IConfigurationRoot cfgRoot = CreateBuilder() + .AddJsonFile(s => + { + s.Path = FileName; + s.OnLoadException = jsonLoadError; + s.ReloadOnChange = true; + }) + .Build(); + using IDisposable cfgRootDisposable = cfgRoot as IDisposable; + IChangeToken reloadToken = cfgRoot.GetReloadToken(); + + // No error should be triggered so far. + Assert.Null(failingProvider); + Assert.Null(failureException); + Assert.Equal("JsonValue1", cfgRoot["JsonKey1"]); + Assert.False(reloadToken.HasChanged); + + _fileSystem.WriteFile(FileName, @"{""JsonKey1"": "); + + await WaitForChange(() => failingProvider != null, "File change did not raise OnLoadException event in time."); + + Assert.IsType(failureException); + + // Check that value was removed from config + Assert.Null(cfgRoot["JsonKey1"]); + Assert.True(reloadToken.HasChanged); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + [ActiveIssue("File watching is flaky (particularly on non windows. https://github.com/dotnet/runtime/issues/42036")] + public async Task ReloadIoErrorRaisesOnLoadException() + { + const string FileName = $"{nameof(ReloadIoErrorRaisesOnLoadException)}.json"; + + _fileSystem.WriteFile(FileName, @"{""JsonKey1"": ""JsonValue1"" }"); + + FileConfigurationProvider failingProvider = null; + Exception failureException = null; + Action jsonLoadError = c => + { + failureException = c.Exception; + failingProvider = c.Provider; + c.Ignore = true; + }; + + IConfigurationRoot cfgRoot = CreateBuilder() + .AddJsonFile(s => + { + s.Path = FileName; + s.OnLoadException = jsonLoadError; + s.ReloadOnChange = true; + }) + .Build(); + using IDisposable cfgRootDisposable = cfgRoot as IDisposable; + IChangeToken reloadToken = cfgRoot.GetReloadToken(); + + // No error should be triggered so far. + Assert.Null(failingProvider); + Assert.Null(failureException); + Assert.Equal("JsonValue1", cfgRoot["JsonKey1"]); + Assert.False(reloadToken.HasChanged); + + using (_fileSystem.LockFileReading(FileName)) + { + // we need NoWait because Wait reads file under the hood and that is restricted in LockFileReading context + _fileSystem.WriteFileNoWait(FileName, @"{""JsonKey1"": ""JsonValue1Updated"" }"); + + await WaitForChange(() => failingProvider != null, "File change did not raise OnLoadException event in time."); + Assert.IsType(failureException); + + // IO error on reload does not invalidate existing config, yet value is not updated + Assert.Equal("JsonValue1", cfgRoot["JsonKey1"]); + Assert.False(reloadToken.HasChanged); + } + } + [Fact] [ActiveIssue("File watching is flaky (particularly on non windows. https://github.com/dotnet/runtime/issues/42036")] public async Task TouchingFileWillReload() @@ -689,7 +882,7 @@ await WaitForChange( Assert.Equal("IniValue1", config["Key"]); Assert.True(token.HasChanged); } - + [Theory] [ActiveIssue("File watching is flaky (particularly on non windows. https://github.com/dotnet/runtime/issues/42036")] [InlineData(false)] diff --git a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/DisposableFileSystem.cs b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/DisposableFileSystem.cs index 0c16295d5be547..b5bd0c1968bda9 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/DisposableFileSystem.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/DisposableFileSystem.cs @@ -48,7 +48,7 @@ public DisposableFileSystem WriteFile(string path, string text = "temp", bool ab ? path : Path.Combine(RootPath, path); - File.WriteAllText(fullPath, text); + WriteFileNoWait(path, text, absolute); WaitForFileSystem( () => File.ReadAllText(fullPath).Length == text.Length, @@ -57,6 +57,17 @@ public DisposableFileSystem WriteFile(string path, string text = "temp", bool ab return this; } + public DisposableFileSystem WriteFileNoWait(string path, string text = "temp", bool absolute = false) + { + var fullPath = absolute + ? path + : Path.Combine(RootPath, path); + + File.WriteAllText(fullPath, text); + + return this; + } + public DisposableFileSystem DeleteFile(string path, bool absolute = false) { var fullPath = absolute @@ -91,6 +102,16 @@ public DisposableFileSystem CreateFiles(params string[] fileRelativePaths) return this; } + /// + /// Lock specified file for reading. However, it can still be written to, and changes trigger FileSystemWatcher events. + /// + /// IDisposable which removes lock on Dispose() + public IDisposable LockFileReading(string path) + { + var fullPath = Path.Combine(RootPath, path); + return new FileStream(fullPath, FileMode.Open, FileAccess.Read, FileShare.Write); + } + public void Dispose() { try