require('gcloud') causing forge to crash #362

musicalshore · 2016-02-16T20:34:04Z

My app suddenly started crashing and I've narrowed it down to a require('gcloud') on a server controller. This happened after merging from another branch. I've looked and looked, tried deleting and reinstalling all node_modules and nothing has helped. There are no differences between the package.json files on either branch. I am using Node version 5.0.0 and I have tried using gcloud 0.24.0, 0.25.1 and 0.28.0.

app/services/node_modules/node-forge/js/forge.js:41 mods[i](forge);

TypeError: mods[i] is not a function at Object.module.exports (/Users/myhome/Development/app/services/node_modules/node-forge/js/forge.js:41:14) at defineFunc (/Users/myhome/Development/app/services/node_modules/node-forge/js/forge.js:47:10) at Object.<anonymous> (/Users/myhome/Development/app/services/node_modules/node-forge/js/forge.js:90:14) at global.define (/Users/myhome/Development/app/services/node_modules/amd-loader/amd-loader.js:79:28) at define (/Users/myhome/Development/app/services/node_modules/node-forge/js/forge.js:57:17) at /Users/myhome/Development/app/services/node_modules/node-forge/js/forge.js:59:1 at Object.<anonymous> (/Users/myhome/Development/app/services/node_modules/node-forge/js/forge.js:92:3) at Module._compile (module.js:425:26) at Module.module.constructor._compile (/Users/myhome/Development/app/services/node_modules/amd-loader/amd-loader.js:10:31) at Object.Module._extensions..js (module.js:432:10) at Module.load (module.js:356:32) at Function.Module._load (module.js:311:12) at Module.require (module.js:366:17) at require (module.js:385:17) at Object.<anonymous> (/Users/myhome/Development/app/services/node_modules/google-p12-pem/index.js:1:75) at Module._compile (module.js:425:26) at Module.module.constructor._compile (/Users/myhome/Development/app/services/node_modules/amd-loader/amd-loader.js:10:31) at Object.Module._extensions..js (module.js:432:10) at Module.load (module.js:356:32) at Function.Module._load (module.js:311:12) at Module.require (module.js:366:17) at require (module.js:385:17) at Object.<anonymous> (/Users/myhome/Development/app/services/node_modules/gtoken/lib/index.js:1:77) at Module._compile (module.js:425:26) at Module.module.constructor._compile (/Users/myhome/Development/app/services/node_modules/amd-loader/amd-loader.js:10:31) at Object.Module._extensions..js (module.js:432:10) at Module.load (module.js:356:32) at Function.Module._load (module.js:311:12) at Module.require (module.js:366:17) at require (module.js:385:17) at Object.<anonymous> (/Users/myhome/Development/app/services/node_modules/google-auth-library/lib/auth/jwtclient.js:20:14) at Module._compile (module.js:425:26) at Module.module.constructor._compile (/Users/myhome/Development/app/services/node_modules/amd-loader/amd-loader.js:10:31) at Object.Module._extensions..js (module.js:432:10) at Module.load (module.js:356:32) at Function.Module._load (module.js:311:12) at Module.require (module.js:366:17) at require (module.js:385:17) at Object.<anonymous> (/Users/myhome/Development/app/services/node_modules/google-auth-library/lib/auth/googleauth.js:19:17) at Module._compile (module.js:425:26) at Module.module.constructor._compile (/Users/myhome/Development/app/services/node_modules/amd-loader/amd-loader.js:10:31) at Object.Module._extensions..js (module.js:432:10) at Module.load (module.js:356:32) at Function.Module._load (module.js:311:12) at Module.require (module.js:366:17) at require (module.js:385:17)

The text was updated successfully, but these errors were encountered:

dshishkov · 2016-02-16T21:37:50Z

The issue occurs specifically in /js/forge.js

var defineFunc = function(require, module) {
        module.exports = function(forge) {
            var mods = deps.map(function(dep) {
                return require(dep);
            });
            // handle circular dependencies
            forge = forge || {};
            forge.defined = forge.defined || {};
            if(forge.defined[name]) {
                return forge[name];
            }
            forge.defined[name] = true;
            for(var i = 0; i < mods.length; ++i) {
                // NOTE: this works fine when the node app is not using AMD/RequireJS. In our case however we use RequireJS server side so we get the error above
                mods[i](forge);
            }
            return forge;
        };
        // set to true to disable native code if even it's available
        module.exports.disableNativeCode = false;
        module.exports(module.exports);
    };

musicalshore · 2016-02-24T19:10:13Z

bump

alexewerlof · 2016-10-18T11:58:48Z

bump

dlongley · 2016-10-18T16:15:01Z

We hope to get this fixed per #357 (comment).

mdebruijne · 2016-11-30T18:33:47Z

@dlongley

Would you be so kind to apply the 4 lines patch from #445?

The patch fixes this bug and other forge on Node.js bugs.

davidlehn · 2016-12-07T18:23:21Z

The patch from #445 was released in 0.6.46 since it hopefully won't cause other problems. But I was unable to reproduce the issue with basic require tests. Can someone please post a test case that fails? If not, can someone use console.log or similar and report what non-function data is causing the failure? It would be good to fix the cause of this problem rather than depend on the hack to fix it.

mdebruijne · 2016-12-08T12:33:43Z

@davidlehn

Thanks for merging and the quick release!

Steps to reproduce;

run npm install jest node-forge
create a file __tests__/node-forge.js with the following content require('node-forge')
execute jest

If you are using Visual Studio Code, you can use this launch.json for a matching debugging environment;

{
  "version": "0.2.0",
  "configurations": [
    {
      "cwd": "${workspaceRoot}",
      "name": "Debug with Node",
      "program": "${workspaceRoot}/__tests__/node-forge.js",
      "request": "launch",
      "type": "node"
    },
    {
      "cwd": "${workspaceRoot}",
      "name": "Debug with Jest",
      "request": "launch",
      "runtimeArgs": [
        "--debug-brk",
        "./node_modules/.bin/jest",
        "-i",
        "__tests__/node-forge.js"
      ],
      "type": "node"
    }
  ]
}

I have done some quick debugging. Jest is wrapping require to check if mocking is required. This wrapped require is being called at line 31 return require(dep); in forge.js. With Jest it returns an empty object. With native Node.js it returns a function.

See also this Jest issue report; jestjs/jest#1946

gschier · 2016-12-15T23:26:31Z

@mdebruijne have you been able to get this working with Jest yet? I've become stuck on it as well.

davidlehn · 2016-12-15T23:38:41Z

@gschier The latest forge release fixed the simple jest + node-forge test case shown above. Is there some other simple test case that also fails?

mdebruijne · 2016-12-16T00:24:48Z

@gschier

Yes, this has been fixed, everything is working as expected now.

There are two things you should check;

gcloud (https://www.npmjs.com/package/gcloud) has been replaced by google-cloud (https://www.npmjs.com/package/google-cloud) a couple of months ago.
Google Cloud Node.js Client shrinkwraps dependencies, so you need to pull in at least version 0.45.0 of google-cloud or the corresponding version of the direct modules to get the correct version of node-forge.

gschier · 2016-12-16T01:28:07Z

@davidlehn @mdebruijne thanks for the responses.

I'm using forge directly (not through gcloud). It works fine in the regular NodeJS and I no longer receive the above error when using Jest. But, in Jest, the sub objects are now missing.

Here's what I mean.

const forge = require('node-forge')();

console.log(forge.random); // > undefined
console.log(forge.util); // > undefined
// the list goes on ...

All of the root-level functions (that I'm using) seem to work fine though. Any insight as to why this might be happening?

I can try putting together a simple test project tonight or tomorrow if that will help.

mdebruijne · 2016-12-16T02:35:16Z

@gschier

The difference in your case vs the google-cloud case is that you are actually using this code path from node-forge. The google-cloud package only use node-forge for a tool to convert PKCS12/P12 to PKCS1/PEM. This is a one time operation and therefore isn't something that you normally use in code and needs to be unit tested.

The require statements in forge.js are still empty objects so the underlying problem hasn't been fixed at all. The only thing the pull request does is prevent crashes for applications that don't use this specific code path. If your application needs this code path then it is still broken.

davidlehn · 2016-12-16T02:43:53Z

For those who want a test case, mkdir a dir and:

npm install node-forge jest

Add a __tests__/test.js file:

const forge = require('node-forge');
require('jest');

describe('forge', () => {
  it('forge', () => {expect(forge).toBeDefined()})
  it('forge.random', () => {expect(forge.random).toBeDefined()})
  it('forge.util', () => {expect(forge.util).toBeDefined()})
});

node ./node_modules/.bin/jest

Yeah, it's failing.

I just a bit ago I added another attempt at converting forge to CommonJS style. Testing with that work-in-progress and the above test works fine. So this will fix itself at some point after that cjs branch or similar gets merged and released.

#456

gschier · 2016-12-16T03:53:00Z

@davidlehn that PR looks awesome! So excited for that.

dlongley · 2017-01-11T16:13:14Z

Should be addressed in a new release now that #456 has been merged.

gschier · 2017-01-12T06:32:04Z

That's great news! 👏

musicalshore mentioned this issue Feb 16, 2016

require('gcloud') crashing node app googleapis/google-cloud-node#1129

Closed

davidlehn mentioned this issue Dec 7, 2016

Fix Node module loading #445

Merged

dlongley closed this as completed Jan 11, 2017

davidlehn added this to the v0.7.0 milestone Jan 12, 2017

jasonblanchard mentioned this issue May 12, 2017

xml-encryption update to fix Jest bug Clever/saml2#107

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

require('gcloud') causing forge to crash #362

require('gcloud') causing forge to crash #362

musicalshore commented Feb 16, 2016

dshishkov commented Feb 16, 2016

musicalshore commented Feb 24, 2016

alexewerlof commented Oct 18, 2016

dlongley commented Oct 18, 2016

mdebruijne commented Nov 30, 2016

davidlehn commented Dec 7, 2016

mdebruijne commented Dec 8, 2016

gschier commented Dec 15, 2016

davidlehn commented Dec 15, 2016

mdebruijne commented Dec 16, 2016

gschier commented Dec 16, 2016 •

edited

Loading

mdebruijne commented Dec 16, 2016 •

edited

Loading

davidlehn commented Dec 16, 2016

gschier commented Dec 16, 2016

dlongley commented Jan 11, 2017

gschier commented Jan 12, 2017

require('gcloud') causing forge to crash #362

require('gcloud') causing forge to crash #362

Comments

musicalshore commented Feb 16, 2016

dshishkov commented Feb 16, 2016

musicalshore commented Feb 24, 2016

alexewerlof commented Oct 18, 2016

dlongley commented Oct 18, 2016

mdebruijne commented Nov 30, 2016

davidlehn commented Dec 7, 2016

mdebruijne commented Dec 8, 2016

gschier commented Dec 15, 2016

davidlehn commented Dec 15, 2016

mdebruijne commented Dec 16, 2016

gschier commented Dec 16, 2016 • edited Loading

mdebruijne commented Dec 16, 2016 • edited Loading

davidlehn commented Dec 16, 2016

gschier commented Dec 16, 2016

dlongley commented Jan 11, 2017

gschier commented Jan 12, 2017

gschier commented Dec 16, 2016 •

edited

Loading

mdebruijne commented Dec 16, 2016 •

edited

Loading