-
-
Notifications
You must be signed in to change notification settings - Fork 422
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
Improve cli exit codes #2221
Conversation
da20369 to
d20e576
Compare
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月28日T06: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...)
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月28日T06: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
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:
arduino-cli/internal/cli/feedback/errorcodes.go
Lines 43 to 44 in d20e576
./arduino-cli compile ./test_sketches/ok
Missing FQBN (Fully Qualified Board Name)
echo $?
1
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.
Please check if the PR fulfills these requirements
See how to contribute
before creating one)
our contributing guidelines
UPGRADING.mdhas been updated with a migration guide (for breaking changes)configuration.schema.jsonupdated 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
compilethat 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:
core install,core upgrade, orboard detailsas their logic uses only their dedicated component.I'll leave this PR open for feedback, especially from @kittaakos