plugin handler: properly handle snapcraftctl errors
Metadata
Current evaluation
Merged PR #3317 in commit 9c622ecda, making snapcraftctl exit nonzero on build/pull failures and stop override scripts after parent operation errors.
Resolution: closed resolved
Reason: The approved PR was merged to master as commit 9c622ecda, resolving Launchpad bug #1898922 by propagating snapcraftctl failures and preventing override scripts from continuing.
Evaluated by: mr-cal-bot
No scores available.
Issue body
Address issues including:
- snapcraftctl operations always exiting with 0, even for failure
cases.
- snapcraft override scripts continue to run even after
parent snapcraft process fails. This is because `set -e` does
not catch the error when snapcraftctl returns 0, and nothing
is otherwise done to terminate the script.
Given the nature of these symptoms, this is likely the cause of
a number of other seemingly unrelated issues.
These issues occur because _handle_builtin_function() only
catches exceptions of type errors.ScriptletBaseError,
ignoring other cases that may result from the build/pull operations,
such as NodejsPluginMissingPackageJsonError, or
SnapcraftPluginBuildError.
Instead of trying to manage exceptions in _handle_builtin_function(),
catch _any_ unhandled exception in _run_scriplet()'s try block. This
simplifies the more complex return value and feedback handling.
Don't print an error from the feedback channel in snapcraftctl,
the parent process will take care of it. This is necessary
because the error that is now seen by snapcraftctl would lead
to redundant output. Just exit with -1 when an error is
read from the feedback fifo.
LP: #1898922
Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
- [ ] Have you followed the [guidelines for contributing](https://github.com/snapcore/snapcraft/blob/master/CONTRIBUTING.md)?
- [ ] Have you signed the [CLA](http://www.ubuntu.com/legal/contributors/)?
- [ ] Have you successfully run `./runtests.sh static`?
- [ ] Have you successfully run `./runtests.sh tests/unit`?
-----
Evaluation history
| Date | Model | Scores | Action | Summary |
|---|---|---|---|---|
| stealth/space-bunny-alpha | — | closed resolved | Merged PR #3317 in commit 9c622ecda, making snapcraftctl exit nonzero on build/pull failures and stop override scripts after parent operation errors. | |
| stealth/space-bunny-alpha | — | closed resolved | Merged PR #3317 (commit 9c622ecda), fixing snapcraftctl to return nonzero on build/pull failures and stop override scripts after parent errors. | |
| qwen/qwen3.6-35b-a3b | — | — | Merged fix for plugin handler to properly catch snapcraftctl errors. Replaced specific exception handling with a broad catch in the scriptlet runner to prevent silent failures and redundant output. Approved, passed CI, and increased coverage. | |
| qwen3.6-35b-a3b-mtp-q6 | — | — | Merged into master. Fixes snapcraftctl error handling in plugin handlers to properly exit on failures and halt override scripts. Updates _run_scriplet() to catch unhandled exceptions and exit with -1, resolving LP #1898922. |
Update history
No update history recorded yet.
Related issues
| Issue | Project | State | Summary | Similarity |
|---|---|---|---|---|
| #1898922 snapcraftctl fails, but exit status is zero | snapcraft (launchpad) | closed | Fixed by merged PR #3317 (commit 9c622ecda), which makes snapcraft propagate snapcraftctl build failures instead of returning a false success status. | |
| #2251 pluginhandler: stop using alias for snapcraftctl | snapcraft | merged | Merged PR #2251 as commit d941f0573, replacing the snapcraftctl alias with direct installation in a dedicated PATH directory to prepare for subshell-based scriptlets. | |
| #3211 pluginhandler: fix stage-snaps for v2 plugins | snapcraft | merged | Merged in canonical/snapcraft#3211 (0870f1738), fixing v2 stage-snaps handling and preventing prepare_build from discarding unpacked snaps. | |
| #1545 pluginhandler: error out on scriptlet errors | snapcraft | merged | Merged pluginhandler now fails on any scriptlet command error, not only the final command, fixing LP #1707189; PR snapcraft#1545 landed as commit 3a18cc87a. | |
| #2002 many: add snapcraftctl command for scriptlets | snapcraft | merged | Merged PR #2002 (commit bbca282fa) added the snapcraftctl CLI for scriptlets to talk to the parent Snapcraft process, including `snapcraftctl build` and the override-build scriptlet. Resolved #1675 and LP #1751749. | |
| #5983 feat!: error on snapcraftctl | snapcraft | merged | Merged #5983 as eaadefa01: snapcraftctl now errors in core26+ override scripts, and related references were renamed to craftctl. | |
| #2283 snapcraft snap: refactor override-build into a script | snapcraft | merged | PR #2283 merged as commit f9592939a, moving the Snapcraft snap's override-build logic into a script to reduce snapcraft.yaml duplication ahead of self-vendoring. | |
| #2103 package: make use of snapcraftctl snapcraft features | snapcraft | merged | Merged in #2103 as commit 60d505347 and shipped in 2.42, replacing prepare/build/install hooks with override-pull/override-build. | |
| #3131 plugin handler: debug build commands if developer debug enabled | snapcraft | closed | Closed unmerged as superseded by snapcraft#3139, which merged isolated PluginV2 command execution and clearer error reporting in commit 389c180ea. | |
| #2507 build providers: improve handling in snap logic | snapcraft | merged | Merged snapcraft#2507 as commit 841b41fa3, improving build-provider snap logic, removing unnecessary sudo calls, and silencing refresh-hold noise. |