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

[bugfix] display stderr on command failure #303

Merged

Conversation

DerekTBrown
Copy link
Contributor

@DerekTBrown DerekTBrown commented Nov 17, 2022

Repro

If you run the SDK CLI against a broken Typescript file, nothing useful is outputted:

(base) ~/D/C/n/near-sdk-js ❯❯❯ ./lib/cli/cli.js build --verbose ./examples/src/log.ts                                                                                       ✘ 1 
…  awaiting  Building ./examples/src/log.ts contract...
…  awaiting  Typechecking ./examples/src/log.ts with tsc...
[exec] › ℹ  info      Running command: node_modules/.bin/tsc  --skipLibCheck --experimentalDecorators --target es2020 --moduleResolution node ./examples/src/log.ts
[exec] › ✖  error     Error: Command failed: node_modules/.bin/tsc  --skipLibCheck --experimentalDecorators --target es2020 --moduleResolution node ./examples/src/log.ts 

    at ChildProcess.exithandler (node:child_process:398:12)
    at ChildProcess.emit (node:events:527:28)
    at maybeClose (node:internal/child_process:1092:16)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:302:5)

Root Cause

  • The promisified childProcess.exec (invoked here) throws an error if the command returns a non-zero code.
  • This error contains stdout and stderr information, but this isn't printed to console in the catch block.

Fix

This PR changes the CLI behavior to print stderr and stdout when a command exits with a non-zero exit code. This is useful for debugging and diagnosis. This is done by unifying the try/catch paths somewhat.

Copy link
Member

@ailisp ailisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix looks reasonable to me. Thanks @DerekTBrown !

@DerekTBrown
Copy link
Contributor Author

@ailisp I don't have write access to the repo. Can you merge?

@ailisp ailisp merged commit 852bf34 into near:develop Nov 22, 2022
@ailisp
Copy link
Member

ailisp commented Nov 22, 2022

@DerekTBrown Done. Thank you!

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

Successfully merging this pull request may close these issues.

2 participants