Details
-
Bug
-
Status: Resolved (View Workflow)
-
Minor
-
Resolution: Fixed
-
None
-
-
workflow-basic-steps 2.19, pipeline-build-step 2.11, workflow-step-api 2.22
Description
We just upgraded pipeline-build-step from 2.9 to 2.10. The following no longer retries:
pipeline {
agent any
stages {
stage('bug') {
steps {
retry(3) {
echo 'in the retry loop'
//throw new Exception('failing in retry loop') // This runs (when uncommented) several times before the pipeline exits with failure.
build(job: 'some/job/that/fails', propagate: true) // This only runs once before the pipeline exits with failure.
}
}
}
}
}
When the sub-build fails, the retry loop exits immediately and does not retry. This didn't used to happen. When raising an error in another way (e.g. the `throw new Exception`, commented out, in the code above), `retry` works as expected.
A workaround is to try/catch the `build()` call and, in catch, `throw new Exception(...)`. If the caught exception is directly re-thrown, we see the same behavior where `retry` does not loop.
Attachments
Issue Links
- is caused by
-
-
- Resolved
-
- relates to
-
-
- Resolved
-
-
-
- Resolved
-
-
-
- Closed
-
- links to
Activity
jglick Noted in this comment that the easiest short-term fix is probably to make retry only rethrow FlowInterruptedException when there is at least one CauseOfInterruption. I did a quick investigation and I think WorkflowRun.doTerm would need to start setting a CauseOfInterruption, but didn't see any other uses that would need to be changed.
He also noted that another possible fix would be to add a flag to FlowInterrupted exception to indicate actual build interruptions as opposed to otherwise-normal exceptions with a result, and have steps like retry look at that flag to decide whether to rethrow the exception. I think that would be easier than introducing new exception types as in my original idea, and a bit more robust than relying on the presence or absence of a CauseOfInterruption, although we would need to update things like the input step and have them set that flag, so it would require a bit more effort.
I am also noting that catchError and warnError have the same problem as retry with the new version of build, and will ignore any errors thrown by it by default.
Basil Crow
added a comment - It might be worth considering the use case in JENKINS-51454 as part of the design of this solution.
Basil Crow
added a comment - It might be worth considering the use case in JENKINS-51454 as part of the design of this solution. basil Good point, I commented on that ticket. I think that the approach from this PR could be tweaked a bit to use https://github.com/jenkinsci/workflow-step-api-plugin/pull/51 (if a setter was added) to change the value of FlowInterruptedException.actualInterruption from true to false when the exception propagates past the timeout step that threw the exception, in which case the fix for this issue would fix that issue as well.
Ok, PRs are up for review:
- https://github.com/jenkinsci/workflow-step-api-plugin/pull/51
- https://github.com/jenkinsci/pipeline-build-step-plugin/pull/39
- https://github.com/jenkinsci/workflow-basic-steps-plugin/pull/102
catchError and warnError are not as affected as retry as I first thought, because they catch all interruptions by default, and only rethrow them if the user passes catchInterruptions: false (I thought the default was true).
A fix for this issue has been released in Pipeline: Basic Steps Plugin 2.19 and Pipeline: Build Step Plugin 2.11. Both plugins must be updated for the fix to take effect. Thanks jonathanb1 for reporting the issue!
This is caused by
JENKINS-49073, and difficult to fix without breakingJENKINS-44379(unless we just revertJENKINS-49073).The cause is that Pipeline Build Step 2.10 now throws FlowInterruptedException when the build fails instead of AbortException so that it can track the result. retry ignores FlowInterruptedException for reasons discussed in
JENKINS-44379(aborting the build via an input step or the sidebar of the build should bypass retry)I'm not really sure how to fix this. I think FlowInterruptedException is overloaded with too many meanings at this point to fix it directly. One approach would be to factor out a ResultCarryingException interface, have FlowInterruptedException and some new FooException implement that interface, and then go through and switch everything that currently just uses FlowInterruptedException for a result to use ResultCarryingException when inspecting exceptions and FooException when creating them, and only use FlowInterruptedException when we only want to capture actual Pipeline interruption.