qos-ch / slf4j

Simple Logging Facade for Java
http://www.slf4j.org
MIT License
2.32k stars 980 forks source link

Message formatting with last argument being Throwable #390

Open zorba128 opened 7 months ago

zorba128 commented 7 months ago

I really often hit issue with slf4j interpreting my messages not the way I'd like it to. I don't think its reasonable to have two following lines result in different behaviour.

Throwable ex = ...
Item item = ...

log.warn("Processing issue; ex={}, item={}", ex, item);   // this works fine
log.warn("Processing issue; item={}, ex={}", item, ex);   // and this produces "... issue=Issue(123), ex={}"  followed by stacktrace

I think the MessageFormatter should take into account number of placeholders, and try to use last argument as throwable candidate only if not bound.

FAQ says:

If the exception is not the last argument, it will be treated as a plain object and its stack trace will NOT be printed. However, such situations should not occur in practice.

For me this is not true. I quite often want to see just ex.toString, specially where exception is part of business logic. I just want to log its name, description, details - I don't care about stack trace. So it's common to treat exception just as any other part of message.

SLF4J, in accordance with the programmer's most probable intention, will interpret NumberFormatException instance as a throwable instead of an unused Object parameter

And this one actually suggests what should be done. It says about choice between exception and unused object parameter. And code actually does not check if parameter is used - it just picks last argument, resulting in message with last '{}' not filled in, and with unwanted stacktrace appended.