[Fixes #6533] Log when XML formatters fail to create a serializer

src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlDataContractSerializerMvcOptionsSetup.cs

@@ -1,8 +1,10 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
+using System;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
@@ -13,6 +15,22 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
     /// </summary>
     public class MvcXmlDataContractSerializerMvcOptionsSetup : IConfigureOptions<MvcOptions>
     {
+        private readonly ILoggerFactory _loggerFactory;
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="MvcXmlDataContractSerializerMvcOptionsSetup"/>.
+        /// </summary>
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        public MvcXmlDataContractSerializerMvcOptionsSetup(ILoggerFactory loggerFactory)
+        {
+            if (loggerFactory == null)
+            {
+                throw new ArgumentNullException(nameof(loggerFactory));
+            }
+
+            _loggerFactory = loggerFactory;
+        }
+
         /// <summary>
         /// Adds the data contract serializer formatters to <see cref="MvcOptions"/>.
         /// </summary>
@@ -21,7 +39,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
         {
             options.ModelMetadataDetailsProviders.Add(new DataMemberRequiredBindingMetadataProvider());
 
-            options.OutputFormatters.Add(new XmlDataContractSerializerOutputFormatter());
+            options.OutputFormatters.Add(new XmlDataContractSerializerOutputFormatter(_loggerFactory));
             options.InputFormatters.Add(new XmlDataContractSerializerInputFormatter(options));
 
             options.ModelMetadataDetailsProviders.Add(new SuppressChildValidationMetadataProvider("System.Xml.Linq.XObject"));

src/Microsoft.AspNetCore.Mvc.Formatters.Xml/Internal/MvcXmlSerializerMvcOptionsSetup.cs

@@ -1,6 +1,8 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
@@ -11,13 +13,29 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
     /// </summary>
     public class MvcXmlSerializerMvcOptionsSetup : IConfigureOptions<MvcOptions>
     {
+        private readonly ILoggerFactory _loggerFactory;
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="MvcXmlSerializerMvcOptionsSetup"/>.
+        /// </summary>
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        public MvcXmlSerializerMvcOptionsSetup(ILoggerFactory loggerFactory)
+        {
+            if (loggerFactory == null)
+            {
+                throw new ArgumentNullException(nameof(loggerFactory));
+            }
+
+            _loggerFactory = loggerFactory;
+        }
+
         /// <summary>
         /// Adds the XML serializer formatters to <see cref="MvcOptions"/>.
         /// </summary>
         /// <param name="options">The <see cref="MvcOptions"/>.</param>
         public void Configure(MvcOptions options)
         {
-            options.OutputFormatters.Add(new XmlSerializerOutputFormatter());
+            options.OutputFormatters.Add(new XmlSerializerOutputFormatter(_loggerFactory));
             options.InputFormatters.Add(new XmlSerializerInputFormatter(options));
         }
     }

src/Microsoft.AspNetCore.Mvc.Formatters.Xml/LoggerExtensions.cs

@@ -0,0 +1,37 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal
+{
+    public static class LoggerExtensions
+    {
+        private static readonly Action<ILogger, string, Exception> _failedToCreateXmlSerializer;
+        private static readonly Action<ILogger, string, Exception> _failedToCreateDataContractSerializer;
+
+        static LoggerExtensions()
+        {
+            _failedToCreateXmlSerializer = LoggerMessage.Define<string>(
+                LogLevel.Warning,
+                1,
+                "An error occurred while trying to create an XmlSerializer for the type '{Type}'.");
+
+            _failedToCreateDataContractSerializer = LoggerMessage.Define<string>(
+                LogLevel.Warning,
+                2,
+                "An error occurred while trying to create a DataContractSerializer for the type '{Type}'.");
+        }
+
+        public static void FailedToCreateXmlSerializer(this ILogger logger, string typeName, Exception exception)
+        {
+            _failedToCreateXmlSerializer(logger, typeName, exception);
+        }
+        
+        public static void FailedToCreateDataContractSerializer(this ILogger logger, string typeName, Exception exception)
+        {
+            _failedToCreateDataContractSerializer(logger, typeName, exception);
+        }
+    }
+}

src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlDataContractSerializerOutputFormatter.cs

@@ -11,6 +11,7 @@ using System.Threading.Tasks;
 using System.Xml;
 using Microsoft.AspNetCore.Mvc.Formatters.Xml;
 using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Mvc.Formatters
 {
@@ -21,22 +22,43 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
     public class XmlDataContractSerializerOutputFormatter : TextOutputFormatter
     {
         private readonly ConcurrentDictionary<Type, object> _serializerCache = new ConcurrentDictionary<Type, object>();
+        private readonly ILogger _logger;
         private DataContractSerializerSettings _serializerSettings;
 
         /// <summary>
         /// Initializes a new instance of <see cref="XmlDataContractSerializerOutputFormatter"/>
-        /// with default XmlWriterSettings
+        /// with default <see cref="XmlWriterSettings"/>.
         /// </summary>
-        public XmlDataContractSerializerOutputFormatter() :
+        public XmlDataContractSerializerOutputFormatter()
-            this(FormattingUtilities.GetDefaultXmlWriterSettings())
+            : this(FormattingUtilities.GetDefaultXmlWriterSettings())
         {
         }
 
         /// <summary>
         /// Initializes a new instance of <see cref="XmlDataContractSerializerOutputFormatter"/>
+        /// with default <see cref="XmlWriterSettings"/>.
+        /// </summary>
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        public XmlDataContractSerializerOutputFormatter(ILoggerFactory loggerFactory)
+            : this(FormattingUtilities.GetDefaultXmlWriterSettings(), loggerFactory)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="XmlDataContractSerializerOutputFormatter"/>.
         /// </summary>
         /// <param name="writerSettings">The settings to be used by the <see cref="DataContractSerializer"/>.</param>
         public XmlDataContractSerializerOutputFormatter(XmlWriterSettings writerSettings)
+            : this(writerSettings, loggerFactory: null)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="XmlDataContractSerializerOutputFormatter"/>.
+        /// </summary>
+        /// <param name="writerSettings">The settings to be used by the <see cref="DataContractSerializer"/>.</param>
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        public XmlDataContractSerializerOutputFormatter(XmlWriterSettings writerSettings, ILoggerFactory loggerFactory)
         {
             if (writerSettings == null)
             {
@@ -57,6 +79,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
             WrapperProviderFactories = new List<IWrapperProviderFactory>();
             WrapperProviderFactories.Add(new EnumerableWrapperProviderFactory(WrapperProviderFactories));
             WrapperProviderFactories.Add(new SerializableErrorWrapperProviderFactory());
+
+            _logger = loggerFactory?.CreateLogger(GetType());
         }
 
         /// <summary>
@@ -138,8 +162,10 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
                 // If the serializer does not support this type it will throw an exception.
                 return new DataContractSerializer(type, _serializerSettings);
             }
-            catch (Exception)
+            catch (Exception ex)
             {
+                _logger?.FailedToCreateDataContractSerializer(type.FullName, ex);
+
                 // We do not surface the caught exception because if CanWriteResult returns
                 // false, then this Formatter is not picked up at all.
                 return null;

src/Microsoft.AspNetCore.Mvc.Formatters.Xml/XmlSerializerOutputFormatter.cs

@@ -11,6 +11,7 @@ using System.Xml;
 using System.Xml.Serialization;
 using Microsoft.AspNetCore.Mvc.Formatters.Xml;
 using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Mvc.Formatters
 {
@@ -21,13 +22,33 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
     public class XmlSerializerOutputFormatter : TextOutputFormatter
     {
         private readonly ConcurrentDictionary<Type, object> _serializerCache = new ConcurrentDictionary<Type, object>();
+        private readonly ILogger _logger;
 
         /// <summary>
         /// Initializes a new instance of <see cref="XmlSerializerOutputFormatter"/>
-        /// with default XmlWriterSettings.
+        /// with default <see cref="XmlWriterSettings"/>.
         /// </summary>
-        public XmlSerializerOutputFormatter() :
+        public XmlSerializerOutputFormatter()
-            this(FormattingUtilities.GetDefaultXmlWriterSettings())
+            : this(FormattingUtilities.GetDefaultXmlWriterSettings())
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="XmlSerializerOutputFormatter"/>
+        /// with default <see cref="XmlWriterSettings"/>.
+        /// </summary>
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        public XmlSerializerOutputFormatter(ILoggerFactory loggerFactory)
+            : this(FormattingUtilities.GetDefaultXmlWriterSettings(), loggerFactory)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="XmlSerializerOutputFormatter"/>.
+        /// </summary>
+        /// <param name="writerSettings">The settings to be used by the <see cref="XmlSerializer"/>.</param>
+        public XmlSerializerOutputFormatter(XmlWriterSettings writerSettings)
+            : this(writerSettings, loggerFactory: null)
         {
         }
 
@@ -35,7 +56,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
         /// Initializes a new instance of <see cref="XmlSerializerOutputFormatter"/>
         /// </summary>
         /// <param name="writerSettings">The settings to be used by the <see cref="XmlSerializer"/>.</param>
-        public XmlSerializerOutputFormatter(XmlWriterSettings writerSettings)
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        public XmlSerializerOutputFormatter(XmlWriterSettings writerSettings, ILoggerFactory loggerFactory)
         {
             if (writerSettings == null)
             {
@@ -54,6 +76,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
             WrapperProviderFactories = new List<IWrapperProviderFactory>();
             WrapperProviderFactories.Add(new EnumerableWrapperProviderFactory(WrapperProviderFactories));
             WrapperProviderFactories.Add(new SerializableErrorWrapperProviderFactory());
+
+            _logger = loggerFactory?.CreateLogger(GetType());
         }
 
         /// <summary>
@@ -114,8 +138,10 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
                 // If the serializer does not support this type it will throw an exception.
                 return new XmlSerializer(type);
             }
-            catch (Exception)
+            catch (Exception ex)
             {
+                _logger?.FailedToCreateXmlSerializer(type.FullName, ex);
+
                 // We do not surface the caught exception because if CanWriteResult returns
                 // false, then this Formatter is not picked up at all.
                 return null;

test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test.csproj

@@ -8,6 +8,7 @@
     <ProjectReference Include="..\Microsoft.AspNetCore.Mvc.TestCommon\Microsoft.AspNetCore.Mvc.TestCommon.csproj" />
 
     <PackageReference Include="Microsoft.AspNetCore.Http" Version="$(MicrosoftAspNetCoreHttpPackageVersion)" />
+    <PackageReference Include="Microsoft.Extensions.Logging.Testing" Version="$(MicrosoftExtensionsLoggingTestingPackageVersion)" />
   </ItemGroup>
 
 </Project>

test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlDataContractSerializerOutputFormatterTest.cs

@@ -11,6 +11,8 @@ using System.Xml;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal;
 using Microsoft.AspNetCore.Testing.xunit;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Testing;
 using Microsoft.Extensions.Primitives;
 using Microsoft.Net.Http.Headers;
 using Moq;
@@ -626,6 +628,60 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml
             XmlAssert.Equal(expectedOutput, content);
         }
 
+        public static TheoryData<XmlDataContractSerializerOutputFormatter, TestSink> LogsWhenUnableToCreateSerializerForTypeData
+        {
+            get
+            {
+                var sink1 = new TestSink();
+                var formatter1 = new XmlDataContractSerializerOutputFormatter(new TestLoggerFactory(sink1, enabled: true));
+
+                var sink2 = new TestSink();
+                var formatter2 = new XmlDataContractSerializerOutputFormatter(
+                    new XmlWriterSettings(),
+                    new TestLoggerFactory(sink2, enabled: true));
+
+                return new TheoryData<XmlDataContractSerializerOutputFormatter, TestSink>()
+                {
+                    { formatter1, sink1 },
+                    { formatter2, sink2}
+                };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(LogsWhenUnableToCreateSerializerForTypeData))]
+        public void CannotCreateSerializer_LogsWarning(
+            XmlDataContractSerializerOutputFormatter formatter,
+            TestSink sink)
+        {
+            // Arrange
+            var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer));
+
+            // Act
+            var result = formatter.CanWriteResult(outputFormatterContext);
+
+            // Assert
+            Assert.False(result);
+            var write = Assert.Single(sink.Writes);
+            Assert.Equal(LogLevel.Warning, write.LogLevel);
+            Assert.Equal($"An error occurred while trying to create a DataContractSerializer for the type '{typeof(Customer).FullName}'.",
+                write.State.ToString());
+        }
+
+        [Fact]
+        public void DoesNotThrow_OnNoLoggerAnd_WhenUnableToCreateSerializerForType()
+        {
+            // Arrange
+            var formatter = new XmlDataContractSerializerOutputFormatter(); // no logger is being supplied here on purpose
+            var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer));
+
+            // Act
+            var canWriteResult = formatter.CanWriteResult(outputFormatterContext);
+
+            // Assert
+            Assert.False(canWriteResult);
+        }
+
         private OutputFormatterWriteContext GetOutputFormatterContext(
             object outputValue,
             Type outputType,
@@ -666,5 +722,14 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml
                 return base.CreateSerializer(type);
             }
         }
+
+        public class Customer
+        {
+            public Customer(int id)
+            {
+            }
+
+            public int MyProperty { get; set; }
+        }
     }
 }

test/Microsoft.AspNetCore.Mvc.Formatters.Xml.Test/XmlSerializerOutputFormatterTest.cs

@@ -7,9 +7,12 @@ using System.IO;
 using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
+using System.Xml;
 using System.Xml.Serialization;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.Formatters.Xml.Internal;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Testing;
 using Microsoft.Extensions.Primitives;
 using Microsoft.Net.Http.Headers;
 using Moq;
@@ -330,7 +333,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml
         {
             // Arrange
             var formatter = new XmlSerializerOutputFormatter();
-            var outputFormatterContext = GetOutputFormatterContext(new object(), typeof (object));
+            var outputFormatterContext = GetOutputFormatterContext(new object(), typeof(object));
             outputFormatterContext.ContentType = new StringSegment(mediaType);
             outputFormatterContext.ContentTypeIsServerDefined = isServerDefined;
 
@@ -389,6 +392,61 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml
             }
         }
 
+        public static TheoryData<XmlSerializerOutputFormatter, TestSink> LogsWhenUnableToCreateSerializerForTypeData
+        {
+            get
+            {
+                var sink1 = new TestSink();
+                var formatter1 = new XmlSerializerOutputFormatter(new TestLoggerFactory(sink1, enabled: true));
+
+                var sink2 = new TestSink();
+                var formatter2 = new XmlSerializerOutputFormatter(
+                    new XmlWriterSettings(),
+                    new TestLoggerFactory(sink2, enabled: true));
+
+                return new TheoryData<XmlSerializerOutputFormatter, TestSink>()
+                {
+                    { formatter1, sink1 },
+                    { formatter2, sink2}
+                };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(LogsWhenUnableToCreateSerializerForTypeData))]
+        public void XmlSerializer_LogsWhenUnableToCreateSerializerForType(
+            XmlSerializerOutputFormatter formatter,
+            TestSink sink)
+        {
+            // Arrange
+            var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer));
+
+            // Act
+            var canWriteResult = formatter.CanWriteResult(outputFormatterContext);
+
+            // Assert
+            Assert.False(canWriteResult);
+            var write = Assert.Single(sink.Writes);
+            Assert.Equal(LogLevel.Warning, write.LogLevel);
+            Assert.Equal(
+                $"An error occurred while trying to create an XmlSerializer for the type '{typeof(Customer).FullName}'.",
+                write.State.ToString());
+        }
+
+        [Fact]
+        public void XmlSerializer_DoesNotThrow_OnNoLoggerAnd_WhenUnableToCreateSerializerForType()
+        {
+            // Arrange
+            var formatter = new XmlSerializerOutputFormatter(); // no logger is being supplied here on purpose
+            var outputFormatterContext = GetOutputFormatterContext(new Customer(10), typeof(Customer));
+
+            // Act
+            var canWriteResult = formatter.CanWriteResult(outputFormatterContext);
+
+            // Assert
+            Assert.False(canWriteResult);
+        }
+
         private OutputFormatterWriteContext GetOutputFormatterContext(
             object outputValue,
             Type outputType,
@@ -429,5 +487,14 @@ namespace Microsoft.AspNetCore.Mvc.Formatters.Xml
                 return base.CreateSerializer(type);
             }
         }
+
+        public class Customer
+        {
+            public Customer(int id)
+            {
+            }
+
+            public int MyProperty { get; set; }
+        }
     }
 }