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 <[email protected]> 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
