From 013812082449a121cffba34a89b3cc7b71def71a Mon Sep 17 00:00:00 2001 From: Laura Schanno Date: Fri, 31 May 2024 09:48:31 -0400 Subject: [PATCH 1/3] Ensure CompositeQueryLogic error codes are sent to metrics The query metrics API expects QueryExceptions to be found in the cause of the original exception passed to BaseQueryMetric.setError(Throwable). In the case of CompositeQueryLogic, due to the additional wrapping of exceptions passed from its defined internal logics in a CompositeLogicException, query exceptions were not in the expected place in the stack hierarchy. Update the CompositeLogicException to identify any QueryExceptions present in the stacks of exception arguments, and ensure that they are either already a QueryException, or that if they contain a QueryException in the stack, that they are subsequently wrapped in a CompositeRaisedQueryException with a copy of the desired error code. Fixes #2340 --- .../composite/CompositeLogicException.java | 86 ++++++++++++--- .../CompositeRaisedQueryException.java | 16 +++ .../CompositeLogicExceptionTest.java | 100 ++++++++++++++++++ 3 files changed, 187 insertions(+), 15 deletions(-) create mode 100644 core/query/src/main/java/datawave/core/query/logic/composite/CompositeRaisedQueryException.java create mode 100644 core/query/src/test/java/datawave/core/query/logic/composite/CompositeLogicExceptionTest.java diff --git a/core/query/src/main/java/datawave/core/query/logic/composite/CompositeLogicException.java b/core/query/src/main/java/datawave/core/query/logic/composite/CompositeLogicException.java index 9d8b153ac8b..fe9b7cf49c8 100644 --- a/core/query/src/main/java/datawave/core/query/logic/composite/CompositeLogicException.java +++ b/core/query/src/main/java/datawave/core/query/logic/composite/CompositeLogicException.java @@ -7,38 +7,94 @@ import datawave.webservice.query.exception.QueryException; public class CompositeLogicException extends RuntimeException { + public CompositeLogicException(String message, String logicName, Exception exception) { - super(getMessage(message, Collections.singletonMap(logicName, exception)), exception); + super(getMessage(message, Collections.singletonMap(logicName, exception)), getRaisedQueryException(exception)); } public CompositeLogicException(String message, Map exceptions) { - super(getMessage(message, exceptions), getQueryException(exceptions.values())); + super(getMessage(message, exceptions), getCause(exceptions.values())); if (exceptions.size() > 1) { exceptions.values().forEach(this::addSuppressed); } } - // looking for an exception that has a nested QueryException such that we may return an error code - private static Exception getQueryException(Collection exceptions) { + /** + * Return the cause to use, prioritizing the first {@link QueryException} instance that we see. In the case where the {@link QueryException} is found to be + * the cause or further nested in the stack of an {@link Exception}, a {@link CompositeRaisedQueryException} will be returned with the query exception's + * error code, and the original exception as the cause. This is necessary to ensure the error code is passed to query metrics. + */ + private static Exception getCause(Collection exceptions) { if (exceptions.size() == 1) { return exceptions.iterator().next(); } - Exception e = null; - for (Exception test : exceptions) { - if (e == null) { - e = test; - } else if (isQueryException(test)) { - e = test; + Exception cause = null; + for (Exception exception : exceptions) { + // Establish the initial cause as the first seen exception. + if (cause == null) { + cause = getRaisedQueryException(exception); + // If the first cause we see is a QueryException, there's nothing further to do. + if (cause instanceof QueryException) { + return cause; + } + // If a subsequent exception is a or contains a QueryException in its stack, return it with the query exception error code available at the root + // exception. + } else if (hasQueryExceptionInStack(exception)) { + return getRaisedQueryException(exception); } - if (isQueryException(e)) { - break; + } + return cause; + } + + /** + * Return whether the given throwable contains at least one {@link QueryException} in its stack trace (including itself). + */ + private static boolean hasQueryExceptionInStack(Throwable throwable) { + return getFirstQueryExceptionInStack(throwable) != null; + } + + /** + * Return the given exception with query exception's error code (if present) available at the root exception. This means one of the following cases will + * occur: + *
  • + *
      + * The exception is not a {@link QueryException} and no {@link QueryException} exists in the exception's stack: The exception will be returned. + *
    + *
      + * The exception is a {@link QueryException}: The exception will be returned. + *
    + *
      + * The exception is not a {@link QueryException}, but a {@link QueryException} exists in the exception's stack. A {@link CompositeRaisedQueryException} will + * be returned with the error code of the first {@link QueryException} found in the stack, and the original exception as its cause. + *
    + *
    • + */ + private static Exception getRaisedQueryException(Exception exception) { + if (exception instanceof QueryException) { + return exception; + } else { + // TODO - should we fetch the top-most or bottom-most query exception in the stack? + QueryException queryException = getFirstQueryExceptionInStack(exception); + if (queryException != null) { + return new CompositeRaisedQueryException(exception, queryException.getErrorCode()); + } else { + return exception; } } - return e; } - private static boolean isQueryException(Exception e) { - return new QueryException(e).getQueryExceptionsInStack().size() > 1; + /** + * Return the first {@link QueryException} found in the stack, or null if none were found. + */ + private static QueryException getFirstQueryExceptionInStack(Throwable throwable) { + if (throwable != null) { + if (throwable instanceof QueryException) { + return (QueryException) throwable; + } else { + return getFirstQueryExceptionInStack(throwable.getCause()); + } + } + return null; } private static String getMessage(String message, Map exceptions) { diff --git a/core/query/src/main/java/datawave/core/query/logic/composite/CompositeRaisedQueryException.java b/core/query/src/main/java/datawave/core/query/logic/composite/CompositeRaisedQueryException.java new file mode 100644 index 00000000000..52bdac0455c --- /dev/null +++ b/core/query/src/main/java/datawave/core/query/logic/composite/CompositeRaisedQueryException.java @@ -0,0 +1,16 @@ +package datawave.core.query.logic.composite; + +import datawave.webservice.query.exception.QueryException; + +/** + * This class exists to be used when a {@link CompositeLogicException} has a cause that is not a {@link QueryException}, but contains a {@link QueryException} + * in its stack trace. In order for the error code to be properly passed to query metrics, the error code must be present as part of the + * {@link CompositeLogicException}'s cause. This exception is intended to be a wrapper for the original cause, with the error code of the identified query + * exception. + */ +public class CompositeRaisedQueryException extends QueryException { + + public CompositeRaisedQueryException(Throwable cause, String errorCode) { + super(cause, errorCode); + } +} diff --git a/core/query/src/test/java/datawave/core/query/logic/composite/CompositeLogicExceptionTest.java b/core/query/src/test/java/datawave/core/query/logic/composite/CompositeLogicExceptionTest.java new file mode 100644 index 00000000000..2e5dd88af2f --- /dev/null +++ b/core/query/src/test/java/datawave/core/query/logic/composite/CompositeLogicExceptionTest.java @@ -0,0 +1,100 @@ +package datawave.core.query.logic.composite; + +import static org.junit.Assert.assertEquals; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.junit.Test; + +import datawave.webservice.query.exception.DatawaveErrorCode; +import datawave.webservice.query.exception.QueryException; + +public class CompositeLogicExceptionTest { + + @Test + public void testSingleNonQueryExceptionCause() { + IllegalArgumentException cause = new IllegalArgumentException("illegal argument"); + CompositeLogicException exception = new CompositeLogicException("composite error occurred", "LogicName", cause); + assertEquals("composite error occurred:\nLogicName: illegal argument", exception.getMessage()); + assertEquals(cause, exception.getCause()); + } + + @Test + public void testSingleQueryExceptionCause() { + QueryException cause = new QueryException(DatawaveErrorCode.MODEL_FETCH_ERROR, "connection failed"); + CompositeLogicException exception = new CompositeLogicException("composite error occurred", "LogicName", cause); + + assertEquals("composite error occurred:\nLogicName: Could not get model. connection failed", exception.getMessage()); + assertEquals(cause, exception.getCause()); + assertEquals(DatawaveErrorCode.MODEL_FETCH_ERROR.getErrorCode(), ((QueryException) exception.getCause()).getErrorCode()); + } + + @Test + public void testNestedSingleQueryExceptionCause() { + QueryException nestedCause = new QueryException(DatawaveErrorCode.MODEL_FETCH_ERROR, "connection failed"); + IllegalArgumentException cause = new IllegalArgumentException("illegal argument", nestedCause); + CompositeLogicException exception = new CompositeLogicException("composite error occurred", "LogicName", cause); + assertEquals("composite error occurred:\nLogicName: illegal argument", exception.getMessage()); + assertEquals(CompositeRaisedQueryException.class, exception.getCause().getClass()); + assertEquals(DatawaveErrorCode.MODEL_FETCH_ERROR.getErrorCode(), ((CompositeRaisedQueryException) exception.getCause()).getErrorCode()); + } + + @Test + public void testMultipleNonQueryExceptionCauses() { + IllegalArgumentException expectedCause = new IllegalArgumentException("illegal name"); + Map exceptions = new LinkedHashMap<>(); + exceptions.put("logic1", expectedCause); + exceptions.put("logic2", new NullPointerException("null value")); + exceptions.put("logic3", new IllegalStateException("bad state")); + + CompositeLogicException exception = new CompositeLogicException("failed to complete", exceptions); + assertEquals("failed to complete:\nlogic1: illegal name\nlogic2: null value\nlogic3: bad state", exception.getMessage()); + assertEquals(expectedCause, exception.getCause()); + } + + @Test + public void testMultipleExceptionWithOneTopLevelQueryException() { + QueryException expectedCause = new QueryException(DatawaveErrorCode.MODEL_FETCH_ERROR, "connection failed"); + Map exceptions = new LinkedHashMap<>(); + exceptions.put("logic1", new IllegalArgumentException("illegal name")); + exceptions.put("logic2", new NullPointerException("null value")); + exceptions.put("logic3", expectedCause); + exceptions.put("logic4", new IllegalStateException("bad state")); + + CompositeLogicException exception = new CompositeLogicException("failed to complete", exceptions); + assertEquals("failed to complete:\nlogic1: illegal name\nlogic2: null value\nlogic3: Could not get model. connection failed\nlogic4: bad state", + exception.getMessage()); + assertEquals(expectedCause, exception.getCause()); + } + + @Test + public void testMultipleExceptionWithOneNestedQueryException() { + QueryException nestedCause = new QueryException(DatawaveErrorCode.MODEL_FETCH_ERROR, "connection failed"); + IllegalStateException topCause = new IllegalStateException("bad state", nestedCause); + Map exceptions = new LinkedHashMap<>(); + exceptions.put("logic1", new IllegalArgumentException("illegal name")); + exceptions.put("logic2", topCause); + exceptions.put("logic3", new NullPointerException("null value")); + + CompositeLogicException exception = new CompositeLogicException("failed to complete", exceptions); + assertEquals("failed to complete:\nlogic1: illegal name\nlogic2: bad state\nlogic3: null value", exception.getMessage()); + assertEquals(CompositeRaisedQueryException.class, exception.getCause().getClass()); + assertEquals(DatawaveErrorCode.MODEL_FETCH_ERROR.getErrorCode(), ((CompositeRaisedQueryException) exception.getCause()).getErrorCode()); + } + + @Test + public void testMultipleExceptionWithNestedQueryExceptionSeenFirst() { + QueryException nestedCause = new QueryException(DatawaveErrorCode.MODEL_FETCH_ERROR, "connection failed"); + IllegalStateException topCause = new IllegalStateException("bad state", nestedCause); + Map exceptions = new LinkedHashMap<>(); + exceptions.put("logic1", topCause); + exceptions.put("logic2", new IllegalArgumentException("illegal name")); + exceptions.put("logic3", new NullPointerException("null value")); + + CompositeLogicException exception = new CompositeLogicException("failed to complete", exceptions); + assertEquals("failed to complete:\nlogic1: bad state\nlogic2: illegal name\nlogic3: null value", exception.getMessage()); + assertEquals(CompositeRaisedQueryException.class, exception.getCause().getClass()); + assertEquals(DatawaveErrorCode.MODEL_FETCH_ERROR.getErrorCode(), ((CompositeRaisedQueryException) exception.getCause()).getErrorCode()); + } +} From 162b78b822c2675411a766ec76f432dc2e7e2876 Mon Sep 17 00:00:00 2001 From: Laura Schanno Date: Tue, 4 Jun 2024 09:36:49 -0400 Subject: [PATCH 2/3] Update unit test --- .../query/logic/composite/CompositeQueryLogicTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web-services/query/src/test/java/datawave/webservice/query/logic/composite/CompositeQueryLogicTest.java b/web-services/query/src/test/java/datawave/webservice/query/logic/composite/CompositeQueryLogicTest.java index 985609f1c48..fddf3b70480 100644 --- a/web-services/query/src/test/java/datawave/webservice/query/logic/composite/CompositeQueryLogicTest.java +++ b/web-services/query/src/test/java/datawave/webservice/query/logic/composite/CompositeQueryLogicTest.java @@ -805,7 +805,7 @@ public GenericQueryConfiguration initialize(AccumuloClient connection, Query set c.getTransformer(settings); } catch (CompositeLogicException e) { - Assert.assertEquals("query initialize failed", e.getCause().getCause().getMessage()); + Assert.assertEquals("datawave.webservice.query.exception.QueryException: query initialize failed", e.getCause().getCause().getMessage()); } } From 0b21bed3c0db4d7984866c9d663e52924d071f56 Mon Sep 17 00:00:00 2001 From: Laura Schanno Date: Tue, 4 Jun 2024 10:27:38 -0400 Subject: [PATCH 3/3] Fix javadoc --- .../logic/composite/CompositeLogicException.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/core/query/src/main/java/datawave/core/query/logic/composite/CompositeLogicException.java b/core/query/src/main/java/datawave/core/query/logic/composite/CompositeLogicException.java index fe9b7cf49c8..97db9d0f61c 100644 --- a/core/query/src/main/java/datawave/core/query/logic/composite/CompositeLogicException.java +++ b/core/query/src/main/java/datawave/core/query/logic/composite/CompositeLogicException.java @@ -56,18 +56,12 @@ private static boolean hasQueryExceptionInStack(Throwable throwable) { /** * Return the given exception with query exception's error code (if present) available at the root exception. This means one of the following cases will * occur: - *
  • *
      - * The exception is not a {@link QueryException} and no {@link QueryException} exists in the exception's stack: The exception will be returned. + *
    • The exception is not a {@link QueryException} and no {@link QueryException} exists in the exception's stack: The exception will be returned.
      • + *
    • The exception is a {@link QueryException}: The exception will be returned.
      • + *
    • The exception is not a {@link QueryException}, but a {@link QueryException} exists in the exception's stack. A {@link CompositeRaisedQueryException} + * will be returned with the error code of the first {@link QueryException} found in the stack, and the original exception as its cause.
      • *
    - *
      - * The exception is a {@link QueryException}: The exception will be returned. - *
    - *
      - * The exception is not a {@link QueryException}, but a {@link QueryException} exists in the exception's stack. A {@link CompositeRaisedQueryException} will - * be returned with the error code of the first {@link QueryException} found in the stack, and the original exception as its cause. - *
    - *
    • */ private static Exception getRaisedQueryException(Exception exception) { if (exception instanceof QueryException) {