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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to