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

non-zero exit codes for npm run-script build? #252

Closed
weisjohn opened this issue Jul 28, 2016 · 8 comments · Fixed by #256
Closed

non-zero exit codes for npm run-script build? #252

weisjohn opened this issue Jul 28, 2016 · 8 comments · Fixed by #256

Comments

@weisjohn
Copy link
Contributor

When I try to npm run-script build, if the process fails, I would expect it to return a non-zero exit code. Example:

john at Johns-MacBook-Pro-4 in ~/mysrc/weisjohn/scratch/test/hello-world                                         
$ npm run-script build

> [email protected] build /Users/john/mysrc/weisjohn/scratch/test/hello-world
> react-scripts build

Failed to create a production build. Reason:
Module build failed: SyntaxError: /Users/john/mysrc/weisjohn/scratch/test/hello-world/src/App.js: Identifier directly after number (5:2)
  3 | import './App.css';
  4 | 
> 5 | 10x;
    |   ^
  6 | 
  7 | class App extends Component {
  8 |   render() {
    at Parser.pp.raise (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/location.js:22:13)
    at Parser.readNumber (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:674:78)
    at Parser.getTokenFromCode (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:516:23)
    at Parser.readToken (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:180:21)
    at Parser.<anonymous> (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/plugins/jsx/index.js:51:20)
    at Parser.readToken (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/plugins/flow.js:170:22)
    at Parser.nextToken (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:169:21)
    at Parser.next (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:81:12)
    at Parser.eat (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:90:14)
    at Parser.pp.isLineTerminator (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/util.js:69:15)
    at Parser.pp.semicolon (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/util.js:76:13)
    at Parser.pp.parseImport (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:1011:8)
    at Parser.pp.parseStatement (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:141:56)
    at Parser.parseStatement (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/plugins/flow.js:30:22)
    at Parser.pp.parseBlockBody (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:529:21)
    at Parser.pp.parseTopLevel (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:36:8)
john at Johns-MacBook-Pro-4 in ~/mysrc/weisjohn/scratch/test/hello-world                                         
$ echo $?
0

One of the reasons I want this is to utilize npm run-script build in a pre-commit hook as follows:

#!/bin/sh
# Exit at first failure
set -e
npm run-script build
@weisjohn
Copy link
Contributor Author

Doing some more digging, if I invoke the script directly, then I do get the exit code:

$ node node_modules/react-scripts/scripts/build.js 
Failed to create a production build. Reason:
Module build failed: SyntaxError: /Users/john/mysrc/weisjohn/scratch/test/hello-world/src/App.js: Identifier directly after number (5:2)
  3 | import './App.css';
  4 | 
> 5 | 10x;
    |   ^
  6 | 
  7 | class App extends Component {
  8 |   render() {
    at Parser.pp.raise (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/location.js:22:13)
    at Parser.readNumber (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:674:78)
    at Parser.getTokenFromCode (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:516:23)
    at Parser.readToken (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:180:21)
    at Parser.<anonymous> (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/plugins/jsx/index.js:51:20)
    at Parser.readToken (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/plugins/flow.js:170:22)
    at Parser.nextToken (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:169:21)
    at Parser.next (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:81:12)
    at Parser.eat (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:90:14)
    at Parser.pp.isLineTerminator (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/util.js:69:15)
    at Parser.pp.semicolon (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/util.js:76:13)
    at Parser.pp.parseImport (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:1011:8)
    at Parser.pp.parseStatement (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:141:56)
    at Parser.parseStatement (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/plugins/flow.js:30:22)
    at Parser.pp.parseBlockBody (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:529:21)
    at Parser.pp.parseTopLevel (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:36:8)
john at Johns-MacBook-Pro-4 in ~/mysrc/weisjohn/scratch/test/hello-world                                         
$ echo $?
1

@vjeux
Copy link
Contributor

vjeux commented Jul 28, 2016

Should be reported to npm :)

@weisjohn
Copy link
Contributor Author

@vjeux thanks, sorry

@vjeux
Copy link
Contributor

vjeux commented Jul 28, 2016

Oh I'm sorry, this is very educative, I had no idea that npm run didn't properly forward the return code. This is very unfortunate though and I don't think we can fix it :(

@weisjohn
Copy link
Contributor Author

weisjohn commented Jul 28, 2016

Actually, I'm not sure this is npm's fault.

The default npm run-script build is configured to be "build": "react-scripts build",, which points to react-scripts/bin/react-scripts.js:

$ ./node_modules/react-scripts/bin/react-scripts.js build
Failed to create a production build. Reason:
Module build failed: SyntaxError: /Users/john/mysrc/weisjohn/scratch/test/hello-world/src/App.js: Identifier directly after number (5:2)
  3 | import './App.css';
  4 | 
> 5 | 10x;
    |   ^
  6 | 
  7 | class App extends Component {
  8 |   render() {
    at Parser.pp.raise (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/location.js:22:13)
    at Parser.readNumber (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:674:78)
    at Parser.getTokenFromCode (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:516:23)
    at Parser.readToken (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:180:21)
    at Parser.<anonymous> (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/plugins/jsx/index.js:51:20)
    at Parser.readToken (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/plugins/flow.js:170:22)
    at Parser.nextToken (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:169:21)
    at Parser.next (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:81:12)
    at Parser.eat (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/tokenizer/index.js:90:14)
    at Parser.pp.isLineTerminator (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/util.js:69:15)
    at Parser.pp.semicolon (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/util.js:76:13)
    at Parser.pp.parseImport (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:1011:8)
    at Parser.pp.parseStatement (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:141:56)
    at Parser.parseStatement (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/plugins/flow.js:30:22)
    at Parser.pp.parseBlockBody (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:529:21)
    at Parser.pp.parseTopLevel (/Users/john/mysrc/weisjohn/scratch/test/hello-world/node_modules/react-scripts/node_modules/babylon/lib/parser/statement.js:36:8)
john at Johns-MacBook-Pro-4 in ~/mysrc/weisjohn/scratch/test/hello-world                                         
$ echo $?
0

It looks like we're using the async version of cross-spawn. And so, I think we need spawn sync, a la https://github.com/IndigoUnited/node-cross-spawn#usage and return results to our caller.

If we use cross-spawns sync method, that actually uses spawn-sync, which in turn is just a polyfill for child_process.spawnSync which returns the status code on results.status.

See PR: #254

@weisjohn
Copy link
Contributor Author

Sorry about that, new PR: #256

@vjeux
Copy link
Contributor

vjeux commented Jul 28, 2016

Oh good catch!

@vjeux
Copy link
Contributor

vjeux commented Jul 28, 2016

Thanks for the investigation and the quick fix! Correctly propagating error codes is hard!

@lock lock bot locked and limited conversation to collaborators Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants