[JENKINS-49707] Implement BodyExecution.cancel(Throwable)

[jglick]

Downstream of jenkinsci/workflow-step-api-plugin#124.

[jglick]

Cannot reproduce test failure locally; looks like a flake (investigating).

[dwnusbaum]

With these changes, any code that currently calls cancel(Throwable) with an exception that is not a FlowInterruptedException will no longer result in a FlowInterruptedException at runtime, so behavior would change. Did you check the PCT? I am not sure how many callers of cancel(Throwable) exist.

[jglick]

I am not sure how many callers of cancel(Throwable) [that do not pass FlowInterruptedException] exist.

I could not find any in common Pipeline plugins. There seem to be a few plugins still improperly overriding stop (not updated like e.g. jenkinsci/workflow-durable-task-step-plugin#49) that I could locate in my local Maven index:

• DatabaseConnectionStep
• ExwsExecution
• GitLabBuildsStep
• GitLabCommitStatusStep

behavior would change

Indeed it would, though probably for the better—the precise reason for the failure of the body would be thrown up naturally.

[dwnusbaum]

Ok, that seems fine. I was just worried that if there was widespread usage then this might cause a bunch of test failures because of things like build results changing from ABORTED to FAILURE.

[jglick]

Checked usage-in-plugins with -i -M on

org.jenkinsci.plugins.workflow.steps.BodyExecution#cancel

resulting in

    "org/jenkinsci/plugins/workflow/steps/BodyExecution#cancel(Ljava/lang/Throwable;)Z": [
      "applitools-eyes",
      "browserstack-integration",
      "database",
      "external-workspace-manager",
      "gitlab-plugin",
      "performance-signature-dynatrace",
      "performance-signature-dynatracesaas",
      "pipeline-gitstatuswrapper",
      "pipeline-npm",
      "sauce-ondemand",
      "testingbot",
      "workflow-cps",
      "workflow-step-api"
    ]

so looks like the previously mentioned hits plus a few more obscure ones. Checking these quickly, they are all StepExecution.stop overrides. This idiom is deprecated but unlikely to lead to a behavioral change anyway, since if a block-scoped step is running another step in its body, stop will be called on the innermost.

[jglick]

looks like a flake (investigating)

Did not manage to track down the cause of https://github.com/jenkinsci/workflow-cps-plugin/pull/570/checks?check_run_id=7468722365. Seems that the same thread which ran WorkflowRun.finish also somehow triggered CpsFlowExecution.logTimings, which is called only from CpsVmExecutorService.tearDown and thus should only ever be called from the CPS VM thread. No hypothesis there.

testFullyDurableSurvivesDirtyRestart https://github.com/jenkinsci/workflow-cps-plugin/pull/570/checks?check_run_id=7470410029 seems like a flake also. I do not understand this code well enough to analyze what the failure means.