You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "Niko-O (via GitHub)" <gi...@apache.org> on 2025/02/03 14:07:05 UTC

[I] Make `logger.error(ex);`, `logger.info(ex);`, etc. print stack trace by default (logging-log4j2)

Niko-O opened a new issue, #3433:
URL: https://github.com/apache/logging-log4j2/issues/3433

   It is too easy to write code like this:
   ```
   try
   {
       doThing();
   }
   catch (ThingFailedException ex)
   {
       logger.error("Doing this thing failed.");
       logger.error("Context ID: " + context.id);
       logger.error("SomeSettings: " + someSetting);
       logger.error(ex);
       throw new WhateverException(ex);
   }
   ```
   Especially if you come back to a project after months and you don't remember anything about the log4j API specifics.
   You think you you log everything you need, but you don't get the exception's stack trace here. You'd have to write `logger.error("", ex);` here, which is extremely unintuitive! Or `logger.catching(ex)`, but that's another special thing I'd need to memorize.
   
   I propose to change the default behavior such that the stack trace is printed in this case. I don't care how it's achieved, e.g. by adding a `void error(Throwable)` overload, or adding a check in the existing `void error(Object)` overload. The point is that it should be the default behavior because that's what's expected.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Make `logger.error(ex);`, `logger.info(ex);`, etc. print stack trace by default [logging-log4j2]

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #3433:
URL: https://github.com/apache/logging-log4j2/issues/3433#issuecomment-2726435795

   > ```
   > try
   > {
   >     doThing();
   > }
   > catch (ThingFailedException ex)
   > {
   >     logger.error(ex);
   >     throw new WhateverException(ex); // Maybe also this
   > }
   > ```
   
   Try:
   
   ```java
   try {
       doThing();
   } catch (ThingFailedException ex) {
       throw logger.throwing(new WhateverException(ex));
   }
   ```
   
   **Note**: this is an anti-pattern (cc/@rgoers), because you don't trust the caller to properly handle the exception, so you log it for auditing purposes. If the caller handles the exception properly, it will appear **twice** in the logs, but if he ignores the exception or only shows it to the user (that later denies it) you can easily prove you didn't hide the error.
   
   I understand the intention, I see it often and sometimes I use it myself (should I trust the frontend dev ❓ ), but it is **not** a best practice.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Make `logger.error(ex);`, `logger.info(ex);`, etc. print stack trace by default [logging-log4j2]

Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on issue #3433:
URL: https://github.com/apache/logging-log4j2/issues/3433#issuecomment-2723889226

   @rgoers @ppkarwasz WDYT about this proposal? Instead of adding one more method to the `Logger`, can't we achieve this by branching on `message instanceof Throwable` in `MessageFactory#newMessage(Object)` implementations?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Make `logger.error(ex);`, `logger.info(ex);`, etc. print stack trace by default [logging-log4j2]

Posted by "Niko-O (via GitHub)" <gi...@apache.org>.
Niko-O commented on issue #3433:
URL: https://github.com/apache/logging-log4j2/issues/3433#issuecomment-2724504351

   > 1. Why isn't your first thought to recommend changing this to
   
   Short answer: Because it's not the same thing.
   Longer answer: I have a few situations where I need to log quite a lot of stuff. Reading through all of that in a single line is painful. It's much easier to read if it's split into multiple lines. "This thing failed. Log this data. Log that data. Log the exception. Continue." is so much less mental burden than "This thing failed. Log this data. Log that data. Log the exception, but oh, wait, I need to add a pointless empty string as an argument here or the stack trace won't actually show up. Continue."
   
   > 2. Assuming for some reason they didn't want the first 3 log lines what would you tell someone who just did
   > logger.error(ex);
   
   Assuming someone **only** wrote this:
   ```
   try
   {
       doThing();
   }
   catch (ThingFailedException ex)
   {
       logger.error(ex);
       throw new WhateverException(ex); // Maybe also this
   }
   ```
   Yes, it's kidna weird that they don't add any additional context, but I guess it works for them ¯\\_(ツ)_/¯. Maybe this is the only place that this specific exception is thrown/caught anyways. Maybe the exception message already contains all the additional context they need. I don't want to dictate how other people debug their own code, I primarily care about my own debugging experience.
   
   > `LOGGER.warn("You need new software engineers");`
   
   That's unreasonable.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Make `logger.error(ex);`, `logger.info(ex);`, etc. print stack trace by default [logging-log4j2]

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #3433:
URL: https://github.com/apache/logging-log4j2/issues/3433#issuecomment-2723930666

   Some thoughts:
   
   - This is a recurring question. I can't find the reference now, but multiple times users asked for `log(Object)` to treat exceptions in a different way.
   - There is a [`Logger.catching`](https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/Logger.html#catching(java.lang.Throwable) method that does exactly what @Niko-O asks.
   
   Given Ralph's remarks, we should probably have an Error Prone rule that issues a warning if a `Throwable` is passed as a logging message and suggests using `catching` instead.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Make `logger.error(ex);`, `logger.info(ex);`, etc. print stack trace by default [logging-log4j2]

Posted by "rgoers (via GitHub)" <gi...@apache.org>.
rgoers commented on issue #3433:
URL: https://github.com/apache/logging-log4j2/issues/3433#issuecomment-2723912447

   First, I have to ask some obvious questions.
   
   1. Why isn't your first thought to recommend changing this to
   ```
   logger.error("Doing this thing failed. Context Id: {}, SomeSettings: {}", context.id, someSetting, ex);
   ```
   2. Assuming for some reason they didn't want the first 3 log lines what would you tell someone who just did
   ```
   logger.error(ex);
   ```
   If I saw that I would tell them to add some descriptive text to describe what was being attempted. A stack trace with no other info is a software engineers nightmare.
   
   to be honest, I would very close to doing:
   ```
   if (obj instanceof Throwable) {
        LOGGER.warn("You need new software engineers");
   }
   ```
   and then ignoring the event.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org