-
Notifications
You must be signed in to change notification settings - Fork 5.4k
FileConfigurationProvider: Handle and forward IO exceptions to OnLoadException #126093
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
cbb97be
64d03ea
018c6f6
a6e7d92
7e07276
afaa6a4
0a76a86
4a972f6
7096d71
a77d5ca
6f9e61c
af033ee
2ce4531
e0df1e8
e667aee
442f97d
e51da87
39a548b
41c0b78
42c6eb8
0ecbe92
19ee6ee
7540b29
6655a91
3265c67
ef8b329
0708b4f
3e38ca4
6e934f5
ce0f578
c9d6bca
b92542d
3bf4ba7
db8ae23
ddae94e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,58 +58,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<string, string?>(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 | ||
|
mrek-msft marked this conversation as resolved.
|
||
| HandleException(ExceptionDispatchInfo.Capture(ex)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consumers who registered OnLoadException callbacks expecting only FileNotFoundException or InvalidDataException will now receive:
Maybe it is worth documenting it as a breaking change? |
||
| return; | ||
| } | ||
|
|
||
| return fileInfo.CreateReadStream(); | ||
| } | ||
| bool updated = false; | ||
|
|
||
| using Stream stream = OpenRead(file); | ||
| using (stream) | ||
| { | ||
| try | ||
| { | ||
| Load(stream); | ||
| updated = true; | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| if (reload) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a small behavior change here. Before when the first load fail (reload is false), the OnReload was called. Now it is not. This is fine change with me but would be good if we document it somehow. |
||
| { | ||
| Data = new Dictionary<string, string?>(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(); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Handles a missing configuration file during the initial load or a reload. | ||
| /// </summary> | ||
| /// <param name="reload"><see langword="true"/> when the provider is reloading after a change notification; | ||
| /// <see langword="false"/> when loading for first time.</param> | ||
| /// <param name="file">The file information returned by the <see cref="FileConfigurationSource.FileProvider"/>, | ||
| /// or <see langword="null"/> if no file information is available.</param> | ||
| /// <param name="directoryException">Determines if <see cref="FileNotFoundException"/> or | ||
| /// <see cref="DirectoryNotFoundException"/> is thrown on error.</param> | ||
| 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 | ||
| { | ||
|
mrek-msft marked this conversation as resolved.
|
||
| HandleException(ExceptionDispatchInfo.Capture(new DirectoryNotFoundException(error.ToString()))); | ||
| } | ||
| } | ||
| // REVIEW: Should we raise this in the base as well / instead? | ||
| OnReload(); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -122,6 +171,9 @@ static Stream OpenRead(IFileInfo fileInfo) | |
| /// <exception cref="InvalidDataException">An exception was thrown by the concrete implementation of the | ||
| /// <see cref="Load()"/> method. Use the source <see cref="FileConfigurationSource.OnLoadException"/> callback | ||
| /// if you need more control over the exception.</exception> | ||
| /// <exception cref="IOException">An I/O error occurred when opening the file. Other exceptions from the | ||
| /// underlying file provider may also be thrown. Use the source <see cref="FileConfigurationSource.OnLoadException"/> | ||
| /// callback if you need more control over the exception.</exception> | ||
| public override void Load() | ||
| { | ||
| Load(reload: false); | ||
|
|
@@ -152,6 +204,11 @@ private void HandleException(ExceptionDispatchInfo info) | |
| } | ||
| } | ||
|
|
||
| private void ClearData() | ||
| { | ||
| Data = new Dictionary<string, string?>(StringComparer.OrdinalIgnoreCase); | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public void Dispose() => Dispose(true); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.