`Console.WriteLine` within library code dumps Exception to stdout with no elegant override

[sandreas]

The problem

Instantiating Track with a bogus file spits out Exception messages directly to stdout without possibility to prevent / override this behaviour.

Environment

4.9.0 on Ubuntu 18.04 LTS

Details

See tone issue: https://github.com/sandreas/tone/issues/26

The problem is, that as soon as Track is instantiated, the problem occurs and later when setting properties it occurs again. So I think that there are multiple pieces of code that use Console.WriteLine:

• https://github.com/Zeugma440/atldotnet/blob/9c5f8dd82e2efb34dceb55cf24258a05d7a60a7f/ATL/AudioData/AudioDataManager.cs#L339
• https://github.com/Zeugma440/atldotnet/blob/6bd86e00d584b89c3199bcaca19ce87165aaae59/ATL/Playlist/PlaylistIO.cs#L99
• https://github.com/Zeugma440/atldotnet/blob/6bd86e00d584b89c3199bcaca19ce87165aaae59/ATL.test/IO/MetaData/MP4.cs#L548

Current workaround is to use a custom TextWriter for Console:

public class DiscardTextWriter: TextWriter
{
    public override void Write(char value)
    {
    }

    public override void Write(string? _)
    {
    }

    public override Encoding Encoding => Encoding.ASCII;
}

var old = Console.Out;
Console.SetOut(new DiscardTextWriter());
var track = new Track("my-file.m4b");
// ...
Console.SetOut(old);

[Zeugma440]

Thanks for the feedback.

I've just created a new switch : Settings.OutputStacktracesToConsole which defaults to true

Regardless of this setting, stacktraces are aways logged to ATL's logging system

Is that OK to you ?

[Zeugma440]

By the way

https://github.com/Zeugma440/atldotnet/blob/6bd86e00d584b89c3199bcaca19ce87165aaae59/ATL.test/IO/MetaData/MP4.cs#L548

^ that one's in the test project, not in the library. I'll do what I fancy there 😉

[sandreas]

First of all: Thank you for your quick feedback.

^ that one's in the test project, not in the library. I'll do what I fancy there wink

Oh, I did not see the test there :-)

I've just created a new switch : Settings.OutputStacktracesToConsole which defaults to true

Well, although this would solve my problem (kind of), I'm not sure, if this is the right way... Should a library really report errors via Console.Log? This makes it quite difficult to handle these errors. I would prefer maybe something like this:

// default
Settings.ExceptionHandler = (e, _) => {
    Console.WriteLine(e.Message);
    Console.WriteLine(e.StackTrace);
    if (e.InnerException != null)
    {
        Console.WriteLine("Inner Exception BEGIN");
        Console.WriteLine(e.InnerException.Message);
        Console.WriteLine(e.InnerException.StackTrace);
        Console.WriteLine("Inner Exception END");
    }
};

Settings.ExceptionHandler = null; // disable exception reporting

Settings.ExceptionHandler = (e, t) => {

    // do some custom stuff
}

try {
 // ...
} catch(Exception e) {
    Settings.ExceptionHandler?.Invoke(e, typeof(PlaylistIO));
}

Wouldn't this be more flexible?

[Zeugma440]

This makes it quite difficult to handle these errors

If you really do mean "handle", then the stakes are very different than mere logging output.

Is your core problem the fact that stdout/stderr are polluted by stuff that shouldn't appear there, or are you trying to react to these exceptions inside your app ?

If we're in the latter, it's an API issue, not a logging issue.

[sandreas]

Is your core problem the fact that stdout/stderr are polluted by stuff that shouldn't appear there, or are you trying to react to these exceptions inside your app ?

Well, kind of both. My main problem is that stdout is polluted (which would be fixed with the settings flag), but silently ignoring metadata reading errors may lead to other issues (e.g. tone is not reading tags properly). My best case scenario is errors going to an optional logfile while not polluting stdout.

Would that be possible with the flag and the current API?

[Zeugma440]

My main problem is that stdout is polluted (which would be fixed with the settings flag). My best case scenario is errors going to an optional logfile while not polluting stdout. Would that be possible with the flag and the current API?

Yup. You can use the ATL logging system to capture all log output and flush it to a file.

You might find inspiration in those two custom implementations located in the test project :

• ArrayLogger which logs all messages to an in-memory array
• ConsoleLogger which logs all messages to the console

If you were to write FileLogger, its DoLog method would append one line to the file. You may also leverage asynchronous logging to reduce I/O stress and only write to the file once the whole processing is done.

silently ignoring metadata reading errors may lead to other issues (e.g. tone is not reading tags properly)

That's a concern, given the current API not returning any feedback when reading fails.

I'd gladly add an alternate method like bool Track.Load(). Would that be fine to you ?

[sandreas]

I'd gladly add an alternate method like bool Track.Load(). Would that be fine to you ?

I think now that I understood your logging approach, the Settings.OutputStacktracesToConsole flag is fine for now. That way I can ignore the errors by default using Settings.OutputStacktracesToConsole=false, but put a tone --debug dump my-file.m4b where I set this to true on --debug mode.

[Zeugma440]

Fine. I'll release that as soon as I get feedback on other pending issues.

[sandreas]

Fine. I'll release that as soon as I get feedback on other pending issues.

Yeah sure, I'm not in a hurry :-) Thank you.

[Zeugma440]

Released in today's v4.11

[sandreas]

Awesome, thank you.