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

Production servers do not respect package-lock #1988

Closed
paulmelnikow opened this issue Aug 28, 2018 · 4 comments
Closed

Production servers do not respect package-lock #1988

paulmelnikow opened this issue Aug 28, 2018 · 4 comments
Labels
operations Hosting, monitoring, and reliability for the production badge servers

Comments

@paulmelnikow
Copy link
Member

https://sentry.io/shields/shields/issues/664377681/

AssertionError [ERR_ASSERTION]: Invalid regular expression
  File "/home/m/shields/services/pypi/pypi-base.js", line 15, in Object.<anonymous>
    .pattern(
  File "/home/m/shields/services/pypi/pypi-djversions.service.js", line 3, in Object.<anonymous>
    const PypiBase = require('./pypi-base')
  File "/home/m/shields/services/index.js", line 9, in glob.sync.map.path
    .map(path => require(path))
  File "/home/m/shields/services/index.js", line 9, in loadServiceClasses
    .map(path => require(path))
  File "/home/m/shields/server.js", line 116, in Object.<anonymous>
    loadServiceClasses().forEach(
...
(24 additional frame(s) were not displayed)

AssertionError [ERR_ASSERTION]: Invalid regular expression

It crashed the server repeatedly on startup, which caused a couple minutes of downtime on s0.

This isn't manifesting in any of the other environments. The server might be running a different version of joi. According to Sentry, the server is running joi 13.3.0, not 13.6.0 as the lockfile declares. I don't know how the server installs its dependencies and am not sure how to verify exactly what is being installed. I've no reason to believe that API has changed, though.

@paulmelnikow paulmelnikow added bug Bugs in badges and the frontend operations Hosting, monitoring, and reliability for the production badge servers labels Aug 28, 2018
@paulmelnikow
Copy link
Member Author

Aha, according to Sentry, the copy of node_modules/joi/lib/types/object/index.js that's being used precedes hapijs/joi@d97ca0d#diff-3063cd3662b8c3bd5de2439f2ef8ee68L441. Accepting a Joi contract instead of a pattern was indeed new in 13.6.0.

I'm a bit flummoxed that the server isn't using package-lock. It makes the careful dependency reviewing we're doing a bit pointless… or at least not as useful as it might be! It looks like it's running npm install, which I think would look at the lockfile. Maybe it's a very old version of npm.

I guess the short-term fix is to specify a specific joi version in package.json.

The long term fix would be to honor the lockfile, though I think @espadrine is the only person who has access to make the necessary changes.

@paulmelnikow paulmelnikow changed the title Release blocker: Puzzling pypi crash in production Production servers do not respect package-lock Aug 28, 2018
paulmelnikow added a commit that referenced this issue Aug 28, 2018
@chris48s
Copy link
Member

Support for package-lock.json was added in npm 5. My theory would be that we're still on some npm 4.x version. I don't have access to the shields sentry but is there anything in the env debug info in sentry that we can use to check that?

@paulmelnikow
Copy link
Member Author

Good though on checking Sentry, which says we are on Node v9.4.0. I thought we were on Node 8, though I seem to be mis-remembering.

According to this reference Node v9.4.0 ships with npm 5.6.0, although it's possible that there are more than one copy of Node installed, and the post-receive hook is running a different one.

npm isn't part of the environment where Sentry so unfortunately I don't see any way of telling the npm version.

Note that we're also installing the dev dependencies, which is not required, and unnecessarily slows down the deploy process.

By the way, #1989 has fixed the crasher; I've got one server online running 16d4157.

@paulmelnikow
Copy link
Member Author

This is resolved via #4929.

😌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
operations Hosting, monitoring, and reliability for the production badge servers
Projects
None yet
Development

No branches or pull requests

2 participants