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 JsonParser error behavior #1038

Merged
merged 1 commit into from Jan 6, 2022
Merged

Conversation

resnickj
Copy link
Contributor

@resnickj resnickj commented Jan 6, 2022

Issues:

  • JsonParser sometimes throws JavaScriptException (SyntaxError) and sometimes throws ParserException; should be consistent and always throw one or the other.
  • A script that calls JSON.parse(input) should always see a SyntaxError in the case of syntactically invalid input.
  • SyntaxError messages should clearly indicate that the error is in JSON data, and indicate the position within the input string.
  • SyntaxError messages should not include the entire input string in the message, as it may be arbitrarily long.

Changes:

  • Route all exceptions through a common ThrowError(...) method that always throws a SyntaxError
  • Clean-up content of error messages and include position of the error. Unfortunately the parser does not track line number accurately, so position is rendered as a single number as opposed to line:column format.
  • Add tests to verify the expected error output for each call site of ThrowError(...)

Issues:
* JsonParser sometimes throws JavaScriptException (SyntaxError) and sometimes throws ParserException; should be consistent and always throw one or the other.
* A script that calls JSON.parse(input) should always see a SyntaxError in the case of syntactically invalid input.
* SyntaxError messages should clearly indicate that the error is in JSON data, and indicate the position within the input string.
* SyntaxError messages should not include the entire input string in the message, as it may be arbitrarily long.

Changes:
* Route all exceptions through a common ThrowError(...) method.
* Clean-up content of error messages and include position of the error. Unfortunately the parser does not track line number accurately, so position is rendered as a single number as opposed to line:column format.
* Add tests to verify the expected error output for each call site of ThrowError(...)
@lahma lahma changed the title Fix JsonParser error behaviour Fix JsonParser error behavior Jan 6, 2022
@lahma lahma merged commit 7148772 into sebastienros:main Jan 6, 2022
@lahma
Copy link
Collaborator

lahma commented Jan 6, 2022

Great work, thank you!

@resnickj resnickj deleted the syntax-error branch January 6, 2022 17:23
fadoaglauss pushed a commit to takenet/jint that referenced this pull request Oct 11, 2022
fadoaglauss added a commit to takenet/jint that referenced this pull request Oct 18, 2022
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.

None yet

2 participants