Clean up syntax error reporting

common/src/main/java/org/opensearch/sql/common/antlr/SyntaxAnalysisErrorListener.java

@@ -5,19 +5,21 @@
 
 package org.opensearch.sql.common.antlr;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Locale;
-import org.antlr.v4.runtime.BaseErrorListener;
-import org.antlr.v4.runtime.CommonTokenStream;
-import org.antlr.v4.runtime.RecognitionException;
-import org.antlr.v4.runtime.Recognizer;
-import org.antlr.v4.runtime.Token;
+import org.antlr.v4.runtime.*;
 import org.antlr.v4.runtime.misc.IntervalSet;
 
 /**
  * Syntax analysis error listener that handles any syntax error by throwing exception with useful
  * information.
  */
 public class SyntaxAnalysisErrorListener extends BaseErrorListener {
+  // Show up to this many characters before the offending token in the query.
+  private static final int CONTEXT_TRUNCATION_THRESHOLD = 20;
+  // Avoid presenting too many alternatives when many are available.
+  private static final int SUGGESTION_TRUNCATION_THRESHOLD = 5;
 
   @Override
   public void syntaxError(
@@ -35,8 +37,7 @@ public void syntaxError(
     throw new SyntaxCheckException(
         String.format(
             Locale.ROOT,
-            "Failed to parse query due to offending symbol [%s] "
-                + "at: '%s' <--- HERE... More details: %s",
+            "[%s] is not a valid term at this part of the query: '%s' <-- HERE. %s",
             getOffendingText(offendingToken),
             truncateQueryAtOffendingToken(query, offendingToken),
             getDetails(recognizer, msg, e)));
@@ -47,21 +48,41 @@ private String getOffendingText(Token offendingToken) {
   }
 
   private String truncateQueryAtOffendingToken(String query, Token offendingToken) {
-    return query.substring(0, offendingToken.getStopIndex() + 1);
+    int contextStartIndex = offendingToken.getStartIndex() - CONTEXT_TRUNCATION_THRESHOLD;
+    if (contextStartIndex < 3) { // The ellipses won't save us anything below the first 4 characters
+      return query.substring(0, offendingToken.getStopIndex() + 1);
+    }
+    return "..." + query.substring(contextStartIndex, offendingToken.getStopIndex() + 1);
   }
 
-  /**
-   * As official JavaDoc says, e=null means parser was able to recover from the error. In other
-   * words, "msg" argument includes the information we want.
-   */
   private String getDetails(Recognizer<?, ?> recognizer, String msg, RecognitionException e) {
-    String details;
     if (e == null) {
-      details = msg;
+      // According to the ANTLR docs, e == null means the parser was able to recover from the error.
+      // In such cases, `msg` includes the raw error information we care about.
+      return msg;
+    }
+
+    IntervalSet followSet = e.getExpectedTokens();
+    Vocabulary vocab = recognizer.getVocabulary();
+    List<String> tokenNames = new ArrayList<>(SUGGESTION_TRUNCATION_THRESHOLD);
+    for (int tokenType :
+        followSet
+            .toList()
+            .subList(0, Math.min(followSet.size(), SUGGESTION_TRUNCATION_THRESHOLD))) {
+      tokenNames.add(vocab.getDisplayName(tokenType));
+    }
+
+    StringBuilder details = new StringBuilder("Expecting ");
+    if (followSet.size() > SUGGESTION_TRUNCATION_THRESHOLD) {
+      details
+          .append("one of ")
+          .append(tokenNames.size())
+          .append(" possible tokens. Some examples: ")
+          .append(String.join(", ", tokenNames))
+          .append(", ...");
     } else {
-      IntervalSet followSet = e.getExpectedTokens();
-      details = "Expecting tokens in " + followSet.toString(recognizer.getVocabulary());
+      details.append("tokens: ").append(String.join(", ", tokenNames));
     }
-    return details;
+    return details.toString();
   }
 }

docs/user/dql/troubleshooting.rst

@@ -25,22 +25,22 @@ Query:
 
 .. code-block:: JSON
 
-	POST /_plugins/_sql
+	POST /_plugins/_ppl
 	{
-	  "query" : "SELECT * FROM sample:data"
+	  "query" : "SOURCE = test_index | where a > 0)"
 	}
 
 Result:
 
 .. code-block:: JSON
 
     {
-      "reason": "Invalid SQL query",
-      "details": "Failed to parse query due to offending symbol [:] at: 'SELECT * FROM xxx WHERE xxx:' <--- HERE...
-        More details: Expecting tokens in {<EOF>, 'AND', 'BETWEEN', 'GROUP', 'HAVING', 'IN', 'IS', 'LIKE', 'LIMIT',
-        'NOT', 'OR', 'ORDER', 'REGEXP', '*', '/', '%', '+', '-', 'DIV', 'MOD', '=', '>', '<', '!',
-        '|', '&', '^', '.', DOT_ID}",
-      "type": "SyntaxAnalysisException"
+      "error": {
+        "reason": "Invalid Query",
+        "details": "[)] is not a valid term at this part of the query: '..._index | where a > 0)' <-- HERE. extraneous input ')' expecting <EOF>",
+        "type": "SyntaxCheckException"
+      },
+      "status": 400
     }
 
 **Workaround**

integ-test/src/test/java/org/opensearch/sql/ppl/DescribeCommandIT.java

@@ -80,7 +80,7 @@ public void describeCommandWithoutIndexShouldFailToParse() throws IOException {
       fail();
     } catch (ResponseException e) {
       assertTrue(e.getMessage().contains("RuntimeException"));
-      assertTrue(e.getMessage().contains("Failed to parse query due to offending symbol"));
+      assertTrue(e.getMessage().contains("is not a valid term at this part of the query"));
     }
   }
 }

integ-test/src/test/java/org/opensearch/sql/ppl/QueryAnalysisIT.java

@@ -82,25 +82,25 @@ public void queryShouldBeCaseInsensitiveInKeywords() {
   @Test
   public void queryNotStartingWithSearchCommandShouldFailSyntaxCheck() {
     String query = "fields firstname";
-    queryShouldThrowSyntaxException(query, "Failed to parse query due to offending symbol");
+    queryShouldThrowSyntaxException(query, "is not a valid term at this part of the query");
   }
 
   @Test
   public void queryWithIncorrectCommandShouldFailSyntaxCheck() {
     String query = String.format("search source=%s | field firstname", TEST_INDEX_ACCOUNT);
-    queryShouldThrowSyntaxException(query, "Failed to parse query due to offending symbol");
+    queryShouldThrowSyntaxException(query, "is not a valid term at this part of the query");
   }
 
   @Test
   public void queryWithIncorrectKeywordsShouldFailSyntaxCheck() {
     String query = String.format("search sources=%s", TEST_INDEX_ACCOUNT);
-    queryShouldThrowSyntaxException(query, "Failed to parse query due to offending symbol");
+    queryShouldThrowSyntaxException(query, "is not a valid term at this part of the query");
   }
 
   @Test
   public void unsupportedAggregationFunctionShouldFailSyntaxCheck() {
     String query = String.format("search source=%s | stats range(age)", TEST_INDEX_ACCOUNT);
-    queryShouldThrowSyntaxException(query, "Failed to parse query due to offending symbol");
+    queryShouldThrowSyntaxException(query, "is not a valid term at this part of the query");
   }
 
   /** Commands that fail semantic analysis should throw {@link SemanticCheckException}. */

integ-test/src/test/java/org/opensearch/sql/ppl/SearchCommandIT.java

@@ -64,7 +64,7 @@ public void searchCommandWithoutSourceShouldFailToParse() throws IOException {
       fail();
     } catch (ResponseException e) {
       assertTrue(e.getMessage().contains("RuntimeException"));
-      assertTrue(e.getMessage().contains("Failed to parse query due to offending symbol"));
+      assertTrue(e.getMessage().contains("is not a valid term at this part of the query"));
     }
   }
 }

ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java

@@ -95,7 +95,7 @@ public void testSearchFieldsCommandCrossClusterShouldPass() {
   @Test
   public void testSearchCommandWithoutSourceShouldFail() {
     exceptionRule.expect(RuntimeException.class);
-    exceptionRule.expectMessage("Failed to parse query due to offending symbol");
+    exceptionRule.expectMessage("is not a valid term at this part of the query");
 
     new PPLSyntaxParser().parse("search a=1");
   }
@@ -337,7 +337,7 @@ public void testDescribeFieldsCommandShouldPass() {
   @Test
   public void testDescribeCommandWithSourceShouldFail() {
     exceptionRule.expect(RuntimeException.class);
-    exceptionRule.expectMessage("Failed to parse query due to offending symbol");
+    exceptionRule.expectMessage("is not a valid term at this part of the query");
 
     new PPLSyntaxParser().parse("describe source=t");
   }