Don't print errors (or anything) inside of classes

[richardolsson]

I was getting a test failure in #31 because LyraConfig was using console.error (indirectly). I didn't realize this before. I think this is something that we should avoid: console logging from within the depths of the program.

If we want to use logging, we should create a logger role (i.e. an interface) that can be used by classes that want to log something, and inject that as a dependency rather than using global functions.

But ideally I don't see a need for classes like LyraConfig to log errors themselves. They should just raise the error, and the user of the class can decide what is the best way to report the error (logging, HTTP, etc).

Besides being IMO just a generally nice principle and separation of concerns, this also means that the code we write now can be reused in other aspects of the future lyra ecosystem, including libraries and (CLI) tooling.

[richardolsson]

I'm curious to hear your thoughts on this @amerharb!

[amerharb]

I'm curious to hear your thoughts on this @amerharb!

yes agree, moving logging to log.ts was a step one to do that, however IMO I do avoid to use these complicated logging packages that need a lot of configuration that is easy to break, but maybe a simple home made interface and implementation until we need something more advance.

[amerharb]

and yeah regarding not to log error from the class as long as it raise the error, yes I think we should remove remove that.
since anyway throwing exception will print the error itself, we could just enhance error msg instead.

[richardolsson]

yes agree, moving logging to log.ts was a step one to do that, however IMO I do avoid to use these complicated logging packages that need a lot of configuration that is easy to break, but maybe a simple home made interface and implementation until we need something more advance.

I agree that we should not add any logging packages. I realize reading my issue back that the following can be misinterpreted and maybe is why you mention logging packages:

[…] the user of the class can decide what is the best way to report the error (logging, HTTP, etc).

I did not mean that a log message should be sent via HTTP to some logging repository. I just meant that errors should be reported to the user of the program, and if the user is on the command-line, logging it makes sense, but if the user is interacting with the program via a web interface and the error is triggered in an API route, the error needs to be included in the HTTP response so that the UI can show a message.

The above is probably so obvious that it goes without saying, but to me that also means that there is no value in logging anything inside a class like LyraConfig, because the user of the class (the programmer, i.e. us) will need to handle the error anyway and present output in a way that makes sense to their user.