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

Couldn't generate the package-lock.json file for certain projects #62

Closed
jadoonf opened this issue Feb 10, 2023 · 6 comments · Fixed by #68
Closed

Couldn't generate the package-lock.json file for certain projects #62

jadoonf opened this issue Feb 10, 2023 · 6 comments · Fixed by #68
Assignees
Labels
wontfix This will not be worked on

Comments

@jadoonf
Copy link
Member

jadoonf commented Feb 10, 2023

Describe the bug

Facing the below error when scanning certain project repos (e.g. tier and n8n)

Error: couldn't generate the package-lock.json file for certain projects

Steps to reproduce the behavior

  1. Clone the project repo
  2. Run lstn in inside the project root
  3. View the output Error: couldn't generate the package-lock.json file

Environment

  • lstn version: 807cfc9.20230209
  • go version: go1.19.4 darwin/arm64
  • macOS Monterey 12.0.1

Expected vs actual behavior

Currently, if you run lstn to tier --json it works.
Running lstn in for a project's package.json should return the same output as using lstn to <package>.

@jadoonf jadoonf added the bug Something isn't working label Feb 10, 2023
@github-actions github-actions bot added the needs-triage Mantainers team needs to take a first look at the issue label Feb 10, 2023
@didof didof self-assigned this Feb 10, 2023
@didof
Copy link
Contributor

didof commented Feb 10, 2023

Thanks for reporting, @jadoonf!

Bug replicated. Working on it.

@didof
Copy link
Contributor

didof commented Feb 10, 2023

Let's analyze one repo at a time.

n8n

In this repo, there is a block-npm-install.js registered on the preinstall hook.

Screenshot 2023-02-10 at 14 06 54

Screenshot 2023-02-10 at 14 07 13

This leaves the npmPackageLockOnly.Run() hanging. One could argue the script actually process.exit(1)s, thus the error should be returned into go.

Still, we can see how the program fails because of reaching the deadline (1 minute at the time of writing).

Screenshot 2023-02-10 at 14 16 23

My first guess is that the preinstall runs the script on another thread (unsupported by facts at this time), so the exit code 1 is not picked up by the thread that spawned npm install --package-lock-only --no-audit. However, it is not properly closed and hangs until the timeout chimes.

I will investigate if my conjecture is right.

My proposal

Assuming my analysis is correct, here is how I would approach a solution.

  1. looks for all possible ways by which it is possible to intercept npm install (let's assume is only preinstall - it may be for what I currently know)
  2. If one of the found in point 1 is present in the package.json, replicate a tmp version of it without those, temporarily changing the name of the original package.json
  3. catch up with the flow: hence lstn will internally run npm i --package-lock-only --no-audit
  4. remove tmp package.json, and restore the name of the original

I recognize this approach to be very naïve and I am sure it can be improved by bringing it to the attention of @leodido.

@didof
Copy link
Contributor

didof commented Feb 10, 2023

Let's move to the other repo

node-sdk

Similarly to preinstall in the previous case, here we have instead prepare. If I am not mistaken, it assumes that tsc is installed globally (otherwise it would be npx tsc).

Hence the cli fails.

Screenshot 2023-02-10 at 14 37 42

My proposal

Same as the previous comment, replacing prepare other than preinstall

@leodido
Copy link
Member

leodido commented Feb 10, 2023

This is not a lstn bug.

It's not even an npm bug.

This is what I get by npm installing tier: error. So how could possibly lstn do it? :)

Screenshot 2023-02-10 at 15 59 21

@leodido
Copy link
Member

leodido commented Feb 10, 2023

For lstn (and npm) to work the package.json file MUST be both syntactically and semantically valid.

If it assumes that a tool is installed globally and then it runs it via a preinstall (or whatever) script, then it's semantically broken, and even npm breaks (correctly).

@leodido leodido removed the bug Something isn't working label Feb 10, 2023
@leodido
Copy link
Member

leodido commented Feb 10, 2023

Same thing for n8n

Screenshot 2023-02-10 at 16 10 15

Closing.

@leodido leodido closed this as completed Feb 10, 2023
@leodido leodido mentioned this issue Feb 10, 2023
@didof didof linked a pull request Feb 10, 2023 that will close this issue
4 tasks
@leodido leodido added wontfix This will not be worked on and removed needs-triage Mantainers team needs to take a first look at the issue labels Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants