Jackie-Jiang commented on a change in pull request #7460:
URL: https://github.com/apache/pinot/pull/7460#discussion_r713499775
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##########
@@ -144,17 +153,29 @@ public static ProcessingException
getException(ProcessingException processingExc
return copiedProcessingException;
}
- public static String getTruncatedStackTrace(Exception exception) {
+ public static String getTruncatedStackTrace(Throwable exception) {
StringWriter stringWriter = new StringWriter();
exception.printStackTrace(new PrintWriter(stringWriter));
String fullStackTrace = stringWriter.toString();
String[] lines = fullStackTrace.split("\n");
- int numLinesOfStackTrace = Math.min(lines.length, _maxLinesOfStackTrace);
- int lengthOfStackTrace = numLinesOfStackTrace - 1;
- for (int i = 0; i < numLinesOfStackTrace; i++) {
- lengthOfStackTrace += lines[i].length();
+ // exception should at least have one line, no need to check here.
+ StringBuilder sb = new StringBuilder(lines[0]);
+ int lengthOfStackTrace = 1;
Review comment:
(nit)
```suggestion
int numLinesOfStackTrace = 1;
```
##########
File path:
pinot-common/src/test/java/org/apache/pinot/common/exception/QueryExceptionTest.java
##########
@@ -0,0 +1,32 @@
+package org.apache.pinot.common.exception;
+
+import org.apache.pinot.common.response.ProcessingException;
+import org.testng.annotations.Test;
+
+import static org.testng.AssertJUnit.assertEquals;
+
+
+public class QueryExceptionTest {
+
+ @Test
+ public void testExceptionMessage() throws Exception {
+ Exception exception = new UnsupportedOperationException("Caught
exception.");
+ ProcessingException processingException =
+ QueryException.getException(QueryException.QUERY_EXECUTION_ERROR,
exception);
+ // should match QueryException._maxLinesOfStackTrace + 1 lines for the top
level wrapper.
+ assertEquals(5 + 1, processingException.getMessage().split("\n").length);
+
+ Exception withSuppressedException = new IllegalStateException("Suppressed
exception");
+ withSuppressedException.addSuppressed(processingException);
+ ProcessingException withSuppressedProcessingException =
+ QueryException.getException(QueryException.QUERY_EXECUTION_ERROR,
withSuppressedException);
+ // should match QueryException._maxLinesOfStackTrace * 2 + 1 line
separator + 1 lines for the top level wrapper.
Review comment:
(nit) line separator or omitted signal?
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##########
@@ -144,17 +153,29 @@ public static ProcessingException
getException(ProcessingException processingExc
return copiedProcessingException;
}
- public static String getTruncatedStackTrace(Exception exception) {
+ public static String getTruncatedStackTrace(Throwable exception) {
StringWriter stringWriter = new StringWriter();
exception.printStackTrace(new PrintWriter(stringWriter));
String fullStackTrace = stringWriter.toString();
String[] lines = fullStackTrace.split("\n");
- int numLinesOfStackTrace = Math.min(lines.length, _maxLinesOfStackTrace);
- int lengthOfStackTrace = numLinesOfStackTrace - 1;
- for (int i = 0; i < numLinesOfStackTrace; i++) {
- lengthOfStackTrace += lines[i].length();
+ // exception should at least have one line, no need to check here.
+ StringBuilder sb = new StringBuilder(lines[0]);
+ int lengthOfStackTrace = 1;
+ for (int i = 1; i < lines.length; i++) {
+ if (CAUSE_CAPTION_REGEXP.matcher(lines[i]).find() ||
SUPPRESSED_CAPTION_REGEXP.matcher(lines[i]).find()) {
+ // reset stack trace print counter when a new cause or suppressed
Throwable were found.
+ if (lengthOfStackTrace >= _maxLinesOfStackTrace) {
+ sb.append("\n").append(OMITTED_SIGNAL);
Review comment:
(nit) Append char should be faster, same for other places
```suggestion
sb.append('\n').append(OMITTED_SIGNAL);
```
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##########
@@ -144,17 +153,29 @@ public static ProcessingException
getException(ProcessingException processingExc
return copiedProcessingException;
}
- public static String getTruncatedStackTrace(Exception exception) {
+ public static String getTruncatedStackTrace(Throwable exception) {
StringWriter stringWriter = new StringWriter();
exception.printStackTrace(new PrintWriter(stringWriter));
String fullStackTrace = stringWriter.toString();
String[] lines = fullStackTrace.split("\n");
Review comment:
(nit) `StringUtils.split(fullStackTrace, '\n')` will be faster because
no regex matching required
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
