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

cldr-data installation fails when request is present in package.json #33

Open
perazzi opened this issue Apr 27, 2016 · 10 comments
Open

Comments

@perazzi
Copy link

perazzi commented Apr 27, 2016

For some weird reason, the deployment of my app started failing today.
Nothing changed in the code. I'm trying to deploy the same commit that was deployed yesterday, but I'm getting now errors on cldr-data npm's installation.

It happens when my package.json has both request and cldr-data as dependencies.

The error changes from time to time, but mostly, it says

> [email protected] install /Users/andreperazzi/Documents/IBM/Projects/Web Innovations/ifundit/ifundit/node_modules/cldr-data
> node install.js

module.js:338
    throw err;
          ^
Error: Cannot find module 'boom'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:278:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/andreperazzi/Documents/IBM/Projects/Web Innovations/ifundit/ifundit/node_modules/request/node_modules/hawk/lib/index.js:3:33)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)`

I found this closed issue: #11

It is true for me that running npm install request before npm install solves the issue, however, I'm deploying my code to IBM Bluemix, and I can't manually run npm install. The module must be inside package.json

I have no idea why this started happening today, with no code changes.

Also, I was able to reproduce the issue by just creating an empty app and trying npm install (run npm cache clear first:

package.json:

{
  "name": "tst",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "cldr-data": "^28.0.2",
    "request": "^2.40.0"
  }
}
@rxaviers
Copy link
Owner

I could not reproduce your bug. I'm running npm 3.3.6, node 5, on linux. I've created a brand new directory, created a package.json with the contents above and ran npm install, it worked just fine.

Are you experiencing this problem on your machine as well as in your deploy at bluemix? What's your setup? It's indeed a weird error.

PS: FWIW, cldr-data uses cldr-data-downloader, which depends on request": "2.72.0".

@perazzi
Copy link
Author

perazzi commented Apr 28, 2016

This problem was happening with everyone in my team.
I was able to fix the problem by removing the ^ character before one of the module versions.
If I leave the character on both, the installation fails.

@rxaviers
Copy link
Owner

Please which OS, npm, and node versions are you using?

@perazzi
Copy link
Author

perazzi commented Apr 28, 2016

Locally we're all using:
Mac OS El Capitan
NPM 2.11.3
Node v0.12.7

On Bluemix, we're using:

"engines": {
    "node": "0.12"
  }

I'm not sure which is the NPM version on Bmx, trying to figure out...

@mike14511
Copy link

Also experiencing this problem. FWIW, Bluemix will default to npm version: 2.14.12.

@rxaviers
Copy link
Owner

rxaviers commented May 3, 2016

I havent had time myself to debug this, but I'm definitely open for bug fixes.

@jasonkarns
Copy link

jasonkarns commented Jun 22, 2016

@jasonkarns
Copy link

jasonkarns commented Jun 23, 2016

I suspect what's happening is:

  • npm begins downloading direct deps in parallel
  • npm sees that request can be deduped so it is not installed as a transitive dep of cldr-data
  • npm finishes cldr-data download (and including its deps) and kicks off install and postinstall hooks before request is finished installing
  • cldr-data install hook requires cldr-downloader which requires request which isn't yet completely installed
  • error is thrown when request attempts to load one of its deps that isn't yet installed

This explains why it only occurs with npm2 (since npm3 has rewritten most of the deduping and builds a full dep tree before running); and it explains why the error message varies slightly each time, since it is nondeterministic which (if any) of request's deps aren't fully installed at the time cldr-data's install script runs.

Unfortunately, the postinstall hook doesn't run at a meaningfully different time, so doing the node install at postinstall time doesn't solve this issue. Though I'm curious why this step isn't part of prepublish. It's pretty typical that any non-architecture-dependent tasks be run at prepublish time. From npm's docs:

If you need to perform operations on your package before it is used, in a way that is not dependent on the operating system or architecture of the target system, use a prepublish script. This includes tasks such as:

  • Compiling CoffeeScript source code into JavaScript.
  • Creating minified versions of JavaScript source code.
  • Fetching remote resources that your package will use.

(emphasis mine)

And even stronger language from the docs:

Don't use install. Use a .gyp file for compilation, and prepublish for anything else. You should almost never have to explicitly set a preinstall or install script. If you are doing this, please consider if there is another option. The only valid use of install or preinstall scripts is for compilation which must be done on the target architecture.

Downloading the CLDR data is precisely what prepublish is intended for and would certainly solve this issue. Why is the node install script run at install time and not prepublish?

@jasonkarns
Copy link

Here are the relevant issues on npm for this bug (it is, after all, npm's bug)

npm/npm#8850
npm/npm#4134
npm/npm#4096
npm/npm#8152

The fix is not being backported to npm2. The workarounds as mentioned in npm/npm#4134 and npm/npm#8850 are:

  • use prepublish instead of install as recommended by npm or
  • use npm3

@puzrin
Copy link
Contributor

puzrin commented Mar 21, 2017

I confirm, had this problem with npm2, and no issues with npm3.

Probably almost all moved to node 6+ for es2015+, and this issue can be closed as "wontfix / outdated"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants