You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2025/08/21 15:42:37 UTC
(tomcat) branch 10.1.x updated: Review error logging. Include stack trace by default. Minor clean-up.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 4334698619 Review error logging. Include stack trace by default. Minor clean-up.
4334698619 is described below
commit 43346986198338e3f3405975ea2eeae650d465eb
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/NamingContextListener.java | 26 +++++++++++-----------
.../apache/catalina/ha/session/DeltaRequest.java | 4 ++--
java/org/apache/catalina/realm/CombinedRealm.java | 4 ++--
.../apache/catalina/session/DataSourceStore.java | 16 ++++++-------
.../catalina/session/LocalStrings.properties | 8 +++----
.../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 ++++++++
11 files changed, 45 insertions(+), 35 deletions(-)
diff --git a/java/org/apache/catalina/core/NamingContextListener.java b/java/org/apache/catalina/core/NamingContextListener.java
index ccf19e8bfe..0118dde312 100644
--- a/java/org/apache/catalina/core/NamingContextListener.java
+++ b/java/org/apache/catalina/core/NamingContextListener.java
@@ -235,7 +235,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
try {
createNamingContext();
} catch (NamingException e) {
- log.error(sm.getString("naming.namingContextCreationFailed", e));
+ log.error(sm.getString("naming.namingContextCreationFailed", container), e);
}
namingResources.addPropertyChangeListener(this);
@@ -248,7 +248,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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);
}
}
@@ -257,7 +257,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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);
@@ -572,7 +572,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
// 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);
}
}
@@ -581,7 +581,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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);
}
}
@@ -655,7 +655,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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);
}
}
@@ -755,7 +755,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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);
}
}
}
@@ -846,7 +846,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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) {
@@ -881,7 +881,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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) {
@@ -942,7 +942,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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);
}
}
@@ -977,7 +977,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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()) ||
@@ -1029,7 +1029,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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);
}
}
@@ -1061,7 +1061,7 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
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 3b1eeed892..5dd582d69c 100644
--- a/java/org/apache/catalina/session/DataSourceStore.java
+++ b/java/org/apache/catalina/session/DataSourceStore.java
@@ -363,7 +363,7 @@ public class DataSourceStore extends StoreBase {
}
}
} catch (SQLException e) {
- manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException", e));
+ manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException"), e);
keys = new String[0];
// Close the connection so that it gets reopened next time
} finally {
@@ -397,7 +397,7 @@ public class DataSourceStore extends StoreBase {
numberOfTries = 0;
}
} catch (SQLException e) {
- manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException", e));
+ manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException"), e);
} finally {
release(_conn);
}
@@ -444,7 +444,7 @@ public class DataSourceStore extends StoreBase {
numberOfTries = 0;
}
} catch (SQLException e) {
- contextLog.error(sm.getString("dataSourceStore.SQLException", e));
+ contextLog.error(sm.getString("dataSourceStore.SQLException"), e);
} finally {
context.unbind(Globals.IS_SECURITY_ENABLED, oldThreadContextCL);
release(_conn);
@@ -470,7 +470,7 @@ public class DataSourceStore extends StoreBase {
// Break out after the finally block
numberOfTries = 0;
} catch (SQLException e) {
- manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException", e));
+ manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException"), e);
} finally {
release(_conn);
}
@@ -518,7 +518,7 @@ public class DataSourceStore extends StoreBase {
// Break out after the finally block
numberOfTries = 0;
} catch (SQLException e) {
- manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException", e));
+ manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException"), e);
} finally {
release(_conn);
}
@@ -564,7 +564,7 @@ public class DataSourceStore extends StoreBase {
numberOfTries = 0;
}
} catch (SQLException e) {
- manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException", e));
+ manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException"), e);
} catch (IOException e) {
// Ignore
} finally {
@@ -601,7 +601,7 @@ public class DataSourceStore extends StoreBase {
}
}
} catch (SQLException ex) {
- manager.getContext().getLogger().error(sm.getString("dataSourceStore.checkConnectionSQLException", ex));
+ manager.getContext().getLogger().error(sm.getString("dataSourceStore.checkConnectionSQLException"), ex);
}
return conn;
@@ -686,7 +686,7 @@ public class DataSourceStore extends StoreBase {
try {
dbConnection.close();
} catch (SQLException e) {
- manager.getContext().getLogger().error(sm.getString("dataSourceStore.close", e));
+ manager.getContext().getLogger().error(sm.getString("dataSourceStore.close"), e);
}
}
diff --git a/java/org/apache/catalina/session/LocalStrings.properties b/java/org/apache/catalina/session/LocalStrings.properties
index 547c8ec122..c891d0d361 100644
--- a/java/org/apache/catalina/session/LocalStrings.properties
+++ b/java/org/apache/catalina/session/LocalStrings.properties
@@ -16,11 +16,11 @@
# Do not edit this file directly.
# To edit translations see: https://tomcat.apache.org/getinvolved.html#Translations
-dataSourceStore.SQLException=SQL Error [{0}]
+dataSourceStore.SQLException=SQL Error
dataSourceStore.checkConnectionDBClosed=The database connection is null or was found to be closed. Trying to re-open it.
dataSourceStore.checkConnectionDBReOpenFail=The re-open on the database failed. The database could be down.
-dataSourceStore.checkConnectionSQLException=A SQL exception occurred [{0}]
-dataSourceStore.close=Exception closing database connection [{0}]
+dataSourceStore.checkConnectionSQLException=A SQL exception occurred checking the connection
+dataSourceStore.close=Exception closing database connection
dataSourceStore.commitSQLException=SQLException committing connection before closing
dataSourceStore.loading=Loading Session [{0}] from database [{1}]
dataSourceStore.missingDataSource=No data source available
@@ -54,7 +54,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 e9cab923ff..791de5ca8c 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 c5714baf9c..a2adf02c90 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
@@ -1616,7 +1616,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 5ac7148fd8..96114c730d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,15 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 10.1.45 (schultz)" rtext="in development">
+ <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 10.1.44 (schultz)" rtext="2025-08-07">
<subsection name="Catalina">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org