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

JENKINS-53635 - xunit step failure is not displayed #64

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@Evildethow
Copy link
Contributor

commented Sep 18, 2018

Simply throws the exception to ensure it can be handled upstream.

JENKINS-53635 - xunit step failure is not displayed
Simply throws the exception to ensure it can be handled upstream.
@Evildethow

This comment has been minimized.

Copy link
Contributor Author

commented Sep 18, 2018

@gboissinot @nfalco79 could someone review this?

@nfalco79

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

The exception in a publisher does not cause the build stop. You got only the stacktrace in the console. The stacktrace does not help to understand the cause of failure than the message so by design I choose to only see the failure message instead the whole stracktrace to avoid confusion.

@nfalco79 nfalco79 closed this Sep 20, 2018

@Evildethow

This comment has been minimized.

Copy link
Contributor Author

commented Sep 20, 2018

@nfalco79 The issue with your choice to swallow the exception means that the exception cannot be handled upstream. The user has no ability to control the output, using a try/catch/finally block, as an example.

My point is that your "design choice" seems to go against convention.

This PR is less about choice vs what's correct vs incorrect.

Can we re-open this to discuss further?

@Evildethow

This comment has been minimized.

Copy link
Contributor Author

commented Sep 20, 2018

@nfalco79

This comment has been minimized.

Copy link
Member

commented Sep 21, 2018

For what I can see the link you expose is done for steps that does not handle build status it self.

Have a look to other publish strategy, none of them throws exception.

  • hudson.tasks.Fingerprinter.perform()
  • hudson.tasks.ArtifactArchiver.perform()
  • hudson.tasks.Mailer.perform() (perform an e.printStackTrace to listener -> console)
  • hudson.plugins.sonar.SonarPublisher.perform() -> (exception captured and handled in executeSonar())
  • ...
@nfalco79

This comment has been minimized.

Copy link
Member

commented Sep 21, 2018

Reopen since I see policy change for publisher (I see the PR of artifactarchiver).

@nfalco79 nfalco79 reopened this Sep 21, 2018

@@ -164,6 +164,7 @@ public void perform(final Run<?, ?> build, FilePath workspace, Launcher launcher
// also if we throws AbortException the all publisher steps are always performed. I prefer hide the stacktrace.
listener.error("The plugin hasn't been performed correctly: " + e.getMessage());
build.setResult(Result.FAILURE);
throw e;

This comment has been minimized.

Copy link
@nfalco79

nfalco79 Sep 21, 2018

Member

leave AbortException, instead wrap TransformerException into an AbortException with exception message

@nfalco79 nfalco79 closed this Sep 22, 2018

@nfalco79

This comment has been minimized.

Copy link
Member

commented Sep 22, 2018

merged manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.