From dbd7ce1f486297068046d9eb80c2c4786bea5bc7 Mon Sep 17 00:00:00 2001 From: Eva Vasques Date: Tue, 30 Jan 2024 15:32:41 +0000 Subject: [PATCH] MNT-24094 - error message logged in the logs when a webscript fails is not helpful as it should indicate what webscript triggers the error (#2421) * Add WebScriptRequest to the methods that call the render exception so we can log more info on the request when an error occurs. * Add request uri, user ans status code to the logging * Always log on exception but only show stack trace on debug or on internal server exception --- .../rest/api/NetworkWebScriptGet.java | 4 +- .../rest/api/NetworksWebScriptGet.java | 4 +- ...ublicApiTenantWebScriptServletRuntime.java | 2 +- .../rest/api/search/SearchApiWebscript.java | 2 +- .../api/search/SearchSQLApiWebscript.java | 4 +- .../rest/framework/tools/ResponseWriter.java | 103 ++++++++++++------ .../webscripts/AbstractResourceWebScript.java | 12 +- 7 files changed, 85 insertions(+), 46 deletions(-) diff --git a/remote-api/src/main/java/org/alfresco/rest/api/NetworkWebScriptGet.java b/remote-api/src/main/java/org/alfresco/rest/api/NetworkWebScriptGet.java index c1a17aa8ab..71c406b79a 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/NetworkWebScriptGet.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/NetworkWebScriptGet.java @@ -109,11 +109,11 @@ public class NetworkWebScriptGet extends ApiWebScript implements ResponseWriter } catch (ApiException | WebScriptException apiException) { - renderException(apiException, res, assistant); + renderException(apiException, res, req, assistant); } catch (RuntimeException runtimeException) { - renderException(runtimeException, res, assistant); + renderException(runtimeException, res, req, assistant); } } } diff --git a/remote-api/src/main/java/org/alfresco/rest/api/NetworksWebScriptGet.java b/remote-api/src/main/java/org/alfresco/rest/api/NetworksWebScriptGet.java index e5f4183060..0c82fb2f0e 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/NetworksWebScriptGet.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/NetworksWebScriptGet.java @@ -118,11 +118,11 @@ public class NetworksWebScriptGet extends ApiWebScript implements RecognizedPara } catch (ApiException | WebScriptException apiException) { - renderException(apiException, res, assistant); + renderException(apiException, res, req, assistant); } catch (RuntimeException runtimeException) { - renderException(runtimeException, res, assistant); + renderException(runtimeException, res, req, assistant); } } } \ No newline at end of file diff --git a/remote-api/src/main/java/org/alfresco/rest/api/PublicApiTenantWebScriptServletRuntime.java b/remote-api/src/main/java/org/alfresco/rest/api/PublicApiTenantWebScriptServletRuntime.java index fa20941698..43dad0fea0 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/PublicApiTenantWebScriptServletRuntime.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/PublicApiTenantWebScriptServletRuntime.java @@ -143,7 +143,7 @@ public class PublicApiTenantWebScriptServletRuntime extends TenantWebScriptServl else { try { - renderException((Exception)exception, response, apiAssistant); + renderException((Exception)exception, response, request, apiAssistant); } catch (IOException e) { logger.error("Internal error", e); throw new WebScriptException("Internal error", e); diff --git a/remote-api/src/main/java/org/alfresco/rest/api/search/SearchApiWebscript.java b/remote-api/src/main/java/org/alfresco/rest/api/search/SearchApiWebscript.java index c8fe871ddf..8ea59fa1e0 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/search/SearchApiWebscript.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/search/SearchApiWebscript.java @@ -108,7 +108,7 @@ public class SearchApiWebscript extends AbstractWebScript implements RecognizedP renderJsonResponse(webScriptResponse, toRender, assistant.getJsonHelper()); } catch (Exception exception) { - renderException(exception,webScriptResponse,assistant); + renderException(exception,webScriptResponse,webScriptRequest,assistant); } } diff --git a/remote-api/src/main/java/org/alfresco/rest/api/search/SearchSQLApiWebscript.java b/remote-api/src/main/java/org/alfresco/rest/api/search/SearchSQLApiWebscript.java index d4b1452031..a783286d3b 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/search/SearchSQLApiWebscript.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/search/SearchSQLApiWebscript.java @@ -102,11 +102,11 @@ public class SearchSQLApiWebscript extends AbstractWebScript implements Recogniz { if (exception instanceof QueryParserException) { - renderException(exception,res,assistant); + renderException(exception,res,webScriptRequest,assistant); } else { - renderException(new WebScriptException(400, exception.getMessage()), res, assistant); + renderException(new WebScriptException(400, exception.getMessage()), res, webScriptRequest, assistant); } } } diff --git a/remote-api/src/main/java/org/alfresco/rest/framework/tools/ResponseWriter.java b/remote-api/src/main/java/org/alfresco/rest/framework/tools/ResponseWriter.java index baa4a0282b..cef2358de9 100644 --- a/remote-api/src/main/java/org/alfresco/rest/framework/tools/ResponseWriter.java +++ b/remote-api/src/main/java/org/alfresco/rest/framework/tools/ResponseWriter.java @@ -26,37 +26,34 @@ package org.alfresco.rest.framework.tools; -import com.fasterxml.jackson.core.JsonGenerationException; -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.JsonMappingException; -import com.fasterxml.jackson.databind.ObjectMapper; -import org.alfresco.rest.framework.Api; +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.rest.framework.core.exceptions.DefaultExceptionResolver; import org.alfresco.rest.framework.core.exceptions.ErrorResponse; -import org.alfresco.rest.framework.core.exceptions.UnsupportedResourceOperationException; -import org.alfresco.rest.framework.jacksonextensions.BeanPropertiesFilter; -import org.alfresco.rest.framework.jacksonextensions.ExecutionResult; import org.alfresco.rest.framework.jacksonextensions.JacksonHelper; -import org.alfresco.rest.framework.resource.SerializablePagedCollection; -import org.alfresco.rest.framework.resource.content.BinaryResource; import org.alfresco.rest.framework.resource.content.ContentInfo; import org.alfresco.rest.framework.resource.content.ContentInfoImpl; -import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo; -import org.alfresco.rest.framework.resource.parameters.Params; import org.alfresco.rest.framework.webscripts.WithResponse; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.json.simple.JSONObject; -import org.springframework.beans.BeanUtils; import org.springframework.extensions.surf.util.I18NUtil; -import org.springframework.extensions.webscripts.*; +import org.springframework.extensions.webscripts.Cache; +import org.springframework.extensions.webscripts.Description; +import org.springframework.extensions.webscripts.Format; +import org.springframework.extensions.webscripts.Status; +import org.springframework.extensions.webscripts.WebScriptRequest; +import org.springframework.extensions.webscripts.WebScriptResponse; +import org.springframework.extensions.webscripts.WrappingWebScriptResponse; import org.springframework.extensions.webscripts.servlet.WebScriptServletResponse; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Map; +import com.fasterxml.jackson.core.JsonGenerationException; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; /* * Writes to the response @@ -193,24 +190,52 @@ public interface ResponseWriter default void renderErrorResponse(final ErrorResponse errorResponse, final WebScriptResponse res, final JacksonHelper jsonHelper) throws IOException { + renderErrorResponse(errorResponse, res, null, jsonHelper); + } - String logId = ""; - - if (Status.STATUS_INTERNAL_SERVER_ERROR == errorResponse.getStatusCode() || resWriterLogger().isDebugEnabled()) - { - logId = org.alfresco.util.GUID.generate(); - resWriterLogger().error(logId + " : " + errorResponse.getStackTrace()); - } - + /** + * Renders a JSON error response + * + * @param errorResponse The error + * @param res web script response + * @param req web script request + * @throws IOException + */ + default void renderErrorResponse(final ErrorResponse errorResponse, final WebScriptResponse res, final WebScriptRequest req, + final JacksonHelper jsonHelper) throws IOException + { String stackMessage = I18NUtil.getMessage(DefaultExceptionResolver.STACK_MESSAGE_ID); + String logId = org.alfresco.util.GUID.generate(); final ErrorResponse errorToWrite = new ErrorResponse(errorResponse.getErrorKey(), errorResponse.getStatusCode(), - errorResponse.getBriefSummary(), stackMessage, logId, errorResponse.getAdditionalState(), DefaultExceptionResolver.ERROR_URL); + errorResponse.getBriefSummary(), stackMessage, logId, errorResponse.getAdditionalState(), + DefaultExceptionResolver.ERROR_URL); + + String reqUrl = (req != null) ? req.getURL() : "unknown"; + String userName = AuthenticationUtil.getFullyAuthenticatedUser() != null ? AuthenticationUtil.getFullyAuthenticatedUser() + : "unauthenticated user"; + + // If internal server error or class in debug then print the stack trace + if (Status.STATUS_INTERNAL_SERVER_ERROR == errorResponse.getStatusCode() || resWriterLogger().isDebugEnabled()) + { + resWriterLogger().error("Exception " + errorToWrite.getLogId() + ". Request " + reqUrl + " executed by " + userName + + " returned status code " + errorResponse.getStatusCode() + " with message: " + + errorResponse.getBriefSummary() + " - Stack Trace: " + errorResponse.getStackTrace()); + } + else + { + resWriterLogger().error("Exception " + errorToWrite.getLogId() + ". Request " + reqUrl + " executed by user " + + userName + " returned status code " + errorResponse.getStatusCode() + " with message: " + + errorResponse.getBriefSummary() + " - Increase logging on " + this.getClass().getName() + + " for stack trace."); + } setContentInfoOnResponse(res, DEFAULT_JSON_CONTENT); - // Status must be set before the response is written by Jackson (which will by default close and commit the response). - // In a r/w txn, web script buffered responses ensure that it doesn't really matter but for r/o txns this is important. + // Status must be set before the response is written by Jackson (which will by default close and commit the + // response). + // In a r/w txn, web script buffered responses ensure that it doesn't really matter but for r/o txns this is + // important. res.setStatus(errorToWrite.getStatusCode()); jsonHelper.withWriter(res.getOutputStream(), new JacksonHelper.Writer() @@ -218,7 +243,7 @@ public interface ResponseWriter @SuppressWarnings("unchecked") @Override public void writeContents(JsonGenerator generator, ObjectMapper objectMapper) - throws JsonGenerationException, JsonMappingException, IOException + throws JsonGenerationException, JsonMappingException, IOException { JSONObject obj = new JSONObject(); obj.put("error", errorToWrite); @@ -236,7 +261,21 @@ public interface ResponseWriter */ default void renderException(final Exception exception, final WebScriptResponse response, final ApiAssistant assistant) throws IOException { - renderErrorResponse(assistant.resolveException(exception), response, assistant.getJsonHelper()); + renderException(exception, response, null, assistant); + } + + /** + * Renders an exception to the output stream as Json. + * + * @param exception + * @param response + * @param request + * @throws IOException + */ + default void renderException(final Exception exception, final WebScriptResponse response, final WebScriptRequest request, + final ApiAssistant assistant) throws IOException + { + renderErrorResponse(assistant.resolveException(exception), response, request, assistant.getJsonHelper()); } /** diff --git a/remote-api/src/main/java/org/alfresco/rest/framework/webscripts/AbstractResourceWebScript.java b/remote-api/src/main/java/org/alfresco/rest/framework/webscripts/AbstractResourceWebScript.java index bb2a547ab9..32c0180c9c 100644 --- a/remote-api/src/main/java/org/alfresco/rest/framework/webscripts/AbstractResourceWebScript.java +++ b/remote-api/src/main/java/org/alfresco/rest/framework/webscripts/AbstractResourceWebScript.java @@ -180,15 +180,15 @@ public abstract class AbstractResourceWebScript extends ApiWebScript implements } catch (ContentIOException cioe) { - handleContentIOException(res, cioe); + handleContentIOException(res, req, cioe); } catch (AlfrescoRuntimeException | ApiException | WebScriptException xception ) { - renderException(xception, res, assistant); + renderException(xception, res, req, assistant); } catch (RuntimeException runtimeException) { - renderException(runtimeException, res, assistant); + renderException(runtimeException, res, req, assistant); } finally { @@ -224,17 +224,17 @@ public abstract class AbstractResourceWebScript extends ApiWebScript implements return toReturn; } - private void handleContentIOException(final WebScriptResponse res, ContentIOException exception) throws IOException + private void handleContentIOException(final WebScriptResponse res, final WebScriptRequest req, ContentIOException exception) throws IOException { // If the Content-Length is not set back to -1 any client will expect to receive binary and will hang until it times out res.setHeader(HEADER_CONTENT_LENGTH, String.valueOf(-1)); if (exception instanceof ArchivedIOException) { - renderException(new ArchivedContentException(exception.getMsgId(), exception), res, assistant); + renderException(new ArchivedContentException(exception.getMsgId(), exception), res, req, assistant); } else { - renderException(exception, res, assistant); + renderException(exception, res, req, assistant); } }