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

App fails to start if not invoked from the app root. #55

Closed
lmarkus opened this issue Dec 9, 2013 · 7 comments
Closed

App fails to start if not invoked from the app root. #55

lmarkus opened this issue Dec 9, 2013 · 7 comments
Labels

Comments

@lmarkus
Copy link
Contributor

lmarkus commented Dec 9, 2013

eg: App lives in ~/dev/myApp/index.js

If I invoke the app from a different directory it fails:

eg:

~dev $ node myApp/index.js

Solution (PR to be submitted later)
This line

this._config = config.create(this._resolve('.'));
could be changed to
this._config = config.create(this._resolve(path.dirname(process.argv[1])));

To reflect the true root of the app, as opposed to the current working directory.

@totherik
Copy link
Member

totherik commented Dec 9, 2013

FYI: Accessing the main module

Also, consider that this breaks down when process managers are involved. For example, when using PM2 the main module is not the file you think it is.

Additionally, this was built to start in the current directory and traverse upward, looking for a valid kraken application to support the experimental mounting of apps. If you'll notice, it explicitly DOESN'T use process.cwd(), but starts with the current file location (inside node_modules, hence the .); Please see the root resolver to get a better understanding of what's going on here.

EDIT: It doesn't use process.cwd() unless a valid kraken app isn't found. If there's a bug, it might be there.

@lmarkus
Copy link
Contributor Author

lmarkus commented Dec 9, 2013

Yep, I was just going to point out what you edited.
I was looking through rootResolver and I think the issue might be with how we're using the isKraken(obj) function in this loop.

The parameter being passed by rootResolver wants to compare the prototype of module.exports for whatever module is being evaluated in the loop, against the kraken object (Or the unicode yingyang).

Because of this, I don't think this condition will evaluate to true during the loop, so it eventually stops when there are no more modules to traverse (current.parent=null).

At this point, since root was never defined, the function returns process.cwd()

On my solution, I had not considered other startup methods for the app, so good callout.

Will continue looking at this tomorrow.

@kethinov
Copy link
Contributor

kethinov commented Dec 9, 2013

rootResolver seems like a pretty heavy way to determine the location of the app's root directory. Why not examine module.parent.filename from within kraken to get the directory of the file which required kraken?

@kethinov
Copy link
Contributor

kethinov commented Dec 9, 2013

Also, is pm2 the recommended way to deploy kraken to a live server? Do you prefer it over, say, node-forever? And are there other such tools worth considering as well?

@totherik
Copy link
Member

totherik commented Dec 9, 2013

rootResolver is heavy because it was intended to support more complex application composition. If we choose not to pursue composable, mountable apps then much of this code can be deleted (with no argument from me). module.parent.filename is not necessarily the module that required a file, but the file that required said file/module, and with composition, depending on the version of kraken, it might not be what you're looking for.

The rub here is composition. If I create a kraken application and publish it as a module, depending on the version of kraken I depend on, I may have my own "application root" (with my own copy of kraken, templates, etc.) which is in a nested directory of the actual application root, but this nested directory (within the top level node_modules) contains all my application context. @jasisk and I spent a good amount of time working through possible scenarios creating the "heavy" resolver to be resilient to arbitrarily composed applications. The complexity was added for a specific use-case, but if it's deemed unnecessary, I'm more than happy to remove it.

Also, pm2 isn't necessarily the recommended solution, but it's an example of a situation where a script or application runner defines an alternate require.main.filename crushing all assumptions of what truly is the application root. BTW, there is a flag in pm2 which indicates it may patch require.main.filename, but last I checked it wasn't available for use.

@jeffharrell
Copy link
Member

Is this an issue anymore as of 0.7?

@ghost ghost assigned lmarkus Jan 28, 2014
@lmarkus
Copy link
Contributor Author

lmarkus commented Jan 29, 2014

Nope...

@lmarkus lmarkus closed this as completed Jan 29, 2014
@lmarkus lmarkus removed their assignment Apr 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants