Skip to content

Improve cli exit codes #2221

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

Closed
wants to merge 3 commits into from
Closed

Conversation

alessio-perugini
Copy link
Contributor

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

It adds different cmd exit codes.

What is the current behavior?

Right now almost every error exit code is identified with the number 1

What is the new behavior?

The new behavior expands the exit code numbers to distinguish better what kind of error we had. This is especially useful in commands like compile that perform various operations and it's not clear if it is something related to the compilation itself or other components it's using like sketches file problems or upload issues.

Does this PR introduce a breaking change, and is titled accordingly?

Other information

I've experimented a bit at trying to throw very precise exit codes for every error we had, but it becomes way too unmanageable (plus we have limited exit code numbers available in UNIX systems)
The tradeoff is having one exit code that expresses an error happening in a specific component. For example, if the compile commands use sketch, upload, and compile components and one of them fails we can return their associated exit codes.

Open points:

  • Do we like this kind of handling?
  • Should we return these enhanced exit codes only for commands that use different components, and it's not straightforward to understand what caused the problem? I see no value in creating a specific exit code for commands like core install, core upgrade, or board details as their logic uses only their dedicated component.

I'll leave this PR open for feedback, especially from @kittaakos

@alessio-perugini alessio-perugini added the type: enhancement Proposed improvement label Jun 20, 2023
@alessio-perugini alessio-perugini self-assigned this Jun 20, 2023
@alessio-perugini alessio-perugini linked an issue Jun 20, 2023 that may be closed by this pull request
3 tasks
@alessio-perugini alessio-perugini added topic: code Related to content of the project itself topic: CLI Related to the command line interface labels Jun 20, 2023
@alessio-perugini alessio-perugini force-pushed the 2021-improve-exit-codes branch from da20369 to d20e576 Compare June 21, 2023 10:22
@kittaakos
Copy link
Contributor

kittaakos commented Jun 28, 2023

Thank you for working on this. When the compilation fails, I see the expected non-zero exit code ✅ from the CLI, but the console output❓ is strange. Do you know if this is expected?

./arduino-cli version
arduino-cli  Version: git-snapshot Commit: d20e576d Date: 2023-06-28T06:54:50Z
cat ./test_sketches/broken/broken.ino 
trash
./arduino-cli compile -b arduino:avr:uno ./test_sketches/broken
/Users/a.kitta/dev/git/arduino-cli/test_sketches/broken/broken.ino:1:1: error: 'trash' does not name a type; did you mean 'tanh'?
 trash
 ^~~~~
 tanh


Used platform Version Path                                                                
arduino:avr   1.8.6   /Users/a.kitta/Library/Arduino15/packages/arduino/hardware/avr/1.8.6
Error during build: exit status 1
echo $?
9

Error during build: exit status 1 vs. echo $? 9. I assume exit status 1 comes from avr-g++, and CLI maps it to 9`. Altogether it is working as expected, but the console output is a bit misleading.

Thank you!

I used d20e576. (I continue with the review...)

@kittaakos
Copy link
Contributor

kittaakos commented Jun 28, 2023

I get the same exit code 9 when the platform is not installed, the FQBN is invalid, and the sketch has compiler errors:

./arduino-cli version 
arduino-cli  Version: git-snapshot Commit: d20e576d Date: 2023-06-28T06:54:50Z
./arduino-cli compile -b arduino:mbed:nanorp2040connect ./test_sketches/ok
Error during build: Platform 'arduino:mbed' not found: platform not installed
Try running `arduino-cli core install arduino:mbed`
echo $?                                                                   
9
./arduino-cli compile -b arduino:avr:uno ./test_sketches/broken           
/Users/a.kitta/dev/git/arduino-cli/test_sketches/broken/broken.ino:1:1: error: 'trash' does not name a type; did you mean 'tanh'?
 trash
 ^~~~~
 tanh


Used platform Version Path                                                                
arduino:avr   1.8.6   /Users/a.kitta/Library/Arduino15/packages/arduino/hardware/avr/1.8.6
Error during build: exit status 1
echo $?                                                        
9
./arduino-cli compile -b trash:fqbn ./test_sketches/ok
Error during build: Invalid FQBN: not an FQBN: trash:fqbn
echo $?                                               
9

@kittaakos
Copy link
Contributor

I do not know if this is expected, but when I run the compile command with flawed arguments, I get back exit code 1, although there is a dedicated error for illegal arguments:

// ErrBadArgument is returned when the arguments are not valid (7)
ErrBadArgument

./arduino-cli compile ./test_sketches/ok 
Missing FQBN (Fully Qualified Board Name)
echo $?                                  
1

@alessio-perugini
Copy link
Contributor Author

I do not know if this is expected, but when I run the compile command with flawed arguments, I get back exit code 1, although there is a dedicated error for illegal arguments:

Yes, it's expected because some logic that seems easy to distinguish comes from another component that handles the compile action in this case.

We centralized under the commands pkg all the logic used by the gRPC interface and the CLI. For this reason, we cannot distinguish clearly some errors. The only way would be to wrap every error coming from that pkg and make a type switch to understand which error code to give.
I've brought this to the team, and we cannot find a good enough approach to make this feature consistent. Since error codes are part of the API we cannot guarantee a solid way to always put errors in the right "category". We risk implementing a feature that becomes flaky and untrustable.

At the moment, we prefer to not implement this. We can re-evaluate this feature in the future depending on how many requests come from the community.

@alessio-perugini alessio-perugini mentioned this pull request Sep 19, 2023
3 tasks
@alessio-perugini alessio-perugini deleted the 2021-improve-exit-codes branch October 24, 2023 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: CLI Related to the command line interface topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve exit codes
2 participants