Skip to content

Commit

Permalink
[CSAPI]: Log more details on invalid requests
Browse files Browse the repository at this point in the history
Previously, just the exception message was logged. For some exceptions,
this is quite non-descript, for example:

    2024-01-25 06:17:14.061 DEBUG [SWAPool-1] - Invalid request (BAD_PAYLOAD): Invalid payload: Invalid XML: Received event END_DOCUMENT, instead of START_ELEMENT or END_ELEMENT.

To solve this error, a backtrace is needed to figure out what was
expected exactly.

This particular exception message could probably be improved by itself,
but this commit solves this more generically, since there might be other
exceptions suffering from the same issue.

This commit changes the handling of InvalidRequestExceptions to use the
existing `logError()` function. This logs details of the request and
a backtrace. To ensure that this does not log "Internal server error"
but keep the existing "Invalid request", `logError()` is refactored to
accept an error prefix.
  • Loading branch information
matthijskooijman committed Feb 1, 2024
1 parent d111a69 commit 5ca3684
Showing 1 changed file with 13 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public abstract class RestApiServlet extends HttpServlet
{
static final String LOG_REQUEST_MSG = "{} {}{} (from ip={}, user={})";
static final String INTERNAL_ERROR_MSG = "Internal server error";
static final String INTERNAL_ERROR_LOG_MSG = INTERNAL_ERROR_MSG + " while processing request " + LOG_REQUEST_MSG;
static final String ERROR_LOG_MSG = "{} while processing request " + LOG_REQUEST_MSG;
static final String ACCESS_DENIED_ERROR_MSG = "Permission denied";
static final String JSON_CONTENT_TYPE = "application/json";

Expand Down Expand Up @@ -379,18 +379,24 @@ public Object createWebSocket(ServletUpgradeRequest req, ServletUpgradeResponse
protected void logRequest(HttpServletRequest req)
{
if (log.isInfoEnabled())
logRequestInfo(req, null);
logRequestInfo(req, null, null);
}


protected void logError(HttpServletRequest req, Throwable e)
{
logError(req, null, e);
}


protected void logError(HttpServletRequest req, String error_prefix, Throwable e)
{
if (log.isErrorEnabled())
logRequestInfo(req, e);
logRequestInfo(req, error_prefix, e);
}


protected void logRequestInfo(HttpServletRequest req, Throwable error)
protected void logRequestInfo(HttpServletRequest req, String error_prefix, Throwable error)
{
String method = req.getMethod();
String url = req.getRequestURI();
Expand All @@ -416,7 +422,7 @@ protected void logRequestInfo(HttpServletRequest req, Throwable error)
query = "?" + req.getQueryString();

if (error != null)
log.error(INTERNAL_ERROR_LOG_MSG, method, url, query, ip, user, error);
log.error(ERROR_LOG_MSG, error_prefix, method, url, query, ip, user, error);
else
log.info(LOG_REQUEST_MSG, method, url, query, ip, user);
}
Expand Down Expand Up @@ -459,8 +465,8 @@ public void sendError(int code, String msg, HttpServletRequest req, HttpServletR

protected void handleInvalidRequestException(HttpServletRequest req, HttpServletResponse resp, InvalidRequestException e)
{
log.debug("Invalid request ({}): {}", e.getErrorCode(), e.getMessage());
logError(req, String.format("Invalid request (%s)", e.getErrorCode()), e);

switch (e.getErrorCode())
{
case UNSUPPORTED_OPERATION:
Expand Down

0 comments on commit 5ca3684

Please sign in to comment.