Logging a null value

[veccsolutions]

When calling

Logger.LogError("Unable to resolve token: <{0}>", token)

and token is null string, I get an argument null exception.

I would have expected it to have logged "Unable to resolve token: <>"

Here is the stack trace:

Parameter name: args
   at System.String.Format(IFormatProvider provider, String format, Object[] args)
   at Microsoft.Extensions.Logging.Internal.LogValuesFormatter.Format(Object[] values)
   at Microsoft.Extensions.Logging.Internal.FormattedLogValues.ToString()
   at Microsoft.Extensions.Logging.LoggerExtensions.MessageFormatter(Object state, Exception error)
   at Microsoft.Extensions.Logging.Console.ConsoleLogger.Log(LogLevel logLevel, Int32 eventId, Object state, Exception exception, Func`3 formatter)
   at Microsoft.Extensions.Logging.Logger.Log(LogLevel logLevel, Int32 eventId, Object state, Exception exception, Func`3 formatter)
   --- End of inner exception stack trace ---
   at Microsoft.Extensions.Logging.Logger.Log(LogLevel logLevel, Int32 eventId, Object state, Exception exception, Func`3 formatter)
   at Microsoft.Extensions.Logging.LoggerExtensions.LogError(ILogger logger, String format, Object[] args)
   at Macu.Raptor.MemberSite.Services.Middleware.SsoAuthenticationHandler.HandleAuthenticateAsync() in C:\Development\Code\Macu.Raptor\src\Macu.Raptor.MemberSite\Services\Middleware\SsoAuthenticationHandler.cs:line 31
   at Microsoft.AspNet.Authentication.AuthenticationHandler`1.HandleAuthenticateOnceAsync()
   at Microsoft.AspNet.Authentication.AuthenticationHandler`1.<InitializeAsync>d__48.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at Microsoft.AspNet.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at Microsoft.AspNet.IISPlatformHandler.IISPlatformHandlerMiddleware.<Invoke>d__8.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at Microsoft.AspNet.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__7.MoveNext()

[veccsolutions]

Oh, one possible easy fix,
In Microsoft.Extensions.Logging.Internal.LogValuesFormatter:Format method, change the last 3 lines from

            }

            return string.Format(CultureInfo.InvariantCulture, _format, values);

to

                return string.Format(CultureInfo.InvariantCulture, _format, values);
            }

            return string.Format(CultureInfo.InvariantCulture, _format, (object)null);

[muratg]

@BrennanConroy Could you investigate?

[muratg]

@veccsolutions What's the type of token? Is it a string, or an object[]?

[veccsolutions]

@muratg token is a string value.

[fengjb]

string token = null;
_logger.LogInformation("Unable to resolve token: <{0}>", token);

[niemyjski]

I noticed that even doing logger.Info(null) blows up. I don't think a logger should ever throw an exception..

[Eilon]

I'd rather say that the logging infrastructure should never throw an exception. But loggers themselves - well, if they have major errors (or simply a bug), they might throw, and the logging infrastructure will let it bubble up. We don't want the logging infrastructure to hide bugs in loggers.

[muratg]

Agreed with @Eilon. This bug is about finding out if the abstractions are at fault or specific implementations.

[cesarblum]

@muratg Our loggers should still be resilient to this though.

[cesarblum]

I'll close this in favor of #422.