This is an automated email from the ASF dual-hosted git repository.
maytasm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 5417aa2055 Fix: ParseException swallow cause Exception (#12810)
5417aa2055 is described below
commit 5417aa2055b3abb4b4515f9de81397a19e5ebc73
Author: Maytas Monsereenusorn <[email protected]>
AuthorDate: Fri Jul 22 13:46:28 2022 -0700
Fix: ParseException swallow cause Exception (#12810)
* add impl
* add impl
* fix checkstyle
---
.../java/util/common/parsers/ParseException.java | 20 ++++---
.../FilteringCloseableInputRowIteratorTest.java | 69 +++++++++++++++++++++-
.../segment/incremental/ParseExceptionHandler.java | 13 +++-
3 files changed, 87 insertions(+), 15 deletions(-)
diff --git
a/core/src/main/java/org/apache/druid/java/util/common/parsers/ParseException.java
b/core/src/main/java/org/apache/druid/java/util/common/parsers/ParseException.java
index fc9877de5b..e72c89518b 100644
---
a/core/src/main/java/org/apache/druid/java/util/common/parsers/ParseException.java
+++
b/core/src/main/java/org/apache/druid/java/util/common/parsers/ParseException.java
@@ -57,23 +57,25 @@ public class ParseException extends RuntimeException
public ParseException(@Nullable String input, String formatText, Object...
arguments)
{
- super(StringUtils.nonStrictFormat(formatText, arguments));
- this.input = input;
- this.fromPartiallyValidRow = false;
- this.timeOfExceptionMillis = System.currentTimeMillis();
+ this(input, false, formatText, arguments);
}
public ParseException(@Nullable String input, boolean fromPartiallyValidRow,
String formatText, Object... arguments)
{
- super(StringUtils.nonStrictFormat(formatText, arguments));
- this.input = input;
- this.fromPartiallyValidRow = fromPartiallyValidRow;
- this.timeOfExceptionMillis = System.currentTimeMillis();
+ this(input, fromPartiallyValidRow, null, formatText, arguments);
}
public ParseException(@Nullable String input, Throwable cause, String
formatText, Object... arguments)
{
- this(input, false, StringUtils.nonStrictFormat(formatText, arguments),
cause);
+ this(input, false, cause, formatText, arguments);
+ }
+
+ public ParseException(@Nullable String input, boolean fromPartiallyValidRow,
Throwable cause, String formatText, Object... arguments)
+ {
+ super(StringUtils.nonStrictFormat(formatText, arguments), cause);
+ this.input = input;
+ this.fromPartiallyValidRow = fromPartiallyValidRow;
+ this.timeOfExceptionMillis = System.currentTimeMillis();
}
public boolean isFromPartiallyValidRow()
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/FilteringCloseableInputRowIteratorTest.java
b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/FilteringCloseableInputRowIteratorTest.java
index 844021520a..a0b1c101d4 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/FilteringCloseableInputRowIteratorTest.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/FilteringCloseableInputRowIteratorTest.java
@@ -35,6 +35,8 @@ import org.joda.time.DateTime;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
import java.io.IOException;
import java.util.ArrayList;
@@ -62,12 +64,12 @@ public class FilteringCloseableInputRowIteratorTest
public void setup()
{
rowIngestionMeters = new SimpleRowIngestionMeters();
- parseExceptionHandler = new ParseExceptionHandler(
+ parseExceptionHandler = Mockito.spy(new ParseExceptionHandler(
rowIngestionMeters,
- false,
+ true,
Integer.MAX_VALUE,
1024 // do not use Integer.MAX_VALUE since it will create an object
array of this length
- );
+ ));
}
@Test
@@ -287,6 +289,67 @@ public class FilteringCloseableInputRowIteratorTest
Assert.assertTrue(closed.isTrue());
}
+ @Test
+ public void testParseExceptionSaveExceptionCause()
+ {
+
+ // This iterator throws ParseException every other call to hasNext().
+ final CloseableIterator<InputRow> parseExceptionThrowingIterator = new
CloseableIterator<InputRow>()
+ {
+ final int numRowsToIterate = ROWS.size() * 2;
+ int currentIndex = 0;
+ int nextIndex = 0;
+
+ @Override
+ public boolean hasNext()
+ {
+ currentIndex = nextIndex++;
+ if (currentIndex % 2 == 0) {
+ return currentIndex < numRowsToIterate;
+ } else {
+ try {
+ throw new IllegalArgumentException("this is the root cause of the
exception!");
+ }
+ catch (Exception e) {
+ throw new ParseException(null, e, "Parse exception at [%d]",
currentIndex);
+ }
+ }
+ }
+
+ @Override
+ public InputRow next()
+ {
+ return ROWS.get(currentIndex / 2);
+ }
+
+ @Override
+ public void close()
+ {
+ }
+ };
+
+ final FilteringCloseableInputRowIterator rowIterator = new
FilteringCloseableInputRowIterator(
+ parseExceptionThrowingIterator,
+ row -> true,
+ rowIngestionMeters,
+ parseExceptionHandler
+ );
+
+ final List<InputRow> filteredRows = new ArrayList<>();
+ rowIterator.forEachRemaining(filteredRows::add);
+ ArgumentCaptor<Exception> exceptionArgumentCaptor =
ArgumentCaptor.forClass(Exception.class);
+ Mockito.verify(parseExceptionHandler,
Mockito.times(6)).logParseExceptionHelper(exceptionArgumentCaptor.capture());
+ Exception parseException = exceptionArgumentCaptor.getValue();
+ Assert.assertTrue(parseException.getMessage().contains("Parse exception
at"));
+ Assert.assertNotNull(parseException.getCause());
+ Assert.assertTrue(parseException.getCause().getMessage().contains("this is
the root cause of the exception!"));
+ Assert.assertEquals(IllegalArgumentException.class,
parseException.getCause().getClass());
+
+ Assert.assertEquals(ROWS, filteredRows);
+ Assert.assertEquals(ROWS.size(), rowIngestionMeters.getUnparseable());
+ }
+
+
private static InputRow newRow(DateTime timestamp, Object dim1Val, Object
dim2Val)
{
return new MapBasedInputRow(timestamp, DIMENSIONS, ImmutableMap.of("dim1",
dim1Val, "dim2", dim2Val));
diff --git
a/processing/src/main/java/org/apache/druid/segment/incremental/ParseExceptionHandler.java
b/processing/src/main/java/org/apache/druid/segment/incremental/ParseExceptionHandler.java
index 18ecdd06fb..a67da71727 100644
---
a/processing/src/main/java/org/apache/druid/segment/incremental/ParseExceptionHandler.java
+++
b/processing/src/main/java/org/apache/druid/segment/incremental/ParseExceptionHandler.java
@@ -19,6 +19,7 @@
package org.apache.druid.segment.incremental;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import org.apache.druid.java.util.common.RE;
@@ -77,9 +78,7 @@ public class ParseExceptionHandler
rowIngestionMeters.incrementUnparseable();
}
- if (logParseExceptions) {
- LOG.error(e, "Encountered parse exception");
- }
+ logParseExceptionHelper(e);
if (savedParseExceptionReports != null) {
ParseExceptionReport parseExceptionReport = new ParseExceptionReport(
@@ -103,4 +102,12 @@ public class ParseExceptionHandler
{
return savedParseExceptionReports;
}
+
+ @VisibleForTesting
+ public void logParseExceptionHelper(Exception e)
+ {
+ if (logParseExceptions) {
+ LOG.error(e, "Encountered parse exception");
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
