You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by jo...@apache.org on 2022/09/04 16:11:39 UTC

[sling-org-apache-sling-servlets-post] branch SLING-11399-disable-stacktrace-logging created (now 2f2eb96)

This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a change to branch SLING-11399-disable-stacktrace-logging
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-post.git


      at 2f2eb96  SLING-11399 add option to disable printing the stacktraces of the PersistenceExceptions to the log

This branch includes the following new commits:

     new 2f2eb96  SLING-11399 add option to disable printing the stacktraces of the PersistenceExceptions to the log

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[sling-org-apache-sling-servlets-post] 01/01: SLING-11399 add option to disable printing the stacktraces of the PersistenceExceptions to the log

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a commit to branch SLING-11399-disable-stacktrace-logging
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-post.git

commit 2f2eb96024aadf82b13f0582913e245c05bd4604
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Sun Sep 4 18:10:52 2022 +0200

    SLING-11399 add option to disable printing the stacktraces of the PersistenceExceptions to the log
---
 .../sling/servlets/post/impl/SlingPostServlet.java | 28 +++++++++++++++----
 .../servlets/post/impl/SlingPostServletTest.java   | 31 ++++++++++++++++++++++
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java b/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java
index 55fffb6..24ca1a9 100644
--- a/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java
+++ b/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java
@@ -159,6 +159,10 @@ public class SlingPostServlet extends SlingAllMethodsServlet {
                 description="In backwards compatibility mode exceptions will always create a statuscode "
                     + "500 (see SLING-9896)")
         boolean legacy_statuscode_on_persistence_exception() default false;
+        
+        @AttributeDefinition(name="Log stacktraces on exceptions", 
+                description="Log the full stacktrace in case of an exception")
+        boolean logStacktraceInExceptions() default true;
     }
 
     /**
@@ -204,6 +208,8 @@ public class SlingPostServlet extends SlingAllMethodsServlet {
     private ImportOperation importOperation;
     
     private boolean backwardsCompatibleStatuscode;
+    
+    private boolean logStacktraceInExceptions;
 
     public SlingPostServlet() {
         // the following operations require JCR:
@@ -245,8 +251,7 @@ public class SlingPostServlet extends SlingAllMethodsServlet {
                 htmlResponse.setStatus(HttpServletResponse.SC_NOT_FOUND,
                     rnfe.getMessage());
             } catch (final PreconditionViolatedPersistenceException e) {
-                log.warn("Exception while handling POST on path [{}] with operation [{}]",
-                        request.getResource().getPath(),operation.getClass().getName(),e);
+                logPersistenceException(request, operation, e);
                 if (backwardsCompatibleStatuscode) {
                     htmlResponse.setError(e);
                 } else {
@@ -254,8 +259,7 @@ public class SlingPostServlet extends SlingAllMethodsServlet {
                 }
             } catch (final PersistenceException e) {
                 // also catches the  RetryableOperationException, as the handling is the same
-                log.warn("Exception while handling POST on path [{}] with operation [{}]",
-                        request.getResource().getPath(),operation.getClass().getName(),e);
+                logPersistenceException(request, operation, e);
                 if (backwardsCompatibleStatuscode) {
                     htmlResponse.setError(e);
                 } else {
@@ -266,7 +270,6 @@ public class SlingPostServlet extends SlingAllMethodsServlet {
                         request.getResource().getPath(),operation.getClass().getName(),e);
                 htmlResponse.setError(e);
             }
-
         }
 
         // check for redirect URL if processing succeeded
@@ -280,6 +283,20 @@ public class SlingPostServlet extends SlingAllMethodsServlet {
         htmlResponse.send(response, isSetStatus(request));
     }
 
+    protected void logPersistenceException (SlingHttpServletRequest request, PostOperation operation, Exception e ) {
+        if (logStacktraceInExceptions) {
+            log.warn("Exception while handling POST on path [{}] with operation [{}]",
+                request.getResource().getPath(),operation.getClass().getName(),e);
+        } else {
+            log.warn("{} while handling POST on path [{}] with operation [{}]: {}",
+                    e.getClass().getName(), 
+                    request.getResource().getPath(),
+                    operation.getClass().getName(),
+                    e.getMessage());
+        }
+    }
+    
+    
     /**
      * Redirects the HttpServletResponse, if redirectURL is not empty
      * @param htmlResponse
@@ -559,6 +576,7 @@ public class SlingPostServlet extends SlingAllMethodsServlet {
             this.importOperation.setIgnoredParameterNamePattern(paramMatchPattern);
         }
         this.backwardsCompatibleStatuscode = configuration.legacy_statuscode_on_persistence_exception();
+        this.logStacktraceInExceptions = configuration.logStacktraceInExceptions();
     }
 
     @Override
diff --git a/src/test/java/org/apache/sling/servlets/post/impl/SlingPostServletTest.java b/src/test/java/org/apache/sling/servlets/post/impl/SlingPostServletTest.java
index 2b9d4ed..4a594b9 100644
--- a/src/test/java/org/apache/sling/servlets/post/impl/SlingPostServletTest.java
+++ b/src/test/java/org/apache/sling/servlets/post/impl/SlingPostServletTest.java
@@ -25,16 +25,24 @@ import java.util.StringTokenizer;
 
 import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.request.header.MediaRangeList;
+import org.apache.sling.api.resource.Resource;
 import org.apache.sling.commons.testing.sling.MockSlingHttpServletRequest;
 import org.apache.sling.servlets.post.HtmlResponse;
 import org.apache.sling.servlets.post.JSONResponse;
+import org.apache.sling.servlets.post.PostOperation;
 import org.apache.sling.servlets.post.PostResponse;
 import org.apache.sling.servlets.post.SlingPostConstants;
 import org.apache.sling.servlets.post.impl.helper.MockSlingHttpServlet3Request;
 import org.apache.sling.servlets.post.impl.helper.MockSlingHttpServlet3Response;
+import org.apache.sling.servlets.post.impl.operations.DeleteOperation;
 
 import junit.framework.TestCase;
 
+import org.mockito.Mockito;
+import static org.mockito.Mockito.eq;
+import org.mockito.internal.util.reflection.Whitebox;
+import org.slf4j.Logger;
+
 public class SlingPostServletTest extends TestCase {
     
     private SlingPostServlet servlet;
@@ -116,6 +124,29 @@ public class SlingPostServletTest extends TestCase {
        	result = ((ErrorHandlingPostResponseWrapper)result).getWrapped();
         assertTrue(result instanceof JSONResponse);
     }
+    
+    
+    public void testPersistenceExceptionLogging() {
+        Logger log = Mockito.mock(Logger.class);
+        SlingHttpServletRequest mockRequest = Mockito.mock(SlingHttpServletRequest.class);
+        Resource mockResource = Mockito.mock(Resource.class);
+        Mockito.when(mockResource.getPath()).thenReturn("/path");
+        Mockito.when(mockRequest.getResource()).thenReturn(mockResource);
+        Whitebox.setInternalState(servlet, "log", log);
+        PostOperation operation = new DeleteOperation();
+        Exception exception = new IOException("foo");
+        
+        Whitebox.setInternalState(servlet, "logStacktraceInExceptions", true);
+        String expected = "Exception while handling POST on path [{}] with operation [{}]";
+        servlet.logPersistenceException(mockRequest, operation, exception);
+        Mockito.verify(log).warn(eq(expected),eq("/path"),eq("org.apache.sling.servlets.post.impl.operations.DeleteOperation"),eq(exception));
+        
+        Whitebox.setInternalState(servlet, "logStacktraceInExceptions", false);
+        expected = "{} while handling POST on path [{}] with operation [{}]: {}";
+        servlet.logPersistenceException(mockRequest, operation, exception);
+        Mockito.verify(log).warn(eq(expected),eq("java.io.IOException"),eq("/path"),eq("org.apache.sling.servlets.post.impl.operations.DeleteOperation"),eq("foo"));  
+    }
+    
 
     /**
      * SLING-10006 - verify we get the error handling wrapped PostResponse