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

pnpm audit process exits before flushing stdout #3526

Closed
mdurling opened this issue Jun 10, 2021 · 6 comments · Fixed by #3951
Closed

pnpm audit process exits before flushing stdout #3526

mdurling opened this issue Jun 10, 2021 · 6 comments · Fixed by #3951

Comments

@mdurling
Copy link

pnpm version: 6.7.4

Code to reproduce the issue:

import { execSync } from 'child_process'

try {
  execSync('pnpm audit --json --audit-level=low')
} catch ({ stdout }) {
  const json = stdout.toString('utf-8')
  console.log('BYTES', json.length)
  console.log('JSON', json)
}

Expected behavior:

The output of the child process running pnpm audit is available.

Actual behavior:

The output of the child process is truncated (if the output is greater than 8192 bytes).

Additional information:

This problem only seems to occur on MacOS. I believe it's because the pnpm CLI calls process.exit() prior to stdout being fully flushed. See output truncated at 8192 bytes below:

Screen Shot 2021-06-10 at 9 45 26 AM

  • node -v prints: v15.11.0
  • Windows, macOS, or Linux?: MacOS

Here's an example of how someone else resolved using process.exitCode = 1 rather than process.exit(1).

@zkochan
Copy link
Member

zkochan commented Nov 4, 2021

Good investigation!

Probably we can do this change

@m1heng
Copy link
Member

m1heng commented Nov 4, 2021

should we define a specific Error for process exit, and have extra logic in packages/pnpm/src/pnpm.ts

@zkochan
Copy link
Member

zkochan commented Nov 4, 2021

I think all the exit calls happen in this file https://github.com/pnpm/pnpm/blob/main/packages/pnpm/src/main.ts

@m1heng
Copy link
Member

m1heng commented Nov 4, 2021

I think all the exit calls happen in this file https://github.com/pnpm/pnpm/blob/main/packages/pnpm/src/main.ts

I was think all internal exit should be replaced with throw error and being handled in pnpm.ts/pnpx.ts since they already have error handling logics.

async function runPnpm () {
const errorHandler = (await import('./err')).default
try {
const main = (await import('./main')).default
await main(argv)
} catch (err: any) { // eslint-disable-line
errorHandler(err)
}
}

@zkochan
Copy link
Member

zkochan commented Nov 4, 2021

No, that is not a good idea because some commands don't print an error just exit with a 1 exit code.

@m1heng
Copy link
Member

m1heng commented Nov 6, 2021

I made a PR just fixing exit in packages/pnpm/src/main.ts.
But there are still exit logic packages/plugin-commands-publishing/src/publish.ts and packages/plugin-commands-script-runners/src/exec.ts, I am not really sure if these should be changed too.

@zkochan zkochan added this to the v6.20 milestone Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants