JENKINS-60210: Whitelisting all java.util.Collections methods

[haridsv]

We currently only have Collections.sort whiltelisted, but every method in this class is a static utility method that is safe, so this PR whitelists all of them. This also whitelists a couple of Throwable.printStackTrace variations. PR #249 already whitelisted a few java.io methods to make it possible to write to a StringWriter, so together this will make it possible to log more detailed error messages that include exception stacktraces and make it easier to troubleshoot some issues.

[dwnusbaum]

Like you pointed out, assuming you are using a StringWriter or something like that, this should be safe, but what's the use case? Why not just throw the error up and let the Pipeline fail so you can see the stack trace that way? I guess you are catching errors that you want to suppress, but you still want to see the stack trace?

There is catchError(buildResult: 'SUCCESS', stageResult: 'SUCCESS') { throw new Exception() } if you just want to see the error and stack trace and ignore the failure, but that doesn't allow you to interact with the stack trace programmatically.

[haridsv]

I didn't know about catchError (thanks for pointing it out), but I just tested it and I can see that it does log the exception stacktrace so would have been acceptable in most cases except that it doesn't work correctly. E.g., I need to set the result to UNSTABLE instead of FAILED and also need to log a troubleshooting message, but for some reason, none of these parameters are getting recognized. E.g., I tried the below:

def F() {
    catchError(buildResult: 'UNSTABLE', stageResult: 'UNSTABLE', message: 'Got unexpected exception') {
        throw new Exception("Testing catchError")
    }
}

stage('Test') {
    F()
}

Both the stage and build result are still set to FAILED and the message is nowhere to be found. Could this be a bug?

However, it is still very useful to have these signatures whitelisted because, using using try/catch/finally could make the code look more natural in a scripted approach instead of using catchError and in fact is already pointed out in the documentation here: https://jenkins.io/doc/pipeline/steps/workflow-basic-steps/#catcherror-catch-error-and-set-build-result-to-failure.

[haridsv]

@dwnusbaum I am just wondering if you are waiting for any more inputs from me on this review?

[dwnusbaum]

Both the stage and build result are still set to FAILED and the message is nowhere to be found.

Weird, what version of Pipeline: Basic Steps Plugin and Pipeline: Groovy Plugin do you have installed? (If you are running an old version of Pipeline: Basic Steps Plugin, those options might not be supported, and if you are running an old version of Pipeline: Groovy Plugin, you won't get a warning that the options are unsupported)

[dwnusbaum]

But yeah, I see how these signatures could be useful in scripted, so I'm not totally opposed to it, I just wanted to check if catchError handles your use case.

[haridsv]

Thanks for approving! Here is the version information you asked:

Pipeline: Basic Steps: 2.15 (latest is 2.18)
Pipeline: Groovy: 2.70 (latest is 2.78)

[dwnusbaum]

Ok, thanks for the update, you need Pipeline: Basic Steps 2.16+ for the new options to catchError and Pipeline: Groovy 2.73+ for the fix that logs a warning when you pass unsupported options to a step.