Scripted pipeline continues executing code after `error` step is encountered

[Alex-Vol-SV]

I use the error step in my pipelines in some cases to ensure prerequisites are met, in testing with this library I noticed that the execution of the pipeline Groovy does not end when error step is executed. The next line of Groovy code will proceed although the error step has done what it is supposed to do.

Trivial example

node {
    stage('Check environment') {
        if (env.SOME_ENV_VALUE_EXPECTED_CONFIGURED == null) {
            error('Please configure correctly global environment value for SOME_ENV_VALUE_EXPECTED_CONFIGURED')
        }
    }
    stage('Do work') {
        def myCount = env.SOME_ENV_VALUE_EXPECTED_CONFIGURED.toInteger()
    }
}

The above oversimplified example will throw NPE on the "Do work" stage code where in Jenkins that stage is never entered.

I was able to achieve the desired effect by changing the code in my unit test to mock error like this.

        helper.registerAllowedMethod("error", [String], { msg ->
            updateBuildStatus('FAILURE')
            throw new Exception(msg)
        })

In testing this fix I found several failing existing tests but in my opinion the fix does work as it should. However, the failing tests are all very trivialized and only expect that the currentBuild.result == 'FAILURE' and have no code that might execute beyond the scope of the error step. Therefore I am inclined to believe this is a valid fix and the trivial test cases are insufficient to make the logic validation.

Similar issues are found with the catchError step which should handle exceptional errors and even error invocations. Currently there is no mock for the catchError step, perhaps solving the handling of error should include implementing the logic to handle catchError as well.

For what it is worth, the error implementation in the workflow plugin is in fact throwing hudson.AbortException as part of the step execution so my mock is simulating the behavior fairly accurately.

[nre-ableton]

I've suggested this before: https://github.com/jenkinsci/JenkinsPipelineUnit/issues/176

The problem is that it's difficult to preserve the existing behavior for libraries that depend on it. In short, doing this would be a breaking change and thus is somewhat controversial.

I'm in favor of this change, FWIW. But for this to happen, there should probably be some consensus about this change. We'd also need to probably bump the version to 2.0 to signify a breaking change.

[Alex-Vol-SV]

Thanks, I did not go back far enough to find your issue. We are in agreement, this is a breaking change and will take several changes to existing mocks and handlers to do it correctly. For now I am using my own mock to solve the problem although as pipelines might be more complex later this could become an issue where error thrown by nested calls might not be expected and handled appropriately. I have put some thought on this but have not come up with a silver bullet.

If there was a single way to handle the execution of the pipelines in unit test the solution might have been to throw and catch a very specific exception so that the caller does not need to be concerned with a new exception thrown. But that internally would succeed in interrupting the code execution and unwind the stacks. Still, captured call stacks would most likely change by this so as much as we would like this to be transparent it will end up being a breaking change for many users therefore a major version change is mandated.

[nre-ableton]

While we are at it, I would also like to remove this legacy code: https://github.com/jenkinsci/JenkinsPipelineUnit/blob/ac0f16d7b14eab5ea2f6de3f9eeb9d36014a1e9c/src/main/groovy/com/lesfurets/jenkins/unit/PipelineTestHelper.groovy#L636-L643

There are probably other instances in the code base of legacy code that could be removed as well. Maybe we should start to compile a list of breaking changes in preparation for a 2.0 release

[bnigh]

I also use the error step in declarative pipelines. When the error step is invoked, we have additional logic occur in the post steps. Should invocation of post steps also be considered for these changes?

Out of curiosity, is it possible to mock the behavior of throwing error / skipping rest of execution... and then catching and resuming logic in post declarative) currently by using helper or any other patterns?

[nre-ableton]

Yes, it is possible to mock error by overriding the default mock provided by JenkinsPipelineUnit. That would look something like this:

helper.registerAllowedMethod('error', [String]) { String message ->
  throw new Exception(message)
}

I'm not terribly familiar with declarative pipelines, but I would imagine that post ought to catch any exceptions and act a bit as a finally block.

[bnigh]

Yes, I'm able to override the error step to throw the exception.

However, it seems as of version 1.8, the exception is not caught by the declarative pipeline when invoked by a test class which extends DeclarativePipelineTest and so the exception is propagated up to the invocation from the test skipping the post declaratives.

As per my understanding, throwing exceptions in declarative pipeline should still invoke post steps... so perhaps this should be a separate github issue

Thanks for the help!