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

Two-stage initialization with exception during UseSerilog is not logged to files #75

Open
cremor opened this issue Aug 4, 2023 · 8 comments

Comments

@cremor
Copy link

cremor commented Aug 4, 2023

Example:

Log.Logger = new LoggerConfiguration()
    .WriteTo.Debug()
    .WriteTo.Console()
    .WriteTo.File(BootstrapLogFilePath, LogEventLevel.Error)
    .CreateBootstrapLogger();
	
var builder = WebApplication.CreateBuilder(args);
	
builder.Host.UseSerilog((context, services, loggerConfiguration) => loggerConfiguration
    .ReadFrom.Configuration(context.Configuration)
    .ReadFrom.Services(services));

If an exception happens in the configureLogger action passed to UseSerilog (e.g. because the configuration in appsettings.json is invalid) then the resulting exception will be logged in the console and debug output, but it will not be logged in the file.

The Serilog SelfLog logs a related exception: "Caught exception while emitting to sink Serilog.Core.Sinks.RestrictedSink: System.ObjectDisposedException: Cannot write to a closed TextWriter."
It looks like the bootstrap logger is already disposed, but the new logger doesn't exist yet. So the exception can't be logged to any file.

Would it be possible to only dispose the bootstrap logger once the new logger is fully loaded? Then the exception would be logged to the file sink from the bootstrap logger.

In any case, exceptions that happen during the configureLogger action passed to UseSerilog should be written to the SelfLog.

@nblumhardt
Copy link
Member

It's not possible to wait for the new logger to be fully constructed before disposing the bootstrap logger, as this prevents reopening the same log file in both configurations, for example. Logging to SelfLog seems like a good first step to improving this, though.

@cremor
Copy link
Author

cremor commented Aug 5, 2023

It's not possible to wait for the new logger to be fully constructed before disposing the bootstrap logger, as this prevents reopening the same log file in both configurations, for example.

I see. Would it be possible to recreate the bootstrap logger configuration when an exception happens?

Logging to SelfLog seems like a good first step to improving this, though.

Thanks!

One additional idea:
Depending on what exactly fails during logging configuration the exception message could be not helpful at all and the stack trace could be complicated. Maybe it would be a good idea to wrap those exceptions with something like "Serilog logger configuration failed, see inner exception for details."?

@nblumhardt
Copy link
Member

Unfortunately the bootstrap logger is configured outside of the code callable by this package, so it can't be re-created (otherwise it'd be a reasonable way to go).

SelfLog supports arbitrary string arguments, so the message could include the text you mention, e.g.:

SelfLog.Write($"Logger reconfiguration failed: {ex}");

(It's not necessary to mention that the message is from Serilog, since it's Serilog's internal log - nothing else writes to it.)

@cremor
Copy link
Author

cremor commented Aug 9, 2023

The wrapper exception idea was for the exception that is actually thrown - not for the self log.

@nblumhardt
Copy link
Member

Ah, sorry, now I understand what you mean 👍

@nblumhardt
Copy link
Member

Closing as I don't think there's anything to action in this one, currently. Let me know if I'm mistaken. Thanks!

@cremor
Copy link
Author

cremor commented Nov 27, 2024

@nblumhardt You said that the main ask of my issue - keeping the bootstrap logger open - is not possible. But what about the other points mentioned?

A) In any case, exceptions that happen during the configureLogger action passed to UseSerilog should be written to the SelfLog.
You confirmed that:

Logging to SelfLog seems like a good first step to improving this, though.

B) Maybe it would be a good idea to wrap those exceptions with something like "Serilog logger configuration failed, see inner exception for details."?
The wrapper exception idea was for the exception that is actually thrown - not for the self log.
Which you also confirmed:

Ah, sorry, now I understand what you mean 👍

@nblumhardt
Copy link
Member

Thanks for checking this over! 👍 Working through a lot of issue triage in a lot of repositories 😅

I'm on board with writing these exceptions to SelfLog; I don't expect to have time to implement this in the near future, so all help appreciated there.

@nblumhardt nblumhardt reopened this Nov 27, 2024
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