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

Issue #299: unclear error message in case of multiline string argument #412

Merged
merged 5 commits into from
Nov 13, 2020

Conversation

suntoucha
Copy link
Contributor

Fix #299

Package text/scanner, which is used to parse queries, already detects multiline strings as an error. But it only puts log message to console, as it was pointed in the issue description. Scanner can also call a function each time it encounters error

So the idea of the fix is:

  • provide Scanner a function to catch errors
  • generate high-level errors in that function
  • couple of tests

@suntoucha
Copy link
Contributor Author

Excuse me, can anyone give a hint what could be the difference between local test run and semaphoreci?

Local run passes ok:

--- PASS: TestMultilineString (0.00s)
--- PASS: TestMultilineString/Oneline_strings_are_okay (0.00s)
--- PASS: TestMultilineString/Multiline_strings_are_not_allowed (0.00s)

Semaphoreci failes (part of the returned error string after "See near..." is missing):

--- FAIL: TestMultilineString (0.00s)
--- PASS: TestMultilineString/Oneline_strings_are_okay (0.00s)
--- FAIL: TestMultilineString/Multiline_strings_are_not_allowed (0.00s)
lexer_test.go:126: Test 'Multiline strings are not allowed' failed with wrong error: 'graphql: syntax error: literal not terminated. See near (line 1, column 1)'. Error should be: 'graphql: syntax error: literal not terminated. See near "Hello (line 1, column 1)'

Thank you!

@suntoucha
Copy link
Contributor Author

Tests are OK now

The only difference I've noticed is that SemaphoreCI runs on go 1.11 by default

Copy link
Member

@pavelnikolov pavelnikolov left a comment

Choose a reason for hiding this comment

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

Thank you. Can you, please fix the empty lines and I'd be happy to merge this PR.

@pavelnikolov pavelnikolov merged commit 9c36bba into graph-gophers:master Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiline strings in mutation error
2 participants