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

fix(scully): make sure scully emits non-zero exit code on errors #115

Merged
merged 1 commit into from
Dec 30, 2019

Conversation

beeman
Copy link
Contributor

@beeman beeman commented Dec 29, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Other... Please describe:

What is the current behavior?

Currently scully always emits a 0 as exit code, even when it fails. Emitting a non-zero exit code is essential if you want to use scully in an automated process, as it will halt the process if something fails - currently it just continues.

What is the new behavior?

Emit exit code 5 (Fatal exception) when something fails.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@SanderElias
Copy link
Contributor

@beeman Thanks for the PR.
This one was already on my radar, thanks for handling it.
However, error number 5 is in my view not suitable. the node documentation says: There was a fatal unrecoverable error in V8 which is clearly not the case.

Can you change to 15? As this is not being used by any node internals, it sends out a clear signal that it is something we do exit on deliberate. We might even refactor to a couple of different codes in te futue

@beeman
Copy link
Contributor Author

beeman commented Dec 30, 2019

@SanderElias just updated the PR to emit 15

@aaronfrost aaronfrost merged commit 8410508 into scullyio:master Dec 30, 2019
@aaronfrost
Copy link
Contributor

@beeman what does a non-zero exit code do? I want to learn. Please teach me.

@beeman
Copy link
Contributor Author

beeman commented Dec 30, 2019

@aaronfrost this is relevant when we want to automate stuff, emitting a non-zero exit code from a child process will instruct the parent process to halt as something failed.

This is useful when you want to automate things in for instance a CI setup. There we want to execute all the steps, but we only want to continue with the next step if the current one successfully exits.

This is an example of npm emitting exit codes 0 or 1 (Generally running echo $? in a shell will show the exit code of the previous command):

➜  ~ npm install -g --silent @angular/cli
/usr/local/bin/ng -> /usr/local/lib/node_modules/@angular/cli/bin/ng
+ @angular/cli@8.3.21
updated 2 packages in 13.692s
➜  ~ echo $?
0
➜  ~ npm install -g --silent @angular/cli-this-does-not-work
➜  ~ echo $?
1

@beeman beeman deleted the beeman/scully-exit-code branch December 30, 2019 20:20
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.

3 participants