Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Invalid key SHARDING #2

Closed
download13 opened this issue Apr 13, 2017 · 10 comments
Closed

Invalid key SHARDING #2

download13 opened this issue Apr 13, 2017 · 10 comments
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@download13
Copy link

I get the following error when running npm test on Windows 10.

This also results in a failed build when using js-ipfs which depends on it.

Error: Invalid key: \SHARDING
    at new Key (C:\Users\download\Desktop\js-datastore-core\node_modules\interface-datastore\src\key.js:42:13)
    at Object.<anonymous> (C:\Users\download\Desktop\js-datastore-core\src\sharding.js:12:18)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (C:\Users\download\Desktop\js-datastore-core\src\index.js:5:27)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (C:\Users\download\Desktop\js-datastore-core\test\keytransform.spec.js:16:27)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at C:\Users\download\Desktop\js-datastore-core\node_modules\mocha\lib\mocha.js:222:27
    at Array.forEach (native)
    at Mocha.loadFiles (C:\Users\download\Desktop\js-datastore-core\node_modules\mocha\lib\mocha.js:219:14)
    at Mocha.run (C:\Users\download\Desktop\js-datastore-core\node_modules\mocha\lib\mocha.js:487:10)
    at Object.<anonymous> (C:\Users\download\Desktop\js-datastore-core\node_modules\mocha\bin\_mocha:459:18)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.runMain (module.js:605:10)
    at run (bootstrap_node.js:420:7)
    at startup (bootstrap_node.js:139:9)
    at bootstrap_node.js:535:3

Is there a reason that the sharding keys don't conform to the rules of the Key class?

@dignifiedquire
Copy link
Member

It seems the different path separators on windows are playing a trick on us here, when using path.normalize to generate the key. Thank you for the report

@dignifiedquire dignifiedquire added the kind/bug A bug in existing code (including security flaws) label Apr 13, 2017
@dignifiedquire
Copy link
Member

I don't have a windows machine to test right now, could you test with this PR ipfs/interface-datastore#5 please, I believe it should make the tests pass on windows

@download13
Copy link
Author

I have an alternative pull request: ipfs/interface-datastore#6

This one keeps the path style consistent and changes as little as possible.

If you're willing to add another dependency though, we could use normalize-path instead of my homebrew function which should always produce consistent results.

@nightwolfz
Copy link

Any progress on this? This is literally the only issue stopping me from using IPFS

@dignifiedquire
Copy link
Member

@download13 @nightwolfz I just merged the PR, I was able to get it working including passing CI on appveyor so it doesn't break immediately again.

We will release bubble up the changes in the coming days, so a fix should be soon available in js-ipfs itself

@daviddias
Copy link
Member

👏🏽👏🏽👏🏽👏🏽👏🏽👏🏽 :D thank you, @dignifiedquire

@daviddias daviddias added status/ready Ready to be worked P2 Medium: Good to have, but can wait until someone steps up exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue labels Oct 13, 2017
@daviddias
Copy link
Member

The problem persists. @dignifiedquire did you remember testing it?

@dignifiedquire
Copy link
Member

I never tested the upper layers I think.

@richardschneider
Copy link
Contributor

PR ipfs/interface-datastore#12 fixes the windows issue.

Perhaps someone could review it.

@daviddias
Copy link
Member

Being fixed with #3

@ghost ghost removed the status/ready Ready to be worked label Nov 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

5 participants