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: log the detailed error message for mvn execution failures #42

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

xieshenzh
Copy link
Contributor

Description

Describe what you did and why.

When mvn command execution fails, add the output to the exception message.
So that, the command execution failure details can be found in the IDE (e.g. IntelliJ IDEA) logs, which helps users to troubleshoot the issues.

Related issue (if any): fixes #issue_number_goes_here

Checklist

  • I have followed this repository's contributing guidelines.
  • I will adhere to the project's code of conduct.

Additional information

Anything else?

Copy link

@bmozaffa bmozaffa left a comment

Choose a reason for hiding this comment

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

Thanks, Xieshen, LGTM!

@zvigrinberg I was testing the IntelliJ plugin and realized a maven issue was causing the report to fail, but the -q prevented any useful information from being surfaced. Please add your thoughts!

Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

Hi @xieshenzh, @bmozaffa , LGTM, will be very helpful to troubleshoots potential problems with underlying maven commands invocations.

the key here is that stdout is going to a file, to be processed by the api, and in case it's empty, and there is an error, stderr is handled by your change and thrown to the caller and the client/user can see everything that happened and act accordingly..

@xieshenzh , please make sure that -

  1. Before merging, Kindly go on my remarks, so the error message, containing the particular error message from maven , will be displayed best on all os/platforms.
  2. I've introduced some bug fixes, so once you're done, kindly rebase your work on top of main branch, before merging.

Thanks for the efforts!

src/main/java/com/redhat/exhort/tools/Operations.java Outdated Show resolved Hide resolved
src/main/java/com/redhat/exhort/tools/Operations.java Outdated Show resolved Hide resolved
src/main/java/com/redhat/exhort/tools/Operations.java Outdated Show resolved Hide resolved
@xieshenzh xieshenzh merged commit 8be9c42 into RHEcosystemAppEng:main Sep 14, 2023
3 of 5 checks passed
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