coreybutler / node-mac

Node utilities for Mac
Other
539 stars 51 forks source link

Path error #14

Closed kitzler-walli closed 8 years ago

kitzler-walli commented 8 years ago

When I try to install the service I get this message:

TypeError: Path must be a string. Received undefined at assertPath (path.js:7:11) at Object.dirname (path.js:1326:5) at new daemon (/Users/stephan/nodejs/uwz/node_modules/node-mac/lib/daemon.js:124:30) at repl:1:11 at sigintHandlersWrap (vm.js:32:31) at sigintHandlersWrap (vm.js:96:12) at ContextifyScript.Script.runInContext (vm.js:31:12) at REPLServer.defaultEval (repl.js:308:29) at bound (domain.js:280:14) at REPLServer.runBound as eval

Here's the code I wrote:

var svc = new Service({
        name:'UWZ',
        description: 'UWZ Logger.',
        script: '/Users/stephan/nodejs/uwz/server.js'
});
coreybutler commented 8 years ago

This could be a problem within your script as well. Does the script run without node-mac?

kitzler-walli commented 8 years ago

Yes - my script works fine, but when I try to install it as a service I get this error. Is any of the node modules required to be installed as global?

fcardona commented 8 years ago

I'm getting the same error too.

coreybutler commented 8 years ago

node-mac doesn't require any specific node modules that aren't self-contained.

In every case I've ever seen this, the root cause has always been a relative path somewhere in the code base. This could be in a require statement, it could be the use of __dirname, or something else looking for a relative path. In 90% of the cases, folks don't even realize they're using relative paths. If you're using __dirname, consider using process.cwd() instead.

Background processes on every OS operate within their own root directory... /Library, /etc/systemd, C:\Users\<profile>, not within the scope of a traditional directory (as is done in 99% of node development).

I have actually been experiencing this myself recently in packaging Electron apps, because it operates in a similar way. Personally, I troubleshoot it by commenting out pretty much everything to make sure I can get the daemon installed. Then I go through and uncomment pieces one at a time, reinstalling until I find the source of the problem. Yes, it sucks, but in every case I've found something within my code, not the daemon.

warling commented 8 years ago

@coreybutler, it seems to be a difference in node itself between the 5.x and 6.x branches. The error described by @kitzler-walli and @fcardona shows up only under 6.x; it works fine under 5.x. I'm still trying to pinpoint the actual source of the error, but I can easily reproduce it by switching between branches:

If I use homebrew to install the 6.x branch using brew install node I'll get the error. If I uninstall the 6.x branch and revert to the 5.x branch (brew unlink node; brew install homebrew/versions/node5) then everything works fine again.

I do hope this gets updated soon; I have to run 5.x in my production environment because it requires node-mac; my local development version (where a simple npm test or npm start suffices) is proceeding on the 6.x branch.

In the meantime, I'll continue trying to find the error so I can at least patch it locally...

(Thanks for making an incredibly great and useful utility by the way — I use a Mac Pro as my server hardware, and I've built up my whole production environment around your node-mac library! :-)

warling commented 8 years ago

By the way, I get slightly different stack trace, but the endpoint is the same:

path.js:7
    throw new TypeError('Path must be a string. Received ' + inspect(path));
    ^

TypeError: Path must be a string. Received undefined
    at assertPath (path.js:7:11)
    at Object.dirname (path.js:1326:5)
    at new daemon (/Users/darinw/libtb/site.com/node_modules/node-mac/lib/daemon.js:126:30)
    at Object.<anonymous> (/Users/darinw/libtb/site.com/server.js:134:17)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.runMain (module.js:590:10)

Line 134:17 in the above stack track is the const service = new Service( { in the following code:

//  Define our file path:
const filePath = path.join( directoryPathRoot, fileName );
log( 'file path is ' + filePath );

//  Define our service name:
const serviceName = configuration.serviceName;
log( 'service name is "' + serviceName + '"' );

//  Define our service description:
const serviceDescription = configuration.serviceDescription;
log( 'service description is "' + serviceDescription + '"' );

// Create a new service object:
const service = new Service( {
    name: serviceName,
    description: serviceDescription,
    script: filePath,
    env: {
        name: 'NODE_ENV',
        value: 'production'
    }
} );
assert.isNonEmptyObject( service );
log( 'node-mac Service object created' );

When run I get the following filePath logged from the beginning of the above code snippet:

20160825013605  file path is /Users/darinw/site.com/gdm.js

So I'm definitely not using relative paths.

warling commented 8 years ago

Continuing my investigation...

On line 126 of daemon.js you have the line:

value: config.cwd || p.dirname(this.script)

In both node5 and node6, console.log( this.script ); returns undefined; however, in node6, p.dirname( undefined ) throws an assertion error, but in node5 it doesn't.

According to the latest node path.dirname documentation, if the dirname path isn't a string it will throw a TypeError:

https://nodejs.org/api/path.html#path_path_dirname_path

It looks like the solution is to maybe recode this along the lines of:

value: config.cwd || p.dirname( ( this.script !== undefined ) && ( this.script !== null ) ) ? this.script.toString() : '' )

(This seems to have solved the problem in my environment; I'll let you put in the "real" fix though :-)

coreybutler commented 8 years ago

@warling - great job tracking this down. I think your solution is fine.... it should be backwards compatible and support Node 6. Why don't you submit a PR? I'll merge it.

For anyone else coming across this issue, it should be known there were a whole bunch of breaking changes between node 5 and 6. This is one of them. There may be more, and it's next to impossible for me to write unit tests for this (creating daemons aren't usually allowed on most CI services, and Travis is the only one support OSX). If you come across an issue you suspect may be a breaking change, please file an issue (a PR with the fix would be even better!)

warling commented 8 years ago

@coreybutler Done!

coreybutler commented 8 years ago

I just published this to npm as version 0.1.5.