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(errors): reformat plugin build error output #1012

Merged

Conversation

cmatsuoka
Copy link
Collaborator

@cmatsuoka cmatsuoka commented Feb 7, 2025

In case of Python plugin build errors, print error messages to stderr
instead of stdout so they can be captured and properly displayed to the
user. Don't start printing from the last script execution trace line because
messages printed to stderr appear before the traced exit line.

Fixes #1000

Signed-off-by: Claudio Matsuoka [email protected]

  • Have you signed the CLA?
  • Have you added an entry to the changelog (docs/reference/changelog.rst)?

@cmatsuoka cmatsuoka changed the title fix(errors): reformat user error output fix(errors): reformat plugin build error output Feb 7, 2025
@cmatsuoka cmatsuoka force-pushed the CRAFT-4070-Error-presentation-sometimes-omits-the-last-lines branch 3 times, most recently from 10f4740 to a4ab131 Compare February 7, 2025 20:36
In case of a plugin build error, pass the combined output to the
error message and display the last lines as the error detail. It was
previously displaying only the stderr output, starting at the last
line with '+' as the first character.

Signed-off-by: Claudio Matsuoka <[email protected]>
@cmatsuoka cmatsuoka force-pushed the CRAFT-4070-Error-presentation-sometimes-omits-the-last-lines branch from a4ab131 to 8cc1627 Compare February 10, 2025 12:41
@cmatsuoka
Copy link
Collaborator Author

This is a very interesting error because it seems that, despite being reported as happening randomly, it should have never worked in the first place: only stderr was captured and the string being tested by Rockcraft was sent to stdout. Now we're capturing the combined output, and accounting for the race of stdout text appearing before or after the +exit 1 trace message sent to stderr.

@cmatsuoka cmatsuoka force-pushed the CRAFT-4070-Error-presentation-sometimes-omits-the-last-lines branch 2 times, most recently from 9b75036 to 8cc1627 Compare February 10, 2025 19:51
@cmatsuoka cmatsuoka force-pushed the CRAFT-4070-Error-presentation-sometimes-omits-the-last-lines branch from 11150cf to 236229c Compare February 11, 2025 15:04
@cmatsuoka cmatsuoka marked this pull request as ready for review February 11, 2025 16:25
@cmatsuoka
Copy link
Collaborator Author

It seems that related Rockcraft issues are gone after preventing the Python interpreter search to return an error and stop the build process. See canonical/rockcraft#809

@cmatsuoka
Copy link
Collaborator Author

Note that capturing only stderr can sometimes show us error messages with the echo command trace and the corresponding output. This is still better than displaying output starting at the last trace line -- in this case it would show only the exit 1 line.

Failed to run the build script for part 'my-part'.
Detailed information: 
:: + echo 'No suitable Python interpreter found, giving up.'
:: No suitable Python interpreter found, giving up.
:: + exit 1
Recommended resolution: Check the build output and verify the project can work with the 'python' plugin.

Properly redirect error messages instead of using the combined stream
output.

Signed-off-by: Claudio Matsuoka <[email protected]>
@cmatsuoka cmatsuoka force-pushed the CRAFT-4070-Error-presentation-sometimes-omits-the-last-lines branch 2 times, most recently from 592d34e to 96710f0 Compare February 11, 2025 18:10
Signed-off-by: Claudio Matsuoka <[email protected]>
@cmatsuoka cmatsuoka force-pushed the CRAFT-4070-Error-presentation-sometimes-omits-the-last-lines branch from 96710f0 to c045cc5 Compare February 11, 2025 18:44
@cmatsuoka
Copy link
Collaborator Author

cmatsuoka commented Feb 11, 2025

Also note that capturing the last three lines will capture only the last error message, along with its echo command and exit 1. We can increase the amount of captured lines, at the risk of increasing noise if e.g. a compiler command line is captured.

@cmatsuoka cmatsuoka requested review from tigarmo and bepri February 11, 2025 19:34
@cmatsuoka
Copy link
Collaborator Author

cmatsuoka commented Feb 11, 2025

As a reference, this is how the error message usually appears if using combined output:

Failed to run the build script for part 'my-part'.
Detailed information: 
:: + exit 1
:: Python interpreter not found in payload.
:: No suitable Python interpreter found, giving up.
Recommended resolution: Check the build output and verify the project can work with the 'python' plugin.

Signed-off-by: Claudio Matsuoka <[email protected]>
craft_parts/errors.py Outdated Show resolved Hide resolved
craft_parts/plugins/base.py Outdated Show resolved Hide resolved
@cmatsuoka
Copy link
Collaborator Author

Not a huge difference in this case, but here's the output including the last three traced lines:

Failed to run the build script for part 'my-part'.
Detailed information: 
:: + '[' -z '' ']'
:: + echo 'No suitable Python interpreter found, giving up.'
:: No suitable Python interpreter found, giving up.
:: + exit 1
Recommended resolution: Check the build output and verify the project can work with the 'python' plugin.

Signed-off-by: Claudio Matsuoka <[email protected]>
@cmatsuoka cmatsuoka force-pushed the CRAFT-4070-Error-presentation-sometimes-omits-the-last-lines branch from 12d2a89 to bd20e82 Compare February 12, 2025 18:11
@cmatsuoka cmatsuoka requested a review from bepri February 12, 2025 19:49
Copy link
Contributor

@bepri bepri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last 3 commands looks good, I'm happy with this! Thanks.

Copy link
Contributor

@tigarmo tigarmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

craft_parts/errors.py Show resolved Hide resolved
craft_parts/errors.py Outdated Show resolved Hide resolved
tests/integration/plugins/test_poetry.py Outdated Show resolved Hide resolved
tests/integration/plugins/test_python.py Outdated Show resolved Hide resolved
@cmatsuoka cmatsuoka force-pushed the CRAFT-4070-Error-presentation-sometimes-omits-the-last-lines branch 2 times, most recently from 8382ef0 to 3721fa5 Compare February 13, 2025 18:51
@cmatsuoka cmatsuoka merged commit f47837a into main Feb 14, 2025
13 checks passed
@cmatsuoka cmatsuoka deleted the CRAFT-4070-Error-presentation-sometimes-omits-the-last-lines branch February 14, 2025 11:43
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.

Error presentation sometimes omits the last lines
3 participants