-
Bug
-
Resolution: Unresolved
-
Minor
-
envinject-plugin master branch
EnvInject throws a StringIndexOutOfBoundsException in BuildCauseRetriever.getTriggeredCause if the class that triggered it was anonymous. This is because getTriggerName uses cause.getClass().getSimpleName(), which returns an empty string for anonymous classes, empty strings are skipped when getTriggeredCause iterates over build causes, but it always assumes that there is at least one non-empty build cause.
To reproduce, create a trigger that uses an anonymous Cause like this:
private static void scheduleBuild(BuildableItem job) { job.scheduleBuild(new Cause() { @Override public String getShortDescription() { return "Triggered by XYZ"; } }); }
To work around this, the above snippet can be rewritten to avoid the anonymous class:
private static void scheduleBuild(BuildableItem job) { job.scheduleBuild(new XyzCause()); } private static class XyzCause extends Cause { @Override public String getShortDescription() { return "Triggered by XYZ"; } }
I'm not sure which fix would be most consistent with the intent of the ENV_CAUSE variable for custom triggers. I see these options:
- Skip setting an ENV_CAUSE, possibly log a warning
- Use the parent class as the trigger name
- make the trigger "ANONYMOUSTRIGGER", possibly prefixed by the parent class
- is related to
-
-
- Resolved
-
[JENKINS-24994] Poor error reporting when an anonymous Cause is used
David Eckel
added a comment - Daniel, I'm not sure what you mean about XML serialization. That exception does cause my builds to fail if that's what you mean.
I've added an example snippet to reproduce the error in the description.
David Eckel
added a comment - Daniel, I'm not sure what you mean about XML serialization. That exception does cause my builds to fail if that's what you mean.
I've added an example snippet to reproduce the error in the description. Builds are stored as XML on disk in files called build.xml. If you create causes like that, they serialize a lot of garbage to disk, and are brittle in the face of code changes. I replaced the cause of ParameterizedJobMixin.scheduleBuild() (the overload without arguments) by something similar to your sample code, and this is what it looks like on disk:
<causes>
<jenkins.model.ParameterizedJobMixIn_-1>
<outer-class class="hudson.model.AbstractProject$2">
<outer-class class="hudson.model.FreeStyleProject">
<actions/>
<description></description>
<keepDependencies>false</keepDependencies>
<properties/>
<scm class="hudson.scm.NullSCM"/>
<canRoam>true</canRoam>
<disabled>false</disabled>
<blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
<blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
<triggers/>
<concurrentBuild>false</concurrentBuild>
<builders/>
<publishers/>
<buildWrappers/>
</outer-class>
</outer-class>
</jenkins.model.ParameterizedJobMixIn_-1>
</causes>
Note that the demo job I used is empty, and this would contain the entire XML configuration at the time the cause was serialized, because it's defined inside ParameterizedJobMixIn and inherits the outer classes' environment. It deserializes a separate instance of the outer class as well that is different from the regularly loaded instance.
I think it's pointless to fix this issue, as any implementations of anonymous inner classes suffer from similar problems, making them into bugs waiting to happen that should be avoided. You should probably check to see which of these issues affect the plugin you're using and report issues against it.
Resolve as Won't Fix?
David Eckel
added a comment - Understood. Another option to resolve is to add a better exception. Out of the 70+ plugins we're using, the only one to error out was envinject. A one-liner error test and exception about an unsupported trigger might save someone some debugging time in the future.
If you feel that Jenkins shouldn't allow that kind of trigger at all, do you think that should be filed as a enhancement against the appropriate core functionality?
David Eckel
added a comment - Understood. Another option to resolve is to add a better exception. Out of the 70+ plugins we're using, the only one to error out was envinject. A one-liner error test and exception about an unsupported trigger might save someone some debugging time in the future.
If you feel that Jenkins shouldn't allow that kind of trigger at all, do you think that should be filed as a enhancement against the appropriate core functionality? It might make sense for Jenkins to define an annotation indicating abstract types whose implementations are intended to be serialized with XStream (Cause, RunAction2, etc.). An annotation processor could then report an error if an implementation were an anonymous inner class or a non-static nested class, or had fields of various types known to be nonserializable or known to themselves be serialized at top level (User, Run, etc.).
How can this be reproduced? What cause is set up like this? Doesn't it lead to problems with XML serialization?