Skip to content

Commit c4bcd3a

Browse files
bbakermanbbakerman
authored
This adds better syntax error handling (graphql-java#1348)
* This adds better syntax error handling * removed the ' ' quotes * PR feedback * Now with better error messages and offending token as a attribute * Moved to its own exception that then turns into a graphql error rather than having the error be an exception * Fixed up test * Fixed more tests
1 parent 0f5a890 commit c4bcd3a

10 files changed

Lines changed: 307 additions & 66 deletions

File tree

‎src/main/java/graphql/GraphQL.java‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@
2222
import graphql.execution.preparsed.PreparsedDocumentEntry;
2323
import graphql.execution.preparsed.PreparsedDocumentProvider;
2424
import graphql.language.Document;
25+
import graphql.parser.InvalidSyntaxException;
2526
import graphql.parser.Parser;
2627
import graphql.schema.GraphQLSchema;
2728
import graphql.validation.ValidationError;
2829
import graphql.validation.Validator;
29-
import org.antlr.v4.runtime.misc.ParseCancellationException;
3030
import org.slf4j.Logger;
3131
import org.slf4j.LoggerFactory;
3232

@@ -40,7 +40,6 @@
4040
import java.util.function.UnaryOperator;
4141

4242
import static graphql.Assert.assertNotNull;
43-
import static graphql.InvalidSyntaxError.toInvalidSyntaxError;
4443
import static graphql.execution.instrumentation.DocumentAndVariables.newDocumentAndVariables;
4544

4645
/**
@@ -529,8 +528,8 @@ private PreparsedDocumentEntry parseAndValidate(AtomicReference<ExecutionInput>
529528
log.debug("Parsing query: '{}'...", query);
530529
ParseResult parseResult = parse(executionInput, graphQLSchema, instrumentationState);
531530
if (parseResult.isFailure()) {
532-
log.warn("Query failed to parse : '{}'", query);
533-
return new PreparsedDocumentEntry(toInvalidSyntaxError(parseResult.getException()));
531+
log.warn("Query failed to parse : '{}'", executionInput.getQuery());
532+
return new PreparsedDocumentEntry(parseResult.getException().toInvalidSyntaxError());
534533
} else {
535534
final Document document = parseResult.getDocument();
536535
// they may have changed the document and the variables via instrumentation so update the reference to it
@@ -560,7 +559,7 @@ private ParseResult parse(ExecutionInput executionInput, GraphQLSchema graphQLSc
560559
documentAndVariables = newDocumentAndVariables()
561560
.document(document).variables(executionInput.getVariables()).build();
562561
documentAndVariables = instrumentation.instrumentDocumentAndVariables(documentAndVariables, parameters);
563-
} catch (ParseCancellationException e) {
562+
} catch (InvalidSyntaxException e) {
564563
parseInstrumentation.onCompleted(null, e);
565564
return ParseResult.ofError(e);
566565
}
@@ -606,9 +605,9 @@ private CompletableFuture<ExecutionResult> execute(ExecutionInput executionInput
606605

607606
private static class ParseResult {
608607
private final DocumentAndVariables documentAndVariables;
609-
private final Exception exception;
608+
private final InvalidSyntaxException exception;
610609

611-
private ParseResult(DocumentAndVariables documentAndVariables, Exception exception) {
610+
private ParseResult(DocumentAndVariables documentAndVariables, InvalidSyntaxException exception) {
612611
this.documentAndVariables = documentAndVariables;
613612
this.exception = exception;
614613
}
@@ -625,15 +624,15 @@ private Map<String, Object> getVariables() {
625624
return documentAndVariables.getVariables();
626625
}
627626

628-
private Exception getException() {
627+
private InvalidSyntaxException getException() {
629628
return exception;
630629
}
631630

632631
private static ParseResult of(DocumentAndVariables document) {
633632
return new ParseResult(document, null);
634633
}
635634

636-
private static ParseResult ofError(Exception e) {
635+
private static ParseResult ofError(InvalidSyntaxException e) {
637636
return new ParseResult(null, e);
638637
}
639638
}

‎src/main/java/graphql/InvalidSyntaxError.java‎

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,36 @@
22

33

44
import graphql.language.SourceLocation;
5-
import org.antlr.v4.runtime.RecognitionException;
65

76
import java.util.ArrayList;
87
import java.util.List;
98

9+
import static java.util.Collections.singletonList;
10+
1011
public class InvalidSyntaxError implements GraphQLError {
1112

1213
private final String message;
14+
private final String sourcePreview;
15+
private final String offendingToken;
1316
private final List<SourceLocation> locations = new ArrayList<>();
1417

1518
public InvalidSyntaxError(SourceLocation sourceLocation, String msg) {
16-
this.message = mkMessage(msg);
17-
if (sourceLocation != null) {
18-
this.locations.add(sourceLocation);
19-
}
19+
this(singletonList(sourceLocation), msg);
2020
}
2121

2222
public InvalidSyntaxError(List<SourceLocation> sourceLocations, String msg) {
23-
this.message = mkMessage(msg);
23+
this(sourceLocations, msg, null, null);
24+
}
25+
26+
public InvalidSyntaxError(List<SourceLocation> sourceLocations, String msg, String sourcePreview, String offendingToken) {
27+
this.message = msg;
28+
this.sourcePreview = sourcePreview;
29+
this.offendingToken = offendingToken;
2430
if (sourceLocations != null) {
2531
this.locations.addAll(sourceLocations);
2632
}
2733
}
2834

29-
private String mkMessage(String msg) {
30-
return "Invalid Syntax" + (msg == null ? "" : " : " + msg);
31-
}
32-
33-
3435
@Override
3536
public String getMessage() {
3637
return message;
@@ -41,6 +42,14 @@ public List<SourceLocation> getLocations() {
4142
return locations;
4243
}
4344

45+
public String getSourcePreview() {
46+
return sourcePreview;
47+
}
48+
49+
public String getOffendingToken() {
50+
return offendingToken;
51+
}
52+
4453
@Override
4554
public ErrorType getErrorType() {
4655
return ErrorType.InvalidSyntax;
@@ -50,30 +59,13 @@ public ErrorType getErrorType() {
5059
public String toString() {
5160
return "InvalidSyntaxError{" +
5261
" message=" + message +
62+
" ,offendingToken=" + offendingToken +
5363
" ,locations=" + locations +
64+
" ,sourcePreview=" + sourcePreview +
5465
'}';
5566
}
5667

5768

58-
/**
59-
* Creates an invalid syntax error object from an exception
60-
*
61-
* @param parseException the parse exception
62-
*
63-
* @return a new invalid syntax error object
64-
*/
65-
public static InvalidSyntaxError toInvalidSyntaxError(Exception parseException) {
66-
String msg = parseException.getMessage();
67-
SourceLocation sourceLocation = null;
68-
if (parseException.getCause() instanceof RecognitionException) {
69-
RecognitionException recognitionException = (RecognitionException) parseException.getCause();
70-
msg = recognitionException.getMessage();
71-
sourceLocation = new SourceLocation(recognitionException.getOffendingToken().getLine(), recognitionException.getOffendingToken().getCharPositionInLine());
72-
}
73-
return new InvalidSyntaxError(sourceLocation, msg);
74-
}
75-
76-
7769
@SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
7870
@Override
7971
public boolean equals(Object o) {

‎src/main/java/graphql/parser/ExtendedBailStrategy.java‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package graphql.parser;
2+
3+
import graphql.language.SourceLocation;
4+
import org.antlr.v4.runtime.BailErrorStrategy;
5+
import org.antlr.v4.runtime.Parser;
6+
import org.antlr.v4.runtime.RecognitionException;
7+
import org.antlr.v4.runtime.Token;
8+
import org.antlr.v4.runtime.misc.ParseCancellationException;
9+
10+
import java.io.BufferedReader;
11+
import java.io.IOException;
12+
import java.io.StringReader;
13+
import java.util.ArrayList;
14+
import java.util.List;
15+
16+
public class ExtendedBailStrategy extends BailErrorStrategy {
17+
private final String input;
18+
private final String sourceName;
19+
20+
public ExtendedBailStrategy(String input, String sourceName) {
21+
this.input = input;
22+
this.sourceName = sourceName;
23+
}
24+
25+
@Override
26+
public void recover(Parser recognizer, RecognitionException e) {
27+
try {
28+
super.recover(recognizer, e);
29+
} catch (ParseCancellationException parseException) {
30+
throw mkException(recognizer, e);
31+
}
32+
}
33+
34+
@Override
35+
public Token recoverInline(Parser recognizer) throws RecognitionException {
36+
try {
37+
return super.recoverInline(recognizer);
38+
} catch (ParseCancellationException parseException) {
39+
throw mkException(recognizer, null);
40+
}
41+
}
42+
43+
InvalidSyntaxException mkMoreTokensException(Token token) {
44+
SourceLocation sourceLocation = new SourceLocation(token.getLine(), token.getCharPositionInLine());
45+
String sourcePreview = mkPreview(token.getLine());
46+
return new InvalidSyntaxException(sourceLocation,
47+
"There are more tokens in the query that have not been consumed",
48+
sourcePreview, token.getText(), null);
49+
}
50+
51+
52+
private InvalidSyntaxException mkException(Parser recognizer, RecognitionException cause) {
53+
String sourcePreview = null;
54+
String offendingToken = null;
55+
SourceLocation sourceLocation = null;
56+
Token currentToken = recognizer.getCurrentToken();
57+
if (currentToken != null) {
58+
int line = currentToken.getLine();
59+
int column = currentToken.getCharPositionInLine();
60+
offendingToken = currentToken.getText();
61+
sourcePreview = mkPreview(line);
62+
sourceLocation = new SourceLocation(line, column, sourceName);
63+
}
64+
return new InvalidSyntaxException(sourceLocation, null, sourcePreview, offendingToken, cause);
65+
}
66+
67+
/* grabs 3 lines before and after the syntax error */
68+
private String mkPreview(int line) {
69+
StringBuilder sb = new StringBuilder();
70+
BufferedReader reader = new BufferedReader(new StringReader(input));
71+
int startLine = line - 3;
72+
int endLine = line + 3;
73+
try {
74+
List<String> lines = readAllLines(reader);
75+
for (int i = 0; i < lines.size(); i++) {
76+
if (i >= startLine && i <= endLine) {
77+
sb.append(lines.get(i)).append('\n');
78+
}
79+
}
80+
} catch (IOException ignored) {
81+
// this cant happen - its in memory
82+
}
83+
return sb.toString();
84+
}
85+
86+
private List<String> readAllLines(BufferedReader reader) throws IOException {
87+
List<String> lines = new ArrayList<>();
88+
String ln;
89+
while ((ln = reader.readLine()) != null) {
90+
lines.add(ln);
91+
}
92+
reader.close();
93+
return lines;
94+
}
95+
}

‎src/main/java/graphql/parser/InvalidSyntaxException.java‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package graphql.parser;
2+
3+
4+
import graphql.GraphQLException;
5+
import graphql.Internal;
6+
import graphql.InvalidSyntaxError;
7+
import graphql.language.SourceLocation;
8+
9+
import java.util.Collections;
10+
import java.util.List;
11+
12+
@Internal
13+
public class InvalidSyntaxException extends GraphQLException {
14+
15+
private final String message;
16+
private final String sourcePreview;
17+
private final String offendingToken;
18+
private final SourceLocation location;
19+
20+
InvalidSyntaxException(SourceLocation location, String msg, String sourcePreview, String offendingToken, Exception cause) {
21+
super(cause);
22+
this.message = mkMessage(msg, offendingToken, location);
23+
this.sourcePreview = sourcePreview;
24+
this.offendingToken = offendingToken;
25+
this.location = location;
26+
}
27+
28+
private String mkMessage(String msg, String offendingToken, SourceLocation location) {
29+
StringBuilder sb = new StringBuilder();
30+
sb.append("Invalid Syntax :");
31+
if (msg != null) {
32+
sb.append(" ").append(msg);
33+
}
34+
if (offendingToken != null) {
35+
sb.append(String.format(" offending token '%s'", offendingToken));
36+
}
37+
if (location != null) {
38+
sb.append(String.format(" at line %d column %d", location.getLine(), location.getColumn()));
39+
}
40+
return sb.toString();
41+
}
42+
43+
public InvalidSyntaxError toInvalidSyntaxError() {
44+
List<SourceLocation> sourceLocations = location == null ? null : Collections.singletonList(location);
45+
return new InvalidSyntaxError(sourceLocations, message, sourcePreview, offendingToken);
46+
}
47+
48+
49+
@Override
50+
public String getMessage() {
51+
return message;
52+
}
53+
54+
public SourceLocation getLocation() {
55+
return location;
56+
}
57+
58+
public String getSourcePreview() {
59+
return sourcePreview;
60+
}
61+
62+
public String getOffendingToken() {
63+
return offendingToken;
64+
}
65+
66+
}
67+

‎src/main/java/graphql/parser/Parser.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,27 @@
44
import graphql.language.Document;
55
import graphql.parser.antlr.GraphqlLexer;
66
import graphql.parser.antlr.GraphqlParser;
7-
import org.antlr.v4.runtime.BailErrorStrategy;
87
import org.antlr.v4.runtime.CharStream;
98
import org.antlr.v4.runtime.CharStreams;
109
import org.antlr.v4.runtime.CommonTokenStream;
1110
import org.antlr.v4.runtime.Token;
1211
import org.antlr.v4.runtime.atn.PredictionMode;
13-
import org.antlr.v4.runtime.misc.ParseCancellationException;
1412

1513
import java.util.List;
1614

1715
@Internal
1816
public class Parser {
1917

20-
public Document parseDocument(String input) {
18+
public Document parseDocument(String input) throws InvalidSyntaxException {
2119
return parseDocument(input, null);
2220
}
2321

24-
public Document parseDocument(String input, String sourceName) {
22+
public Document parseDocument(String input, String sourceName) throws InvalidSyntaxException {
2523

2624
CharStream charStream;
27-
if(sourceName == null) {
25+
if (sourceName == null) {
2826
charStream = CharStreams.fromString(input);
29-
} else{
27+
} else {
3028
charStream = CharStreams.fromString(input, sourceName);
3129
}
3230

@@ -37,12 +35,14 @@ public Document parseDocument(String input, String sourceName) {
3735
GraphqlParser parser = new GraphqlParser(tokens);
3836
parser.removeErrorListeners();
3937
parser.getInterpreter().setPredictionMode(PredictionMode.SLL);
40-
parser.setErrorHandler(new BailErrorStrategy());
41-
GraphqlParser.DocumentContext documentContext = parser.document();
4238

39+
ExtendedBailStrategy bailStrategy = new ExtendedBailStrategy(input, sourceName);
40+
parser.setErrorHandler(bailStrategy);
41+
42+
GraphqlAntlrToLanguage toLanguage = new GraphqlAntlrToLanguage(tokens);
43+
GraphqlParser.DocumentContext documentContext = parser.document();
4344

44-
GraphqlAntlrToLanguage antlrToLanguage = new GraphqlAntlrToLanguage(tokens);
45-
Document doc = antlrToLanguage.createDocument(documentContext);
45+
Document doc = toLanguage.createDocument(documentContext);
4646

4747
Token stop = documentContext.getStop();
4848
List<Token> allTokens = tokens.getTokens();
@@ -55,9 +55,10 @@ public Document parseDocument(String input, String sourceName) {
5555
boolean lastGreaterThanDocument = last.getTokenIndex() > stop.getTokenIndex();
5656
boolean sameChannel = last.getChannel() == stop.getChannel();
5757
if (notEOF && lastGreaterThanDocument && sameChannel) {
58-
throw new ParseCancellationException("There are more tokens in the query that have not been consumed");
58+
throw bailStrategy.mkMoreTokensException(last);
5959
}
6060
}
6161
return doc;
6262
}
63+
6364
}

0 commit comments

Comments
 (0)