★ wanayoo — archive 1999 https://github.com/graphql-java/graphql-java/issues/1322Nouvelle recherche | Portail wanayoo
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for #352 is preventing meaningful Schema parsing errors #1322

Closed
abazlinton opened this issue Nov 21, 2018 · 2 comments
Closed

Fix for #352 is preventing meaningful Schema parsing errors #1322

abazlinton opened this issue Nov 21, 2018 · 2 comments

Comments

@abazlinton
Copy link

@abazlinton abazlinton commented Nov 21, 2018

I made a dumb mistake in my schema (oops. in front of Query) -

type oops.Query {
    pets: [Pet]
    firstPet: Pet
}

type Pet {
    name: String
}

With the fix for #352 in place (current master) this generates a ParseCancellationException with only the text -
There are more tokens in the query that have not been consumed

If I comment out the fix, I get the much more helpful -

Caused by: com.coxautodev.graphql.tools.SchemaClassScannerError: Type definition for root query type 'Query' not found!

@bbakerman
Copy link
Member

@bbakerman bbakerman commented Nov 22, 2018

So the previous behavior was a result of a bug most likely.

oops.Query is not a valid set of characters for a named element, which is defined as

NAME: [_A-Za-z][_0-9A-Za-z]*;

So it is correctly saying its invalid syntax.

We could accept ANY name and then validate but this would lead to silly situations with meaningful characters and open us up parsing bugs.

I understand your point about how its not easy to work out whats wrong but it is correct.

Building loose parsers and then guarding yourself on all the conditions is not a task we would want take on.

@abazlinton
Copy link
Author

@abazlinton abazlinton commented Nov 25, 2018

Sorry I don't think I've described this issue very well. I'll close this for now. Thanks for your time :)

@abazlinton abazlinton closed this Nov 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.