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:40 UTC

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

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