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

Better error text for JSON syntax errors during schema parsing #15

Open
ChrisRx opened this issue Mar 30, 2018 · 2 comments
Open

Better error text for JSON syntax errors during schema parsing #15

ChrisRx opened this issue Mar 30, 2018 · 2 comments

Comments

@ChrisRx
Copy link

ChrisRx commented Mar 30, 2018

I ran into an issue where a particularly large schema was not parsing due to a simple syntax error that was introduced mistakenly while working with it. This was difficult to track down since the behavior in schema.go:929 meant that the json.SyntaxError was never handled and ultimately ends up returning the error from schema.go:976 indicating "Unknown type name:" followed by the entire body of the provided schema.

I am unsure the intention for schema.go:929 but I noticed that it was present in the original version of this library by @elodina. Is there a reason that given an error returned while JSON unmarshaling the schema should still be passed on to schemaByType or is it safe to go ahead and handle the json error since it is not likely (or impossible) that it is a valid avro schema?

I made #14 that may help to show what change I am proposing and as a possible fix/enhancement in cases like this one.

@ChrisRx
Copy link
Author

ChrisRx commented Mar 30, 2018

I'm guessing given that the tests fail for TestPrimitiveSchema that this is my answer for why the json error is disregarded in ParseSchema. Any thoughts on how to best proceed?

@ChrisRx
Copy link
Author

ChrisRx commented Mar 31, 2018

I updated the PR with a possible solution. The primitives are handled as a special case in ParseSchema before JSON unmarshaling occurs. Let me know what you think!

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

No branches or pull requests

1 participant