From 0ce3a5e847a28ffb16e28bd7bb331adb5386f3df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Luthi?= <cedric.luthi@gmail.com>
Date: Mon, 27 Apr 2020 15:48:35 +0200
Subject: [PATCH 1/4] Include the file path in the exception when configuration
 loading fails

If the configuration file doesn't exist, a nice `FileNotFoundException` is thrown, including the physical path where it is expected to be found. If the file is found but fails to load, the file configuration provider exception is thrown, losing the file path information, making it hard to diagnose. Here is what it looks like. The line number is included but the file path is lost:

```
Unhandled exception. System.FormatException: Could not parse the JSON file.
 ---> System.Text.Json.JsonReaderException: Expected depth to be zero at the end of the JSON payload. There is an open JSON object or array that should be closed. LineNumber: 9 | BytePositionInLine: 0.
   at System.Text.Json.ThrowHelper.ThrowJsonReaderException(Utf8JsonReader& json, ExceptionResource resource, Byte nextByte, ReadOnlySpan`1 bytes)
   at System.Text.Json.Utf8JsonReader.ReadSingleSegment()
   at System.Text.Json.Utf8JsonReader.Read()
   at System.Text.Json.JsonDocument.Parse(ReadOnlySpan`1 utf8JsonSpan, Utf8JsonReader reader, MetadataDb& database, StackRowStack& stack)
   at System.Text.Json.JsonDocument.Parse(ReadOnlyMemory`1 utf8Json, JsonReaderOptions readerOptions, Byte[] extraRentedBytes)
   at System.Text.Json.JsonDocument.Parse(ReadOnlyMemory`1 json, JsonDocumentOptions options)
   at System.Text.Json.JsonDocument.Parse(String json, JsonDocumentOptions options)
   at Microsoft.Extensions.Configuration.Json.JsonConfigurationFileParser.ParseStream(Stream input)
   at Microsoft.Extensions.Configuration.Json.JsonConfigurationProvider.Load(Stream stream)
   --- End of inner exception stack trace ---
   at Microsoft.Extensions.Configuration.Json.JsonConfigurationProvider.Load(Stream stream)
   at Microsoft.Extensions.Configuration.FileConfigurationProvider.Load(Boolean reload)
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.Extensions.Configuration.FileConfigurationProvider.HandleException(ExceptionDispatchInfo info)
   at Microsoft.Extensions.Configuration.FileConfigurationProvider.Load(Boolean reload)
   at Microsoft.Extensions.Configuration.FileConfigurationProvider.Load()
   at Microsoft.Extensions.Configuration.ConfigurationRoot..ctor(IList`1 providers)
   at Microsoft.Extensions.Configuration.ConfigurationBuilder.Build()
   at Microsoft.Extensions.Hosting.HostBuilder.BuildAppConfiguration()
   at Microsoft.Extensions.Hosting.HostBuilder.Build()
```

This commit wraps the internal file configuration provider exception into an `InvalidDataException` that include the physical path in the exception's message in order to ease diagnostics.
---
 .../src/FileConfigurationProvider.cs                        | 6 +++++-
 .../src/Resources/Strings.resx                              | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
index d1361140a7d33..d36b3459cdf37 100644
--- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
@@ -88,7 +88,8 @@ private void Load(bool reload)
                     }
                     catch (Exception e)
                     {
-                        HandleException(ExceptionDispatchInfo.Capture(e));
+                        var exception = new InvalidDataException(SR.Format(SR.Error_LoadFailed, file.PhysicalPath), e);
+                        HandleException(ExceptionDispatchInfo.Capture(exception));
                     }
                 }
             }
@@ -101,6 +102,9 @@ private void Load(bool reload)
         /// </summary>
         /// <exception cref="FileNotFoundException">If Optional is <c>false</c> on the source and a
         /// file does not exist at specified Path.</exception>
+        /// <exception cref="InvalidDataException">Wrapping any exception 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>
         public override void Load()
         {
             Load(reload: false);
diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Resources/Strings.resx
index f776e2c275e9f..6d02a4222b4d4 100644
--- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Resources/Strings.resx
+++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Resources/Strings.resx
@@ -123,4 +123,7 @@
   <data name="Error_FileNotFound" xml:space="preserve">
     <value>The configuration file '{0}' was not found and is not optional.</value>
   </data>
+  <data name="Error_LoadFailed" xml:space="preserve">
+    <value>Failed to load configuration from file '{0}'.</value>
+  </data>
 </root>
\ No newline at end of file

From 31f029153417daf811097a0b5d0229f065bba575 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Luthi?= <cedric.luthi@gmail.com>
Date: Mon, 27 Apr 2020 11:43:08 +0200
Subject: [PATCH 2/4] Use strings defined in resources instead of hardcoded
 values

---
 .../src/FileConfigurationProvider.cs                          | 4 ++--
 .../src/Resources/Strings.resx                                | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
index d36b3459cdf37..8da9ec52074de 100644
--- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
@@ -65,10 +65,10 @@ private void Load(bool reload)
                 }
                 else
                 {
-                    var error = new StringBuilder($"The configuration file '{Source.Path}' was not found and is not optional.");
+                    var error = new StringBuilder(SR.Format(SR.Error_FileNotFound, Source.Path));
                     if (!string.IsNullOrEmpty(file?.PhysicalPath))
                     {
-                        error.Append($" The physical path is '{file.PhysicalPath}'.");
+                        error.Append(SR.Format(SR.Error_ExpectedPhysicalPath, file.PhysicalPath));
                     }
                     HandleException(ExceptionDispatchInfo.Capture(new FileNotFoundException(error.ToString())));
                 }
diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Resources/Strings.resx
index 6d02a4222b4d4..96dbbe839013f 100644
--- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Resources/Strings.resx
+++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Resources/Strings.resx
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <root>
   <!-- 
     Microsoft ResX Schema 
@@ -118,7 +118,7 @@
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
   <data name="Error_ExpectedPhysicalPath" xml:space="preserve">
-    <value>The expected physical path was '{0}'.</value>
+    <value> The expected physical path is '{0}'.</value>
   </data>
   <data name="Error_FileNotFound" xml:space="preserve">
     <value>The configuration file '{0}' was not found and is not optional.</value>

From fefbfb2156a466b33790d275b40de34a08ce4bca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Luthi?= <cedric.luthi@gmail.com>
Date: Tue, 28 Apr 2020 19:06:18 +0200
Subject: [PATCH 3/4] Add test for InvalidDataException when load fails

---
 .../tests/FileConfigurationProviderTest.cs    | 49 +++++++++++++++++--
 1 file changed, 44 insertions(+), 5 deletions(-)

diff --git a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/tests/FileConfigurationProviderTest.cs b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/tests/FileConfigurationProviderTest.cs
index 6f952461630f0..af76cc445f6de 100644
--- a/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/tests/FileConfigurationProviderTest.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/tests/FileConfigurationProviderTest.cs
@@ -3,11 +3,9 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Collections.Generic;
 using System.IO;
 using Microsoft.Extensions.Configuration.Test;
 using Microsoft.Extensions.FileProviders;
-using Microsoft.Extensions.Primitives;
 using Moq;
 using Xunit;
 
@@ -35,14 +33,55 @@ public void ProviderDisposesChangeTokenRegistration()
             Assert.Empty(changeToken.Callbacks);
         }
 
+        [Theory]
+        [InlineData(@"C:\path\to\configuration.txt")]
+        [InlineData(@"/path/to/configuration.txt")]
+        public void ProviderThrowsInvalidDataExceptionWhenLoadFails(string physicalPath)
+        {
+            var fileProviderMock = new Mock<IFileProvider>();
+            fileProviderMock.Setup(fp => fp.Watch(It.IsAny<string>())).Returns(new ConfigurationRootTest.ChangeToken());
+            fileProviderMock.Setup(fp => fp.GetFileInfo(It.IsAny<string>())).Returns(new FileInfoImpl(physicalPath));
+
+            var source = new FileConfigurationSourceImpl
+            {
+                FileProvider = fileProviderMock.Object,
+                ReloadOnChange = true,
+            };
+            var exceptionOnLoad = new Exception("An error occured");
+            var provider = new FileConfigurationProviderImpl(source, exceptionOnLoad);
+
+            var exception = Assert.Throws<InvalidDataException>(() => provider.Load());
+            Assert.Contains($"'{physicalPath}'", exception.Message);
+            Assert.Equal(exceptionOnLoad, exception.InnerException);
+        }
+
+        public class FileInfoImpl : IFileInfo
+        {
+            public FileInfoImpl(string physicalPath) => PhysicalPath = physicalPath;
+            public Stream CreateReadStream() => new MemoryStream();
+            public bool Exists => true;
+            public bool IsDirectory => false;
+            public DateTimeOffset LastModified => default;
+            public long Length => default;
+            public string Name => default;
+            public string PhysicalPath { get; }
+        }
+
         public class FileConfigurationProviderImpl : FileConfigurationProvider
         {
-            public FileConfigurationProviderImpl(FileConfigurationSource source)
+            private readonly Exception _exceptionOnLoad;
+
+            public FileConfigurationProviderImpl(FileConfigurationSource source, Exception exceptionOnLoad = null)
                 : base(source)
-            { }
+            {
+                _exceptionOnLoad = exceptionOnLoad;
+            }
 
             public override void Load(Stream stream)
-            { }
+            {
+                if (_exceptionOnLoad != null)
+                    throw _exceptionOnLoad;
+            }
         }
 
         public class FileConfigurationSourceImpl : FileConfigurationSource

From 5985440d1ba9c77571852e966f0a4716bd28d7d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Luthi?= <cedric.luthi@gmail.com>
Date: Tue, 28 Apr 2020 22:14:51 +0200
Subject: [PATCH 4/4] Adapt configuration functional tests

---
 .../tests/FunctionalTests/ConfigurationTests.cs      | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs
index 9b4d8e9462bef..25c61b1df699d 100644
--- a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/ConfigurationTests.cs
@@ -382,9 +382,9 @@ public void OnLoadErrorWillBeCalledOnJsonParseError()
                     .SetFileLoadExceptionHandler(jsonLoadError)
                     .Build();
             }
-            catch (Exception e)
+            catch (InvalidDataException e)
             {
-                Assert.Equal(e, jsonError);
+                Assert.Equal(e.InnerException, jsonError);
             }
 
             Assert.NotNull(provider);
@@ -410,9 +410,9 @@ public void OnLoadErrorWillBeCalledOnXmlParseError()
                     .SetFileLoadExceptionHandler(loadError)
                     .Build();
             }
-            catch (Exception e)
+            catch (InvalidDataException e)
             {
-                Assert.Equal(e, error);
+                Assert.Equal(e.InnerException, error);
             }
 
             Assert.NotNull(provider);
@@ -439,9 +439,9 @@ public void OnLoadErrorWillBeCalledOnIniLoadError()
                     .SetFileLoadExceptionHandler(loadError)
                     .Build();
             }
-            catch (FormatException e)
+            catch (InvalidDataException e)
             {
-                Assert.Equal(e, error);
+                Assert.Equal(e.InnerException, error);
             }
             Assert.NotNull(provider);
         }