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

Sails 1.0.0-33 doesn't boot on Node 8.3.0 when accessing require('sails').config #4165

Closed
sadasant opened this issue Aug 11, 2017 · 19 comments
Closed

Comments

@sadasant
Copy link

sadasant commented Aug 11, 2017

Sails version: 1.0.0-36
Node version: 8.3.0
NPM version: 5.3.0
Operating system: Debian GNU/Linux 8.7 (jessie)


Our project uses Sails 1.0.0-33, it boots perfectly on Node 8.2.0, but it breaks on Node 8.3.0. The error is:

TypeError: Cannot match against 'undefined' or 'null'.
    at Object.<anonymous> (/home/sadasant/code/github.com/project/api/services/IntercomService.js:5:37)
    at Module._compile (module.js:573:30)
    at Object.Module._extensions..js (module.js:584:10)
    at Module.load (module.js:507:32)
    at tryModuleLoad (module.js:470:12)
    at Function.Module._load (module.js:462:3)
    at Module.require (module.js:517:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/home/sadasant/code/github.com/project/api/services/IntercomEmailSender.js:5:93)
    at Module._compile (module.js:573:30)
    at Object.Module._extensions..js (module.js:584:10)
    at Module.load (module.js:507:32)
    at tryModuleLoad (module.js:470:12)
    at Function.Module._load (module.js:462:3)
    at Module.require (module.js:517:17)
    at require (internal/module.js:11:18)
    at /home/sadasant/code/github.com/project/node_modules/include-all/lib/help-include-all-sync.js:293:33
    at Array.forEach (<anonymous>)
    at _recursivelyIncludeAll (/home/sadasant/code/github.com/project/node_modules/include-all/lib/help-include-all-sync.js:178:11)
    at includeAll (/home/sadasant/code/github.com/project/node_modules/include-all/lib/help-include-all-sync.js:317:5)
    at helpBuildDictionary (/home/sadasant/code/github.com/project/node_modules/include-all/lib/help-build-dictionary.js:46:13)
    at Function.module.exports.optional (/home/sadasant/code/github.com/project/node_modules/include-all/index.js:67:10)
    at Hook.loadServices (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/moduleloader/index.js:339:18)
    at Hook.wrapper [as loadServices] (/home/sadasant/code/github.com/project/node_modules/@sailshq/lodash/lib/index.js:3250:19)
    at Hook.loadModules (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/services/index.js:68:21)
    at Hook.wrapper [as loadModules] (/home/sadasant/code/github.com/project/node_modules/@sailshq/lodash/lib/index.js:3250:19)
    at modules (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/index.js:79:25)
    at runTask (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1662:17)
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1602:17
    at processQueue (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1612:17)
    at Object.auto (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1598:9)
    at Hook.load (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/index.js:73:13)
    at Hook.wrapper [as load] (/home/sadasant/code/github.com/project/node_modules/@sailshq/lodash/lib/index.js:3250:19)[67/795]
    at modules (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/index.js:79:25)
    at runTask (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1662:17)
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1602:17
    at processQueue (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1612:17)
    at Object.auto (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1598:9)
    at Hook.load (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/index.js:73:13)
    at Hook.wrapper [as load] (/home/sadasant/code/github.com/project/node_modules/@sailshq/lodash/lib/index.js:3250:19)
    at loadHook (/home/sadasant/code/github.com/project/node_modules/sails/lib/app/private/loadHooks.js:206:17)
    at /home/sadasant/code/github.com/project/node_modules/sails/lib/app/private/loadHooks.js:328:13
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:3047:20
    at eachOfArrayLike (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1002:13)
    at eachOf (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1052:9)
    at Object.eachLimit (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:3111:7)
    at load (/home/sadasant/code/github.com/project/node_modules/sails/lib/app/private/loadHooks.js:326:17)
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:3671:13
    at replenish (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:884:21)
    at iterateeCallback (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:869:21)
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:847:20
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:3676:17
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:339:31
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:952:25
    at iteratorCallback (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:997:17)
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:847:20
    at _combinedTickCallback (internal/process/next_tick.js:131:7)
    at process._tickCallback (internal/process/next_tick.js:180:9)
    at Function.Module.runMain (module.js:611:11)
    at startup (bootstrap_node.js:158:16)
    at bootstrap_node.js:578:3
--
    at /home/sadasant/code/github.com/project/node_modules/include-all/lib/help-include-all-sync.js:298:25
    at Array.forEach (<anonymous>)
    at _recursivelyIncludeAll (/home/sadasant/code/github.com/project/node_modules/include-all/lib/help-include-all-sync.js:178:11)
    at includeAll (/home/sadasant/code/github.com/project/node_modules/include-all/lib/help-include-all-sync.js:317:5)
    at helpBuildDictionary (/home/sadasant/code/github.com/project/node_modules/include-all/lib/help-build-dictionary.js:46:13)
    at Function.module.exports.optional (/home/sadasant/code/github.com/project/node_modules/include-all/index.js:67:10)
    at Hook.loadServices (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/moduleloader/index.js:339:18)
    at Hook.wrapper [as loadServices] (/home/sadasant/code/github.com/project/node_modules/@sailshq/lodash/lib/index.js:3250:19)
    at Hook.loadModules (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/services/index.js:68:21)
    at Hook.wrapper [as loadModules] (/home/sadasant/code/github.com/project/node_modules/@sailshq/lodash/lib/index.js:3250:19)
    at modules (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/index.js:79:25)
    at runTask (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1662:17)
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1602:17
    at processQueue (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1612:17)
    at Object.auto (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1598:9)
    at Hook.load (/home/sadasant/code/github.com/project/node_modules/sails/lib/hooks/index.js:73:13)
    at Hook.wrapper [as load] (/home/sadasant/code/github.com/project/node_modules/@sailshq/lodash/lib/index.js:3250:19)
    at loadHook (/home/sadasant/code/github.com/project/node_modules/sails/lib/app/private/loadHooks.js:206:17)
    at /home/sadasant/code/github.com/project/node_modules/sails/lib/app/private/loadHooks.js:328:13
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:3047:20
    at eachOfArrayLike (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1002:13)
    at eachOf (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:1052:9)
    at Object.eachLimit (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:3111:7)
    at load (/home/sadasant/code/github.com/project/node_modules/sails/lib/app/private/loadHooks.js:326:17)
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:3671:13
    at replenish (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:884:21)
    at iterateeCallback (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:869:21)
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:847:20
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:3676:17
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:339:31
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:952:25
    at iteratorCallback (/home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:997:17)
    at /home/sadasant/code/github.com/project/node_modules/sails/node_modules/async/dist/async.js:847:20
    at _combinedTickCallback (internal/process/next_tick.js:131:7)
    at process._tickCallback (internal/process/next_tick.js:180:9)
    at Function.Module.runMain (module.js:611:11)
    at startup (bootstrap_node.js:158:16)
    at bootstrap_node.js:578:3
error: A hook (`services`) failed to load!
error: A hook (`orm`) failed to load!

Match against undefined or null is just when we try to deconstruct Sails to get a configuration value in a service file.

My code looks like this:

let sails = require('sails')
let {config: { url: websiteUrl }} = sails

I tried removing let sails = require('sails') but it then it says that sails is undefined 🤔 however, it should be globally available, right?

Node 8.2.0 works perfectly though.

Also: Doing require('sails') and then using sails.<property> from a function and not at load time works.

Edit: I had 1.0.0-33 at the beginning, then upraded to 1.0.0-36 and I get the same.

@sailsbot
Copy link

@sadasant Thanks for posting, we'll take a look as soon as possible.


For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her minions in Austin, click here.

@sgress454
Copy link
Member

@sadasant I haven't been able to reproduce your issue, but in general it's not recommended that you rely on the Sails object while files are being loaded (i.e. outside of the module.exports). Differences in how Node and require loads files could cause things to be referenced before they're initialized.

If you're implementing a service that needs some initialization, you might consider changing it to be a hook, which has built-in support for configuration using the Sails app object. If you want to keep it as a service, you have several options, but in general they all revolve around making the service itself a function that returns an object, e.g.:

// api/services/IntercomService.js

// Reference to some third-party library that needs initializing.
var intercomLib;

// The actual service methods.
var serviceMethods = {

   doSomething: function() { intercomLib.doAThing(); },

   doSomethingElse: function() { ... }

};

module.exports = function() {
   // Initialize the library if necessary.
   if (!intercomLib) {
      intercomLib = require('intercom-lib')(sails);
   }

   // Return the service methods, which are defined above to avoid creating
   // a new object every time the service is invoked.
   return serviceMethods;
};

Then in your app would call IntercomService().doSomething();

All that being said, if you think there's an actual bug here and you can post a link to a repo that reliably reproduces it, we can check it out. How have you test 8.2.0 vs 8.3.0? Are you using nvm?

@sgress454 sgress454 added the repro please Could you reproduce this in a repository for us? label Aug 11, 2017
@sadasant
Copy link
Author

@sgress454 My friend @sean-clayton will follow up.

@sailsbot sailsbot removed the repro please Could you reproduce this in a repository for us? label Aug 11, 2017
@sean-clayton
Copy link

sean-clayton commented Aug 11, 2017

@sgress454 we're also getting this issue:

TypeError: Cannot read property 'user' of undefined

from this code:

module.exports.login = (...params) => login(sails.models.user.login)(...params)

Is this not a proper way of referencing a sails model?

@sgress454
Copy link
Member

@sean-clayton It is, and again I can't reproduce these issues, so it would really be helpful to have a repo I could check out. I'd also try wiping and re-installing your node modules; especially if you're using NVM there could be some conflicts with global modules.

@sgress454 sgress454 added the repro please Could you reproduce this in a repository for us? label Aug 11, 2017
@sean-clayton
Copy link

@sgress454 I have dug up some more and found what we think is a bug, either in Sails or Node 8.3.0

Here is a reproducible case. I've included our theory and instructions in the project's README.md
https://github.com/sean-clayton/sails-node-8-3-0

If you have any questions, please feel free to ask away 😸

@sailsbot sailsbot removed the repro please Could you reproduce this in a repository for us? label Aug 14, 2017
@daedalus28
Copy link

@sgress454 to be very specific, there seems to be a bug on node 8.3 when a service requires a service that requires sails, which prevents sails.models from working at all.

@sgress454
Copy link
Member

@sean-clayton Thanks for the repo. This doesn't appear to have anything to do with services; it looks to me like the issue is that in Node 8.3.0, require('sails') gives you a new Sails instance even if called from within the context of a lifted app. That is, you do sails console and:

> var x = require('sails')
> x === sails

you'll get false, when it should be true. If you do:

> var x = require('sails')
> var y = require('sails')
> x === y

you do get true, so it's not a general problem with the require cache, but rather that somewhere the global sails reference is getting detached from the instance in the cache.

I'll keep looking into this, but in the meantime, in your specific case, the answer is just to not use require('sails') in your code -- you shouldn't be doing it anyway. If you have globals turned on, just use the global sails, or else in controllers you can use req._sails. As I said above (and as this issue demonstrates in bizarre fashion), it's not recommended that you try to access sails outside of a module context.

@mikermcneil
Copy link
Member

so it's not a general problem with the require cache, but rather that somewhere the global sails reference is getting detached from the instance in the cache.

That's really strange... and worth looking into why it's happening only in Node 8.3.0.

Maybe it could be a difference in how dependencies are getting picked up? Like, do we see this same behavior when using sails lift (the CLI) vs just running node app?

@sean-clayton
Copy link

do we see this same behavior when using sails lift (the CLI) vs just running node app?

I just tested my example project and it still has the same error when running with sails lift in a terminal window.

@mikermcneil
Copy link
Member

@mikermcneil
Copy link
Member

mikermcneil commented Aug 15, 2017

hmmm nodejs/node@eb023ef7df (it's unrelated, but it made me notice the Module vs require('module') thing)

@mikermcneil
Copy link
Member

@mikermcneil
Copy link
Member

@sean-clayton
Copy link

@mikermcneil I tried out using the global sails—here's the diff:

diff --git a/api/services/Test2Service.js b/api/services/Test2Service.js
index f95a209..45d0250 100644
--- a/api/services/Test2Service.js
+++ b/api/services/Test2Service.js
@@ -1 +1 @@
-let sails = require('sails')
+let sails = sails

And I'm getting this error when starting the application with node app.js:

ReferenceError: sails is not defined

@mikermcneil
Copy link
Member

@sean-clayton quick update: @sgress454 just stumbled upon this: nodejs/node#14132

Looks like the inconsistency is indeed related to the cache busting in include-all (it's there to allow for live reloading backend code). I'll let Scott go into the details as-needed, but the short answer is that, in node 8.3, the behavior for "children" in the require cache changed. We're kicking around some ways we could make Sails not rely on this, because obviously we don't want to be dependent on a feature of Node.js that might very well change again without a major version bump...

@sgress454
Copy link
Member

@sean-clayton re: global sails -- Sails isn't globalized at the time that services are loaded, but again, if you just do your work inside of module.exports, it should work fine.

@mattbrun
Copy link

mattbrun commented Sep 15, 2017

Hi guys,
I confirm a similar issue with node 8.4.0 and 8.5.0.
In my case though my code works fine in developer mode, and crashes when I run it with NODE_ENV=production.

I get the error during the application lift when requiring some files which are required inside my hooks init function: TypeError: Cannot read property 'XXX' of undefined.
My code looks like

const sails = require('sails')
const CONST = sails.config.XXX

Where XXX config is defined in the file config/XXX.js as all other sails configs (module.exports.XXX = {}).

Thanks for your work!
M.

UPDATE: I forgot to mention that the same code run with NODE_ENV=production works with node 8.1.x an 6.10.3

@mikermcneil
Copy link
Member

@mattbrun thanks for the report!

@mikermcneil mikermcneil changed the title Sails 1.0.0-33 doesn't boot on Node 8.3.0 Sails 1.0.0-33 doesn't boot on Node 8.3.0 when accessing require('sails').config Oct 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants