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

Throw an exception if a mandatory field is missing #15

Open
alexduf opened this issue Feb 12, 2021 · 2 comments
Open

Throw an exception if a mandatory field is missing #15

alexduf opened this issue Feb 12, 2021 · 2 comments

Comments

@alexduf
Copy link

alexduf commented Feb 12, 2021

The parser silently ignores a missing field. This isn't the correct behaviour and should throw an exception.

However what do we want to do if the field is present and with the wrong type?

  • Skip the field if it's optional, and throw an exception if it's mandatory?
  • Throw an exception in all cases?

I'm leaning towards throwing an exception in all cases but I'm happy to have my mind changed.

We'll also need a test case for both these cases, which will have to happen in the js part of the tests.

@davidfurey
Copy link
Member

I think it should throw an exception if a field exists in both the thrift definition and in the object being deserialised but the types do not match, regardless of optionality. This suggests that the thrift object is not of the correct format and all bets are off on whether the rest of the data is actually what you expect.

If the field exists in the object being deserialised but does not exist in the thrift definition, I think this should be ignored. This suggests that the object being deserialised is using a backwards compatible newer version of the thrift definition, which shouldn't be a problem.

@davidfurey
Copy link
Member

For unrelated reasons I was looking at the scrooge generated Scala code, and it looks like it follows the behaviour I suggested.

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

2 participants