No exceptions emitted when generated by faulty audit log configuration

[markembling]

Combining this with an audit log Serilog configuration, exceptions are swallowed and never emitted back to the caller as would be expected.

Steps to reproduce:

1. Set up an audit logging configuration for Serilog which is purposefully mal-configured (for example, using the MS SQL Server sink to log events to a non-existent database table).
2. Implement logging using Microsoft.Extensions.Logging.Abstractions and an ILogger using this library.
3. Emit a log event (logger.LogInformation("test");)

Expected behaviour: Exception to be thrown at the call to LogInformation().

Actual behaviour:

• No exception is thrown
• The underlying exception is visible in Serilog's SelfLog

It looks like the culprit of this behaviour is here:

serilog-extensions-logging/src/Serilog.Extensions.Logging/Extensions/Logging/SerilogLogger.cs

Lines 67 to 74 in 7141ae2

	try
	{
	Write(level, eventId, state, exception, formatter);
	}
	catch (Exception ex)
	{
	SelfLog.WriteLine($"Failed to write event through {typeof(SerilogLogger).Name}: {ex}");
	}

Apologies for posting this up as an issue with no accompanying pull request but that's somewhat down to time constraints right now and largely because I'm not sure what the implications of changing this are likely to be. The obvious fix for this specific problem as I see it would just be to get rid of the try/catch block entirely and assume that if an exception is encountered, it's something we care about. However I am aware that my scenario is probably quite specific (it seems like audit logging is far less common than regular logging where errors should be ignored) and my proposed 'fix' might well create new problems for people in other scenarios. And/or not account for other possible causes of exceptions which don't relate to the underlying Serilog logger having emitted them.

Let me know if I can add anything more to clarify and/or if I'm barking up the wrong tree here with this.

[nblumhardt]

Thanks for the report, Mark!

Here's the original bug that led to the introduction of the try/catch block:

#164

I'm not sure what the right approach is, needs some more consideration 🤔

[markembling]

Thanks for the reply, I appreciate the context here.

I'm not really sure what to suggest either. Reading through that issue, I'm not sure I agree with the premise of that fix: I think if MEL would throw, also throwing in a pretty consistent manner would be fine (the reason being that the caller doesn't know it's not dealing with MEL anyway) and that is a MEL behavioural trait rather than Serilog. Obviously the side effect of that would be we'd automatically let through any exceptions including the ones we'd want in this situation. But equally I can see that for others, the opposite would definitely be true and that you could easily argue that Serilog's more tolerant approach should the the one which prevails.

Regarding this issue though, for now I'm having to take a completely alternate (and frankly, worse) approach with my logging as this is a showstopper (I absolutely need the audit-style "loudly failing" behaviour to come through) but would love to come back and put it back to how it should be if/when there's a solution for this.

I'll be back if I have any ideas that might be useful here... 👍

[nblumhardt]

Thanks for the follow-up, Mark.

On the paths where you need auditing, perhaps using Serilog's ILogger/Log class directly is the way to go? Cheers!

[sungam3r]

On the paths where you need auditing, perhaps using Serilog's ILogger/Log class directly is the way to go? Cheers!

@nblumhardt That's not always possible and often contradicts design. #218 fixes issue with a very little code.