Skip to content
This repository was archived by the owner on Jun 26, 2022. It is now read-only.

Conversation

@alexjpwalker
Copy link
Member

What is the goal of this PR?

Previously, when parsing a query failed with an error, on converting the resulting error object to a string, the string would contain two copies of the error message - e.g:

[GQL03] TypeQL Error: There is a syntax error at line 1:
match $x sb thing;
         ^
no viable alternative at input 'match $x sb'
[GQL03] TypeQL Error: There is a syntax error at line 1:
match $x sb thing;
         ^
no viable alternative at input 'match $x sb'

This is now fixed and error messages are no longer duplicated. (Also, the error code is now TQL)

What are the changes implemented in this PR?

@alexjpwalker alexjpwalker added this to the 2.1.1 milestone Jul 21, 2021
@alexjpwalker alexjpwalker self-assigned this Jul 21, 2021
@alexjpwalker alexjpwalker marked this pull request as ready for review July 21, 2021 12:24
Copy link
Member Author

@alexjpwalker alexjpwalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good

// DefaultErrorStrategy + LL_EXACT_AMBIG_DETECTION
// This was not set to default parsing strategy, but it is useful
// to produce detailed/useful error message
errorListener.clearErrors();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling logic is as follows:

  1. Attach error listener to parser - whenever a parser error is thrown, it gets stored in the Listener
  2. Parse query with optimised parser (returns less useful error messages)
  3. If parsing fails, re-parse query with strict parser (returns more detailed error messages)
  4. If parsing fails (which it will), dump all errors in the Listener to an exception string.

Trouble is, this flow causes 2 errors (which may not necessarily be identical, but often are) to be piped into the Listener, and they both make it into the output.

To fix this issue, we clean the first (less precise) error from the Listener before storing the second, more precise one.

@haikalpribadi haikalpribadi merged commit cf36798 into master Jul 21, 2021
@flyingsilverfin flyingsilverfin deleted the duplicate-error-msg branch January 12, 2022 09:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error messages are duplicated

3 participants