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
This commit is contained in:
Eva Vasques
2024-01-30 15:32:41 +00:00
committed by GitHub
parent 60e42b4090
commit dbd7ce1f48
7 changed files with 85 additions and 46 deletions

View File

@@ -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);
}
}
}

View File

@@ -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);
}
}
}

View File

@@ -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);

View File

@@ -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);
}
}

View File

@@ -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);
}
}
}

View File

@@ -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
{
String logId = "";
if (Status.STATUS_INTERNAL_SERVER_ERROR == errorResponse.getStatusCode() || resWriterLogger().isDebugEnabled())
{
logId = org.alfresco.util.GUID.generate();
resWriterLogger().error(logId + " : " + errorResponse.getStackTrace());
renderErrorResponse(errorResponse, res, null, jsonHelper);
}
/**
* 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()
@@ -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());
}
/**

View File

@@ -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);
}
}