-
Notifications
You must be signed in to change notification settings - Fork 99
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
[FIXED JENKINS-32144] Specify that a task was cancelled when CancellationException happens #68
base: master
Are you sure you want to change the base?
Conversation
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
LGTM. Would be useful to see a unit test BTW |
@oleg-nenashev The root-cause of the issue might be fixed by JENKINS-25092 - there is already a pull request done. Notice that the only purpose of this pull request is to provide more information to users so they can better understand what is going on. |
Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests. |
🐝 according to the clarification. This change is positive in any case |
} catch(CancellationException e) { | ||
// TODO perhaps rethrow? | ||
current_state.result = FAILURE | ||
listener.error("Failed to run DSL Script: At least one of the tasks of the buildflow was cancelled") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about e.getMessage() in the log. Just for the sake of better logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oleg-nenashev Is not e.printStackTrace(listener.getLogger())
enough?
@fbelzunc adding a testcase will make sure there is no regression in the future. Could you please add one? thank you. |
@dnozay There is already a test case for this called Notice that this pull request doesn't change the functionality at all. |
yes, you could check the log output; e.g. https://github.com/jenkinsci/build-flow-plugin/blob/master/src/test/groovy/com/cloudbees/plugins/flow/BuildTest.groovy#L186-L194 |
@fbelzunc Side note: I think introducing new features in this plugin is risky and takes away resources that could be better spent on the pipeline plugin. However, please feel free to merge if you are interested in carrying on the flag. |
If a task which is in the queue is cancelled then the following stacktrace happens.
It might be obvious for some Jenkins users that it has happened because someone or something cancelled the task on the queue, however according with my experience providing Jenkins support it is not for all the users.
https://issues.jenkins-ci.org/browse/JENKINS-32144
@reviewbybees