From 571635802a417b8e6ef6905f699d5a6fbb32078f Mon Sep 17 00:00:00 2001 From: Pavan Manish Date: Thu, 28 Nov 2024 17:44:36 +0530 Subject: [PATCH 1/4] FT-800: Added better message logging for unhandled exceptions --- .../atlas/web/errors/ExceptionMapperUtil.java | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java b/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java index e2511e5d50..50b09f11ee 100644 --- a/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java +++ b/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java @@ -19,15 +19,73 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; +import java.util.List; public class ExceptionMapperUtil { protected static final Logger LOGGER = LoggerFactory.getLogger(ExceptionMapperUtil.class); @SuppressWarnings("UnusedParameters") protected static String formatErrorMessage(long id, Exception exception) { - return String.format("There was an error processing your request. It has been logged (ID %016x).", id); + StringBuilder response = new StringBuilder(); + + // Add error ID and general error message + response.append("{\n") + .append(String.format(" \"errorId\": \"%016x\",\n", id)) + .append(" \"message\": \"There was an error processing your request.\",\n") + .append(" \"causes\": [\n"); + + // Traverse through the chain of causes + List causes = new ArrayList<>(); + Throwable currentException = exception; + while (currentException != null) { + causes.add(formatCause(currentException)); + currentException =currentException .getCause(); + } + + // Add all formatted causes to the response + for (int i = 0; i < causes.size(); i++) { + response.append(causes.get(i)); + if (i < causes.size() - 1) { + response.append(",\n"); + } + } + + // Close the JSON structure + response.append("\n ]\n") + .append("}"); + + return response.toString(); + } + + // Helper method to format a single exception cause + private static String formatCause(Throwable exception) { + StringBuilder cause = new StringBuilder(); + + // Extract location details from the first stack trace element + StackTraceElement[] stackTrace = exception.getStackTrace(); + String location = "Unavailable"; + if (stackTrace != null && stackTrace.length > 0) { + StackTraceElement element = stackTrace[0]; + location = String.format("%s.%s (%s:%d)", + element.getClassName(), + element.getMethodName(), + element.getFileName(), + element.getLineNumber()); + } + + // Build JSON object for this cause + cause.append(" {\n") + .append(" \"errorType\": \"").append(exception.getClass().getName()).append("\",\n") + .append(" \"errorMessage\": \"").append(exception.getMessage() != null ? exception.getMessage() : "No additional information provided").append("\",\n") + .append(" \"location\": \"").append(location).append("\"\n") + .append(" }"); + + return cause.toString(); } + + protected static void logException(long id, Exception exception) { LOGGER.error(formatLogMessage(id, exception), exception); } From b8cc3c2d2c2ffd4a1886498aedcbc0b03723d177 Mon Sep 17 00:00:00 2001 From: Pavan Manish Date: Tue, 3 Dec 2024 15:53:22 +0530 Subject: [PATCH 2/4] FT-800: Handle circular references in exception chain in formatErrorMessage method --- .../apache/atlas/web/errors/ExceptionMapperUtil.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java b/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java index 50b09f11ee..d8e45cea1e 100644 --- a/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java +++ b/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java @@ -35,12 +35,18 @@ protected static String formatErrorMessage(long id, Exception exception) { .append(" \"message\": \"There was an error processing your request.\",\n") .append(" \"causes\": [\n"); - // Traverse through the chain of causes + // Traverse through the chain of causes, avoiding cycles List causes = new ArrayList<>(); + List visited = new ArrayList<>(); Throwable currentException = exception; while (currentException != null) { + if (visited.contains(currentException)) { + causes.add(" {\n \"errorType\": \"CircularReferenceDetected\",\n \"errorMessage\": \"A circular reference was detected in the exception chain.\",\n \"location\": \"Unavailable\"\n }"); + break; + } + visited.add(currentException); causes.add(formatCause(currentException)); - currentException =currentException .getCause(); + currentException = currentException.getCause(); } // Add all formatted causes to the response From 079cf4133ce65b3aa23358b4e40fdd0f289c054b Mon Sep 17 00:00:00 2001 From: Pavan Manish Date: Tue, 3 Dec 2024 16:04:02 +0530 Subject: [PATCH 3/4] FT-800: Handle null exceptions gracefully in formatErrorMessage method --- .../org/apache/atlas/web/errors/ExceptionMapperUtil.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java b/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java index d8e45cea1e..03df167d5d 100644 --- a/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java +++ b/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java @@ -27,6 +27,15 @@ public class ExceptionMapperUtil { @SuppressWarnings("UnusedParameters") protected static String formatErrorMessage(long id, Exception exception) { + if (exception == null) { + // If the exception is null, return a minimal error message + return "{\n" + + String.format(" \"errorId\": \"%016x\",\n", id) + + " \"message\": \"No exception provided\",\n" + + " \"causes\": []\n" + + "}"; + } + StringBuilder response = new StringBuilder(); // Add error ID and general error message From 1fba289d96b6bef730a83b66e272041eb846f798 Mon Sep 17 00:00:00 2001 From: Pavan Manish Date: Tue, 3 Dec 2024 16:43:05 +0530 Subject: [PATCH 4/4] FT-800: Use AtlasType.toJson() for JSON serialization --- .../atlas/web/errors/ExceptionMapperUtil.java | 69 ++++++++----------- 1 file changed, 30 insertions(+), 39 deletions(-) diff --git a/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java b/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java index 03df167d5d..f10029266a 100644 --- a/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java +++ b/webapp/src/main/java/org/apache/atlas/web/errors/ExceptionMapperUtil.java @@ -17,10 +17,13 @@ */ package org.apache.atlas.web.errors; +import org.apache.atlas.type.AtlasType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.HashMap; public class ExceptionMapperUtil { protected static final Logger LOGGER = LoggerFactory.getLogger(ExceptionMapperUtil.class); @@ -29,28 +32,31 @@ public class ExceptionMapperUtil { protected static String formatErrorMessage(long id, Exception exception) { if (exception == null) { // If the exception is null, return a minimal error message - return "{\n" - + String.format(" \"errorId\": \"%016x\",\n", id) - + " \"message\": \"No exception provided\",\n" - + " \"causes\": []\n" - + "}"; + Map errorDetails = new HashMap<>(); + errorDetails.put("errorId", String.format("%016x", id)); + errorDetails.put("message", "No exception provided."); + errorDetails.put("causes", new ArrayList<>()); + return AtlasType.toJson(errorDetails); } - StringBuilder response = new StringBuilder(); + // Prepare data for error message + Map errorDetails = new HashMap<>(); + errorDetails.put("errorId", String.format("%016x", id)); + errorDetails.put("message", "There was an error processing your request."); - // Add error ID and general error message - response.append("{\n") - .append(String.format(" \"errorId\": \"%016x\",\n", id)) - .append(" \"message\": \"There was an error processing your request.\",\n") - .append(" \"causes\": [\n"); - - // Traverse through the chain of causes, avoiding cycles - List causes = new ArrayList<>(); + // Create a list of causes + List> causes = new ArrayList<>(); List visited = new ArrayList<>(); Throwable currentException = exception; + while (currentException != null) { if (visited.contains(currentException)) { - causes.add(" {\n \"errorType\": \"CircularReferenceDetected\",\n \"errorMessage\": \"A circular reference was detected in the exception chain.\",\n \"location\": \"Unavailable\"\n }"); + // If circular reference detected, add special entry + Map circularCause = new HashMap<>(); + circularCause.put("errorType", "CircularReferenceDetected"); + circularCause.put("errorMessage", "A circular reference was detected in the exception chain."); + circularCause.put("location", "Unavailable"); + causes.add(circularCause); break; } visited.add(currentException); @@ -58,24 +64,14 @@ protected static String formatErrorMessage(long id, Exception exception) { currentException = currentException.getCause(); } - // Add all formatted causes to the response - for (int i = 0; i < causes.size(); i++) { - response.append(causes.get(i)); - if (i < causes.size() - 1) { - response.append(",\n"); - } - } - - // Close the JSON structure - response.append("\n ]\n") - .append("}"); + errorDetails.put("causes", causes); - return response.toString(); + return AtlasType.toJson(errorDetails); } // Helper method to format a single exception cause - private static String formatCause(Throwable exception) { - StringBuilder cause = new StringBuilder(); + private static Map formatCause(Throwable exception) { + Map cause = new HashMap<>(); // Extract location details from the first stack trace element StackTraceElement[] stackTrace = exception.getStackTrace(); @@ -89,18 +85,14 @@ private static String formatCause(Throwable exception) { element.getLineNumber()); } - // Build JSON object for this cause - cause.append(" {\n") - .append(" \"errorType\": \"").append(exception.getClass().getName()).append("\",\n") - .append(" \"errorMessage\": \"").append(exception.getMessage() != null ? exception.getMessage() : "No additional information provided").append("\",\n") - .append(" \"location\": \"").append(location).append("\"\n") - .append(" }"); + // Populate the cause map + cause.put("errorType", exception.getClass().getName()); + cause.put("errorMessage", exception.getMessage() != null ? exception.getMessage() : "No additional information provided"); + cause.put("location", location); - return cause.toString(); + return cause; } - - protected static void logException(long id, Exception exception) { LOGGER.error(formatLogMessage(id, exception), exception); } @@ -109,5 +101,4 @@ protected static void logException(long id, Exception exception) { protected static String formatLogMessage(long id, Throwable exception) { return String.format("Error handling a request: %016x", id); } - }