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

Ambiguous load/loadAll semantics when dealing with broken input #682

Open
fnmdx111 opened this issue Mar 2, 2021 · 3 comments
Open

Ambiguous load/loadAll semantics when dealing with broken input #682

fnmdx111 opened this issue Mar 2, 2021 · 3 comments

Comments

@fnmdx111
Copy link

fnmdx111 commented Mar 2, 2021

Docs on load (similarly loadAll) says

If no value is found, load will return null.

However, if I feed a piece of broken input into load, I get exception rather than a null value I was expecting. I would argue if an input cannot be loaded as an Ion value, it should be regarded as a "no value is found" situation.

Repro: https://runkit.com/embed/unc0ogmrc1fq

@fnmdx111
Copy link
Author

fnmdx111 commented Mar 7, 2021

I found another case where ion-js surprises me: if you feed load with {broken, you get back an empty Struct. If I'm interpreting the docs on load correctly, I would expect it to return null because there's no valid Ion value in the input.

(Found with 4.1.0)

@zslayton
Copy link
Contributor

zslayton commented Mar 9, 2021

I would argue if an input cannot be loaded as an Ion value, it should be regarded as a "no value is found" situation.

The load method must differentiate between three possible scenarios.

1. The input is valid Ion with one or more values

In this case, load returns the first value it encounters.

2. The input is valid Ion with zero values

It is possible for an input source to represent an Ion stream without any values. For example:

  • The byte array [0xE0, 0x01, 0x00, 0xEA] is a binary Ion 1.0 stream that contains a version marker but no values.
  • The string "$ion_1_0\n" is a text Ion 1.0 stream that contains a version marker but no values.
  • The string "" is a text Ion 1.0 stream that contains neither a version marker nor values.
  • The string "/* This is a long-form comment. */" is a text Ion 1.0 stream that contains a comment but no values.

In cases like these load returns null, indicating that the complete stream was read successfully but no values were found.

3. The input is invalid Ion

The stream cannot be interpreted as Ion. We cannot say whether or not it contains any values because it cannot be decoded. In this case, we throw an exception.

Note that an Ion reader (and by extension, the load method) is not able to detect whether the provided data is Ion with 100% certainty. There are many kinds of input that could accidentally be interpreted as Ion. For example, consider this TSV file:

title                            yearPublished    cost
"The Old Man and the Sea"        1952             9.95

By coincidence, this file is also a valid Ion text stream. Instead of being two rows with three columns each, an Ion reader would consider it to be a series of six top-level values: three symbols, a string, an integer, and a decimal value.

For this reason, it is the user's responsibility to confirm that the data they are passing to the load method is Ion data and not some other format.

I found another case where ion-js surprises me: if you feed load with {broken, you get back an empty Struct. If I'm interpreting the docs on load correctly, I would expect it to return null because there's no valid Ion value in the input.

This is definitely not the expected behavior; load should throw an exception. Thank you for reporting it!

@fnmdx111
Copy link
Author

fnmdx111 commented Mar 11, 2021

Thanks for the detailed explanation! Maybe the docs on load/loadAll can be expanded a bit more on this front (since it caused user confusion).

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