make it work in node 8

Question

make it work in node 8

felixbrucker opened this issue 7 years ago · 8 comments

Upon starting daemon with node 8 you are greeted with

TypeError: "cwd" must be a string
    at normalizeSpawnArguments (child_process.js:401:11)
    at Object.exports.spawn (child_process.js:488:38)
    at Function.module.exports.daemon (C:\Users\Felix\AppData\Roaming\npm\node_modules\storjshare-daemon\node_modules\daemon\index.js:50:31)
    at module.exports (C:\Users\Felix\AppData\Roaming\npm\node_modules\storjshare-daemon\node_modules\daemon\index.js:25:20)
    at utils.checkDaemonRpcStatus (C:\Users\Felix\AppData\Roaming\npm\node_modules\storjshare-daemon\bin\storjshare-daemon.js:54:7)
    at Socket.<anonymous> (C:\Users\Felix\AppData\Roaming\npm\node_modules\storjshare-daemon\lib\utils.js:216:5)
    at Object.onceWrapper (events.js:316:30)
    at emitOne (events.js:115:13)
    at Socket.emit (events.js:210:7)
    at emitErrorNT (internal/streams/destroy.js:64:8)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9)

This is due to a bug in daemon, specifically this one.

As you can see the fix is easy but the maintainer seems inactive and doesn't merge the PR.

I propose to change the following in package.json to enable users to use node 8 if they feel like it:

-    "daemon": "^1.1.0",
+    "daemon": "github:zipang/daemon.node#48d0977c26fb3a6a44ae99aae3471b9d5a761085",

Answer 1 · 2017-10-23T14:56:28.000Z

It would need to be something where the installer would have to select an option or it would have to detect what version of Node it is installing on. Zipang's branch has a lot more changes than the one listed, and his notes state the following...

"Using this module on older versions of node (or older versions of this module) are not recommended due to how node works internally and the issues it can cause for daemons."

Some thought should maybe be given to leaving daemon and move to Forever or pm2 instead. Since daemon is no longer maintained.

Answer 2 · 2017-10-23T15:05:16.000Z

I'm not sure i can follow: his branch (https://github.com/zipang/daemon.node) only has a single commit which inserts () to make the function call work as intended by nodejs. Additionally i can't find the statement you quoted, care to share the source?

Irrelevant of all the above the PR and proposal use the commit hash and thus no other changes are used even if he ever pushes new commits.

Answer 3 · 2017-10-23T15:09:11.000Z

Scroll down on that link you just posted.

Also, note his branch has 75 commits.

Answer 4 · 2017-10-23T15:12:01.000Z

Anyway, we could make our own branch and make that change as well. So that's an option without relying on a third party maintainer. I wonder though since they made so many changes to it, that you'll run into other issues besides this one that they also addressed.

Answer 5 · 2017-10-23T15:12:43.000Z

Only one of the 75 commits is from zipang...

Answer 6 · 2017-10-23T15:14:27.000Z

indexzero:master is the repo and branch used for npm

Answer 7 · 2017-10-23T15:16:48.000Z

Yeah, I'm aware of the parent and he doesn't maintain it. We'll leave it up to the Storj Lab's folks. I could make a PR for this, but I'm not sure of best practice here in terms of inheriting from a third party branch or if we should make our own branch, or even consider moving off of daemon. Someone with more authority will have to make that call.

Answer 8 · 2017-10-23T15:20:57.000Z

This issues goal currently is only to get it working by fixing a broken dependency, if and how storjlabs will use another library (pm2, or else) in the future is a whole other thing and should be discussed in its own issue.