This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 12c27a05b7 Review error logging. Include stack trace by default. Minor
clean-up.
12c27a05b7 is described below
commit 12c27a05b737855085e245562041fa9a8ac849eb
Author: Mark Thomas <[email protected]>
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 | 5 +++++
11 files changed, 41 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 e7e72c6c5a..753dc44f62 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 2516f81768..62bb15b154 100644
--- a/java/org/apache/catalina/session/DataSourceStore.java
+++ b/java/org/apache/catalina/session/DataSourceStore.java
@@ -362,7 +362,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 {
@@ -396,7 +396,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);
}
@@ -443,7 +443,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(oldThreadContextCL);
release(_conn);
@@ -469,7 +469,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);
}
@@ -517,7 +517,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);
}
@@ -563,7 +563,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 {
@@ -600,7 +600,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;
@@ -685,7 +685,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 3b963c9ea0..693257c7da 100644
--- a/java/org/apache/catalina/session/LocalStrings.properties
+++ b/java/org/apache/catalina/session/LocalStrings.properties
@@ -13,11 +13,11 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-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
@@ -51,7 +51,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 d57a39680f..772dd461cd 100644
--- a/java/org/apache/catalina/session/PersistentManagerBase.java
+++ b/java/org/apache/catalina/session/PersistentManagerBase.java
@@ -666,7 +666,7 @@ public abstract class PersistentManagerBase extends
ManagerBase implements Store
try {
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 cc7241a4dc..999b519461 100644
--- a/java/org/apache/catalina/startup/ContextConfig.java
+++ b/java/org/apache/catalina/startup/ContextConfig.java
@@ -736,7 +736,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 81ec2bcb90..c2e490f06c 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java
@@ -227,7 +227,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 01e319a5c6..e7f4b6d1b8 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 2d02f24884..8b5bf997be 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
@@ -1607,7 +1607,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 887518fdfc..d0a2cd4c81 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -271,6 +271,11 @@
Update Derby to 10.17.1.0. (markt)
</update>
<!-- Entries for backport and removal before 12.0.0-M1 below this line
-->
+ <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>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
