On 19/08/2025 15:52, Christopher Schultz wrote:
Mark,
On 8/19/25 3:01 AM, Mark Thomas wrote:
All,
I stumbled across quite few instances of log messages like this while
I was looking at something for $dayjob:
log.error(sm.getString("naming.bindFailed", e));
It would be lot more useful with the stack trace so I think that
really should be:
log.error(sm.getString("naming.bindFailed"), e);
I had to read that 3 or 4 times to see that the ) had moved.
+1 for this change. If there is an exception being logged, I'm 100% in
favor of logging a stack trace. Most logging frameworks can be
configured to log stack traces or not. But they can't log (or not) if
the exception itself is not present.
Unless there are objections, I am going to change any log message with
a level of warning or greater that currently includes e, e.toString(),
e.getMessage() etc in the log message and doesn't log the full stack
trace to log the stack trace. I'll likely remove the exception from
the log message as well to avoid duplicating the text in the logs.
INFO level and is a little different. Those need looking at on a case
by case basis as I can imagine there are some cases where logging the
full stack trace is not necessary.
+1
Reviewing log messages you end up looking at a LOT of catch blocks and
it is apparent that there are multiple naming conventions that have been
used over time / by different developers.
Since I am in the area I've got Cursor looking into using a consistent
naming convention. So far, its fixed:
"catch (Exception t)" to "catch (Exception e)"
"catch (Throwable e)" to "catch (Throwable t)"
which were the obviously confusing ones.
I've currently got it working on
"catch (Exception <not e>)" to "catch (Exception e)"
although I want to look at the diff for that one to see how big it is
before deciding whether or not it is worth the noise to commit.
Depending on how that goes, I might get it to do
"catch (IOException <not ioe>)" to "catch (IOException ioe)"
After that ... ?
There is a question of how far to take this and the commits already made
to main might be enough. There is a trade off to make between consistent
code being easier to read and noisy history making it harder to review
changes over time.
Thoughts?
Mark
P.S. I will be back-porting the logging changes but I might leave it
until tomorrow in case folks object
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
