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

CRITICAL: Support for env param disableLosslessIntegers is broken. #44

Closed
purplemana opened this issue Nov 1, 2018 · 1 comment
Closed

Comments

@purplemana
Copy link
Contributor

After spending an entire day finally figured out that this was the reason why the specs failed mysteriously on my setup but were working in your repo's CI.

The code written to parse this value from .env file sets the value to true even if it is given as false. E.g. in the .env.example file provided with the code -

NEO4J_DISABLE_LOSSLESS_INTEGERS=false

This will still be converted to disableLosslessIntegers:true in the parsing logic present at https://github.com/adam-cowley/neode/blob/master/src/index.js#L74,
because false or true values are strings in process.env so !!value will always be true.

A workaround is to remove the line from .env file.

Although this looks innocent, it breaks Neode for two reasons:

  1. The code is always assuming query results to be in neo4j high-precision number format. With disableLosslessIntegers true, all the places where get is being invoked fails because neo4j returns plain numbers which don't have this method.
  2. Relationship code stops working because the way node ids are being populated in the statement https://github.com/adam-cowley/neode/blob/master/src/Services/RelateTo.js#L34 cause the ids to be sent to neo4j driver as plain integers. The driver then converts them to float. So a value of id 10 becomes 10.0 and the query is no longer able to match the nodes and relation creation fails silently. An exception is thrown by https://github.com/adam-cowley/neode/blob/master/src/Services/RelateTo.js#L45 which is expecting records[0] to be present.

I would suggest disabling this mode completely till the relevant code is fixed. Two changes need to be done:

  1. Remove the NEO4J_DISABLE_LOSSLESS_INTEGERS=false line from env.example so people (like yours truly) don't inadvertently copy that into their setup.
  2. Remove disableLosslessIntegers from config till the code is fixed.
@purplemana
Copy link
Contributor Author

@adam-cowley could you please provide your thoughts on this one?

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