This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new d41dab4141 Review error logging. Include stack trace by default. Minor
clean-up.
d41dab4141 is described below
commit d41dab4141d86d752b2a32b721d2e9fb637428c4
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Aug 19 12:39:35 2025 +0100
Review error logging. Include stack trace by default. Minor clean-up.
---
.../catalina/core/ApplicationFilterChain.java | 12 +++-------
.../catalina/core/NamingContextListener.java | 26 +++++++++++-----------
.../apache/catalina/ha/session/DeltaRequest.java | 4 ++--
java/org/apache/catalina/realm/CombinedRealm.java | 4 ++--
.../apache/catalina/session/DataSourceStore.java | 15 ++++++-------
.../catalina/session/LocalStrings.properties | 4 ++--
.../catalina/session/PersistentManagerBase.java | 2 +-
.../org/apache/catalina/startup/ContextConfig.java | 3 ++-
.../tribes/transport/nio/NioReplicationTask.java | 2 +-
.../util/net/openssl/panama/OpenSSLContext.java | 4 ++--
.../apache/tomcat/jdbc/pool/ConnectionPool.java | 2 +-
webapps/docs/changelog.xml | 9 ++++++++
12 files changed, 45 insertions(+), 42 deletions(-)
diff --git a/java/org/apache/catalina/core/ApplicationFilterChain.java
b/java/org/apache/catalina/core/ApplicationFilterChain.java
index 8f5b5cfd99..3b1894ca6c 100644
--- a/java/org/apache/catalina/core/ApplicationFilterChain.java
+++ b/java/org/apache/catalina/core/ApplicationFilterChain.java
@@ -170,8 +170,8 @@ public final class ApplicationFilterChain implements
FilterChain {
} catch (IOException | ServletException | RuntimeException e) {
throw e;
} catch (Throwable t) {
- e = ExceptionUtils.unwrapInvocationTargetException(t);
- ExceptionUtils.handleThrowable(e);
+ t = ExceptionUtils.unwrapInvocationTargetException(t);
+ ExceptionUtils.handleThrowable(t);
throw new ServletException(sm.getString("filterChain.filter"),
t);
}
return;
@@ -200,16 +200,10 @@ public final class ApplicationFilterChain implements
FilterChain {
}
} catch (IOException | ServletException | RuntimeException e) {
throw e;
-<<<<<<< HEAD
- } catch (Throwable e) {
- e = ExceptionUtils.unwrapInvocationTargetException(e);
- ExceptionUtils.handleThrowable(e);
- throw new ServletException(sm.getString("filterChain.servlet"), e);
-=======
} catch (Throwable t) {
+ t = ExceptionUtils.unwrapInvocationTargetException(t);
ExceptionUtils.handleThrowable(t);
throw new ServletException(sm.getString("filterChain.servlet"), t);
->>>>>>> 99ceef12d3 (Code clean-up - no functional change. Use 't' for
throwable)
} finally {
if (ApplicationDispatcher.WRAP_SAME_OBJECT) {
lastServicedRequest.set(null);
diff --git a/java/org/apache/catalina/core/NamingContextListener.java
b/java/org/apache/catalina/core/NamingContextListener.java
index 5472eceacc..9768c0ecca 100644
--- a/java/org/apache/catalina/core/NamingContextListener.java
+++ b/java/org/apache/catalina/core/NamingContextListener.java
@@ -237,7 +237,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
try {
createNamingContext();
} catch (NamingException e) {
-
log.error(sm.getString("naming.namingContextCreationFailed", e));
+
log.error(sm.getString("naming.namingContextCreationFailed", container), e);
}
namingResources.addPropertyChangeListener(this);
@@ -250,7 +250,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
ContextBindings.bindClassLoader(container, token,
((Context)
container).getLoader().getClassLoader());
} catch (NamingException e) {
- log.error(sm.getString("naming.bindFailed", e));
+ log.error(sm.getString("naming.bindFailed",
container), e);
}
}
@@ -259,7 +259,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
try {
ContextBindings.bindClassLoader(container, token,
this.getClass().getClassLoader());
} catch (NamingException e) {
- log.error(sm.getString("naming.bindFailed", e));
+ log.error(sm.getString("naming.bindFailed",
container), e);
}
if (container instanceof StandardServer) {
((StandardServer)
container).setGlobalNamingContext(namingContext);
@@ -591,7 +591,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
// Ignore because UserTransaction was obviously
// added via ResourceLink
} catch (NamingException e) {
- log.error(sm.getString("naming.bindFailed", e));
+ log.error(sm.getString("naming.bindFailed",
"UserTransaction"), e);
}
}
@@ -600,7 +600,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
try {
compCtx.bind("Resources", ((Context)
container).getResources());
} catch (NamingException e) {
- log.error(sm.getString("naming.bindFailed", e));
+ log.error(sm.getString("naming.bindFailed", "Resources"), e);
}
}
@@ -674,7 +674,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
createSubcontexts(envCtx, ejb.getName());
envCtx.bind(ejb.getName(), ref);
} catch (NamingException e) {
- log.error(sm.getString("naming.bindFailed", e));
+ log.error(sm.getString("naming.bindFailed", ejb.getName()), e);
}
}
@@ -774,7 +774,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
createSubcontexts(envCtx, env.getName());
envCtx.bind(env.getName(), value);
} catch (NamingException e) {
- log.error(sm.getString("naming.invalidEnvEntryValue", e));
+ log.error(sm.getString("naming.invalidEnvEntryValue",
env.getName()), e);
}
}
}
@@ -865,7 +865,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
log.debug(sm.getString("naming.addSlash",
service.getWsdlfile()));
}
} catch (MalformedURLException e) {
- log.error(sm.getString("naming.wsdlFailed", e));
+ log.error(sm.getString("naming.wsdlFailed",
service.getWsdlfile()), e);
}
}
if (wsdlURL == null) {
@@ -900,7 +900,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
log.debug(sm.getString("naming.addSlash",
service.getJaxrpcmappingfile()));
}
} catch (MalformedURLException e) {
- log.error(sm.getString("naming.wsdlFailed", e));
+ log.error(sm.getString("naming.wsdlFailed",
service.getJaxrpcmappingfile()), e);
}
}
if (jaxrpcURL == null) {
@@ -961,7 +961,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
createSubcontexts(envCtx, service.getName());
envCtx.bind(service.getName(), ref);
} catch (NamingException e) {
- log.error(sm.getString("naming.bindFailed", e));
+ log.error(sm.getString("naming.bindFailed", service.getName()), e);
}
}
@@ -996,7 +996,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
createSubcontexts(envCtx, resource.getName());
envCtx.bind(resource.getName(), ref);
} catch (NamingException e) {
- log.error(sm.getString("naming.bindFailed", e));
+ log.error(sm.getString("naming.bindFailed", resource.getName()),
e);
}
if (("javax.sql.DataSource".equals(ref.getClassName()) ||
@@ -1048,7 +1048,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
createSubcontexts(envCtx, resourceEnvRef.getName());
envCtx.bind(resourceEnvRef.getName(), ref);
} catch (NamingException e) {
- log.error(sm.getString("naming.bindFailed", e));
+ log.error(sm.getString("naming.bindFailed",
resourceEnvRef.getName()), e);
}
}
@@ -1080,7 +1080,7 @@ public class NamingContextListener implements
LifecycleListener, ContainerListen
createSubcontexts(envCtx, resourceLink.getName());
ctx.bind(resourceLink.getName(), ref);
} catch (NamingException e) {
- log.error(sm.getString("naming.bindFailed", e));
+ log.error(sm.getString("naming.bindFailed",
resourceLink.getName()), e);
}
ResourceLinkFactory.registerGlobalResourceAccess(getGlobalNamingContext(),
resourceLink.getName(),
diff --git a/java/org/apache/catalina/ha/session/DeltaRequest.java
b/java/org/apache/catalina/ha/session/DeltaRequest.java
index d57bc8a24e..eb0cb715be 100644
--- a/java/org/apache/catalina/ha/session/DeltaRequest.java
+++ b/java/org/apache/catalina/ha/session/DeltaRequest.java
@@ -264,8 +264,8 @@ public class DeltaRequest implements Externalizable {
public void setSessionId(String sessionId) {
this.sessionId = sessionId;
if (sessionId == null) {
- Exception e = new
Exception(sm.getString("deltaRequest.ssid.null"));
- log.error(sm.getString("deltaRequest.ssid.null"),
e.fillInStackTrace());
+ String msg = sm.getString("deltaRequest.ssid.null");
+ log.error(msg, new Exception(msg));
}
}
diff --git a/java/org/apache/catalina/realm/CombinedRealm.java
b/java/org/apache/catalina/realm/CombinedRealm.java
index 70f3debfef..dcef3c3c50 100644
--- a/java/org/apache/catalina/realm/CombinedRealm.java
+++ b/java/org/apache/catalina/realm/CombinedRealm.java
@@ -359,7 +359,7 @@ public class CombinedRealm extends RealmBase {
// Stack trace will show where this was called from
UnsupportedOperationException uoe =
new
UnsupportedOperationException(sm.getString("combinedRealm.getPassword"));
- log.error(sm.getString("combinedRealm.unexpectedMethod"), uoe);
+ log.error(uoe.getMessage(), uoe);
throw uoe;
}
@@ -369,7 +369,7 @@ public class CombinedRealm extends RealmBase {
// Stack trace will show where this was called from
UnsupportedOperationException uoe =
new
UnsupportedOperationException(sm.getString("combinedRealm.getPrincipal"));
- log.error(sm.getString("combinedRealm.unexpectedMethod"), uoe);
+ log.error(uoe.getMessage(), uoe);
throw uoe;
}
diff --git a/java/org/apache/catalina/session/DataSourceStore.java
b/java/org/apache/catalina/session/DataSourceStore.java
index 94006ad55b..bffb9016d0 100644
--- a/java/org/apache/catalina/session/DataSourceStore.java
+++ b/java/org/apache/catalina/session/DataSourceStore.java
@@ -106,7 +106,7 @@ public class DataSourceStore extends JDBCStore {
}
}
} catch (SQLException e) {
-
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException", e));
+
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException"), e);
keys = new String[0];
// Close the connection so that it gets reopened next time
} finally {
@@ -140,7 +140,7 @@ public class DataSourceStore extends JDBCStore {
numberOfTries = 0;
}
} catch (SQLException e) {
-
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException", e));
+
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException"), e);
} finally {
release(_conn);
}
@@ -187,7 +187,7 @@ public class DataSourceStore extends JDBCStore {
numberOfTries = 0;
}
} catch (SQLException e) {
- contextLog.error(sm.getString(getStoreName() +
".SQLException", e));
+ contextLog.error(sm.getString(getStoreName() +
".SQLException"), e);
} finally {
context.unbind(Globals.IS_SECURITY_ENABLED,
oldThreadContextCL);
release(_conn);
@@ -213,7 +213,7 @@ public class DataSourceStore extends JDBCStore {
// Break out after the finally block
numberOfTries = 0;
} catch (SQLException e) {
-
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException", e));
+
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException"), e);
} finally {
release(_conn);
}
@@ -261,7 +261,7 @@ public class DataSourceStore extends JDBCStore {
// Break out after the finally block
numberOfTries = 0;
} catch (SQLException e) {
-
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException", e));
+
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException"), e);
} finally {
release(_conn);
}
@@ -307,7 +307,7 @@ public class DataSourceStore extends JDBCStore {
numberOfTries = 0;
}
} catch (SQLException e) {
-
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException", e));
+
manager.getContext().getLogger().error(sm.getString(getStoreName() +
".SQLException"), e);
} catch (IOException e) {
// Ignore
} finally {
@@ -407,8 +407,7 @@ public class DataSourceStore extends JDBCStore {
try {
dbConnection.close();
} catch (SQLException e) {
- manager.getContext().getLogger().error(sm.getString(getStoreName()
+ ".close", e.toString())); // Just log
-
// it here
+ manager.getContext().getLogger().error(sm.getString(getStoreName()
+ ".close"), e); // Just log it here
}
}
diff --git a/java/org/apache/catalina/session/LocalStrings.properties
b/java/org/apache/catalina/session/LocalStrings.properties
index 8cc056a1fb..ff4a417210 100644
--- a/java/org/apache/catalina/session/LocalStrings.properties
+++ b/java/org/apache/catalina/session/LocalStrings.properties
@@ -16,7 +16,7 @@
# Do not edit this file directly.
# To edit translations see:
https://tomcat.apache.org/getinvolved.html#Translations
-JDBCStore.SQLException=SQL Error [{0}]
+JDBCStore.SQLException=SQL Error
JDBCStore.checkConnectionClassNotFoundException=JDBC driver class not found
[{0}]
JDBCStore.checkConnectionDBClosed=The database connection is null or was found
to be closed. Trying to re-open it.
JDBCStore.checkConnectionDBReOpenFail=The re-open on the database failed. The
database could be down.
@@ -55,7 +55,7 @@ persistentManager.isLoadedError=Error checking if session
[{0}] is loaded in mem
persistentManager.loading=Loading [{0}] persisted sessions
persistentManager.noStore=No Store configured, persistence disabled
persistentManager.removeError=Error removing session [{0}] from the store
-persistentManager.serializeError=Error serializing Session [{0}]: [{1}]
+persistentManager.serializeError=Error serializing Session [{0}]
persistentManager.storeClearError=Error clearning all sessions from the store
persistentManager.storeKeysException=Unable to determine the list of session
IDs for sessions in the session store, assuming that the store is empty
persistentManager.storeLoadError=Error swapping in sessions from the store
diff --git a/java/org/apache/catalina/session/PersistentManagerBase.java
b/java/org/apache/catalina/session/PersistentManagerBase.java
index e257478495..f4d2845d11 100644
--- a/java/org/apache/catalina/session/PersistentManagerBase.java
+++ b/java/org/apache/catalina/session/PersistentManagerBase.java
@@ -797,7 +797,7 @@ public abstract class PersistentManagerBase extends
ManagerBase implements Store
store.save(session);
}
} catch (IOException e) {
- log.error(sm.getString("persistentManager.serializeError",
session.getIdInternal(), e));
+ log.error(sm.getString("persistentManager.serializeError",
session.getIdInternal()), e);
throw e;
}
diff --git a/java/org/apache/catalina/startup/ContextConfig.java
b/java/org/apache/catalina/startup/ContextConfig.java
index 87558aedfa..a7f3a82472 100644
--- a/java/org/apache/catalina/startup/ContextConfig.java
+++ b/java/org/apache/catalina/startup/ContextConfig.java
@@ -739,7 +739,8 @@ public class ContextConfig implements LifecycleListener {
}
} catch (SAXParseException e) {
log.error(sm.getString("contextConfig.contextParse",
context.getName()), e);
- log.error(sm.getString("contextConfig.defaultPosition", "" +
e.getLineNumber(), "" + e.getColumnNumber()));
+ log.error(sm.getString("contextConfig.defaultPosition",
Integer.toString(e.getLineNumber()),
+ Integer.toString(e.getColumnNumber())));
ok = false;
} catch (Exception e) {
log.error(sm.getString("contextConfig.contextParse",
context.getName()), e);
diff --git
a/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java
b/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java
index ab34308717..cff405bbb8 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java
@@ -228,7 +228,7 @@ public class NioReplicationTask extends AbstractRxTask {
}
} catch (RemoteProcessException e) {
if (log.isDebugEnabled()) {
-
log.error(sm.getString("nioReplicationTask.process.clusterMsg.failed"), e);
+
log.debug(sm.getString("nioReplicationTask.process.clusterMsg.failed"), e);
}
if (ChannelData.sendAckSync(msg.getOptions())) {
sendAck(key, (WritableByteChannel) channel,
Constants.FAIL_ACK_COMMAND, saddr);
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index 4b60ec676e..7400d475e6 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -365,7 +365,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
}
}
} catch (Exception e) {
- log.error(sm.getString("opensslconf.checkFailed",
e.getLocalizedMessage()));
+ log.error(sm.getString("opensslconf.checkFailed",
e.getLocalizedMessage()), e);
return false;
}
if (!ok) {
@@ -414,7 +414,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
}
}
} catch (Exception e) {
- log.error(sm.getString("opensslconf.applyFailed"));
+ log.error(sm.getString("opensslconf.applyFailed"), e);
return false;
}
if (rc <= 0) {
diff --git
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
index 3fb8d89fd1..e8acaef630 100644
---
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
+++
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
@@ -1609,7 +1609,7 @@ public class ConnectionPool {
pool.testAllIdle(true);
}
} catch (Exception x) {
- log.error("", x);
+ log.error(x.toString(), x);
}
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ce08edebdb..b12ee80031 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -113,6 +113,15 @@
</fix>
</changelog>
</subsection>
+ <subsection name = "Other">
+ <changelog>
+ <scode>
+ Review logging and include the full stack trace and exception message
+ by default rather then just the exception message when logging an error
+ in response to an exception. (markt)
+ </scode>
+ </changelog>
+ </subsection>
</section>
<section name="Tomcat 9.0.108 (remm)" rtext="2025-08-06">
<subsection name="Catalina">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
