Skip to content
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

Exception render throws an error message with multi threaded logging #25

Closed
duggaraju opened this issue Jul 10, 2023 · 6 comments
Closed

Comments

@duggaraju
Copy link

The OptionsCollection is not thread safe.

Unhandled exception: System.AggregateException: An error occurred while writing to logger(s). (An item with the same key has already been added. Key: Vertical.SpectreLogger.Rendering.ExceptionRenderer+Options)
---> System.ArgumentException: An item with the same key has already been added. Key: Vertical.SpectreLogger.Rendering.ExceptionRenderer+Options
at System.Collections.Generic.Dictionary2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) at System.Collections.Generic.Dictionary2.Add(TKey key, TValue value)
at Vertical.SpectreLogger.Options.OptionsCollection.GetOptionsTOptions
at Vertical.SpectreLogger.Rendering.ExceptionRenderer.Render(IWriteBuffer buffer, LogEventContext& context)
at Vertical.SpectreLogger.Rendering.ExceptionRenderer.Vertical.SpectreLogger.Core.ITemplateRenderer.Render(IWriteBuffer buffer, LogEventContext& context)
at Vertical.SpectreLogger.Rendering.RendererPipeline.Render(LogEventContext& logEventContext)
at Vertical.SpectreLogger.Rendering.RendererPipeline.Vertical.SpectreLogger.Core.IRendererPipeline.Render(LogEventContext& logEventContext)
at Vertical.SpectreLogger.SpectreLogger.Log[TState](LogLevel logLevel, EventId eventId, TState state, Exception exception, Func3 formatter) at Microsoft.Extensions.Logging.Logger.<Log>g__LoggerLog|13_0[TState](LogLevel logLevel, EventId eventId, ILogger logger, Exception exception, Func3 formatter, List1& exceptions, TState& state) --- End of inner exception stack trace --- at Microsoft.Extensions.Logging.Logger.ThrowLoggingError(List1 exceptions)
at Microsoft.Extensions.Logging.Logger.Log[TState](LogLevel logLevel, EventId eventId, TState state, Exception exception, Func3 formatter) at Microsoft.Extensions.Logging.Logger1.Microsoft.Extensions.Logging.ILogger.Log[TState](LogLevel logLevel, EventId eventId, TState state, Exception exception, Func`3 formatter)
at Microsoft.Extensions.Logging.LoggerExtensions.Log(ILogger logger, LogLevel logLevel, EventId eventId, Exception exception, String message, Object[] args)
at Microsoft.Extensions.Logging.LoggerExtensions.Log(ILogger logger, LogLevel logLevel, Exception exception, String message, Object[] args)
at Microsoft.Extensions.Logging.LoggerExtensions.LogError(ILogger logger, Exception exception, String message, Object[] args)

@daningalla
Copy link
Contributor

Thank you for reporting - I'll take a look right away.

@duggaraju
Copy link
Author

duggaraju commented Jul 10, 2023

Thanks. Found another stack trace. Not sure if related threading issue or something different.
Unhandled exception. System.AggregateException: An error occurred while writing to logger(s). (Object reference not set to an instance of an object.)
---> System.NullReferenceException: Object reference not set to an instance of an object.
at Vertical.SpectreLogger.Templates.TemplateSegment.get_CompositeFormatSpan()
at Vertical.SpectreLogger.Output.WriteBufferExtensions.WriteLogValue[T](IWriteBuffer buffer, LogLevelProfile profile, TemplateSegment templateSegment, T value, Action1 writer) at Vertical.SpectreLogger.Rendering.RendererPipeline.Render(LogEventContext& logEventContext) at Vertical.SpectreLogger.SpectreLogger.Log[TState](LogLevel logLevel, EventId eventId, TState state, Exception exception, Func3 formatter)
at Microsoft.Extensions.Logging.Logger.g__LoggerLog|13_0[TState](LogLevel logLevel, EventId eventId, ILogger logger, Exception exception, Func3 formatter, List1& exceptions, TState& state)
--- End of inner exception stack trace ---
at Microsoft.Extensions.Logging.Logger.ThrowLoggingError(List1 exceptions) at Microsoft.Extensions.Logging.Logger.Log[TState](LogLevel logLevel, EventId eventId, TState state, Exception exception, Func3 formatter)
at Microsoft.Extensions.Logging.Logger1.Microsoft.Extensions.Logging.ILogger.Log[TState](LogLevel logLevel, EventId eventId, TState state, Exception exception, Func3 formatter)
at Microsoft.Extensions.Logging.LoggerExtensions.Log(ILogger logger, LogLevel logLevel, EventId eventId, Exception exception, String message, Object[] args)
at Microsoft.Extensions.Logging.LoggerExtensions.Log(ILogger logger, LogLevel logLevel, String message, Object[] args)

@daningalla
Copy link
Contributor

Yes I ran into that as well while writing a driver to repro the original issue. It may be related, but I'm going to create a separate issue for it.

@duggaraju
Copy link
Author

Thanks. For the first issue I found a workaround by explicitly setting the options

            builder
                .SetMinimumLevel(LogLevel.Trace)
                .AddSpectreConsole(builder =>
                    builder
                    .ConfigureProfiles(profile =>
                    {
                        profile
                            .ConfigureOptions<ExceptionRenderer.Options>(options => {})
                            .ConfigureOptions<DateTimeRenderer.Options>(options => {});
                    }))

This will create a default options object inside the dictionary and avoids the multi threading issue later. but this is just workaround. Is there a workaround for the CompositeFormat? where i can configure something to avoid it?

@duggaraju
Copy link
Author

Also in your PR I did not see the actual change to CollectionOptions.cs file

daningalla added a commit that referenced this issue Jul 10, 2023
@daningalla
Copy link
Contributor

Yeah I left out the most important file in the commit 😐. It is committed now and I'll push up a pre-release build to NuGet. As far as the other issue, I'm having issues with a repro. Snippets of your current usage may provide some additional insight, because I can't see how a NRE is even being thrown there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants