WriteError in FluentMigrator

[pergardebrink]

I was looking at the issue #1252 and tried to solve it by removing some of the Console.WriteLine statements (#1253).

However, it seems as when we log an error with an exception (using the LogError extension method from Microsoft.Extensions.Logging), the string message never reaches the logs, just the exception (

fluentmigrator/src/FluentMigrator.Runner.Core/Logging/FluentMigratorLogger.cs

Line 48 in e82aafa

WriteError(exception);

).

This seems to not be the way you would expect.

If the code is indeed incorrect in FluentMgiratorLogger, I can create a PR fixing this, but wanted to know if there are legacy reasons why this could be the expected behavior unless we fix other parts of the code base?

[jzabroski]

So, basically, the else clause should always execute? Or should we do something different if we have an exception + a message, like combine it into a single log statement? I kind of favor the latter approach, but I'm open to alternative.

Agree this doesnt seem desireable. I would also probably structure it such that the string message comes first, then the exception.

[jzabroski]

@pergardebrink Just a nudge - what behavior do you think it should be?

[pergardebrink]

Sorry for not responding. I haven't really digged into the details on the logging setup in FluentMigrator yet and how it's supposed to work as I'm quite new to the project still. There are a few custom loggers and I guess some parts are to adapt and reimplement how the "announcers" work/worked before?.

But yes, I would expect that a call to WriteError with a message and exception should produce one log statement containing the message first and the exception after, as you say, effectively meaning that we would probably remove the if/else statement and rewrite WriteError to also accept an exception along with the message and print it?

We could of course format the message to include the exception when calling WriteError(string) here, but I think this behaviour is something that would make sense everywhere when you get passed a message and an exception...

[jzabroski]

effectively meaning that we would probably remove the if/else statement and rewrite WriteError to also accept an exception along with the message and print it?

Yes, i think that is the most desirable. The only reason I would see for the other approach is it lets the caller decide.

Let me know if you'd like to do a PR to fix this.

As far as the announcers, I believe you saw my comments from the security/password thread where I explained the evolution of the logging API. I kind of like what Jon Sequitur's Its.Log framework does: https://github.com/jonsequitur/Its.Log - its not a logging framework but its designed to forward messages to a logging framework. The downside to his approach is in a highly concurrent application it can create delays due to additional buffering from log caller to log writer, and so a crashed application may not log the crash. However, I believe if you're application is crashing regularly, you need a crash dump and/or crash dump monitor.

[jzabroski]

@pergardebrink Any interest in submitting a PR? Thank you for your prior contributions.