Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-24994

Poor error reporting when an anonymous Cause is used

    Details

    • Similar Issues:

      Description

      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

        Attachments

          Issue Links

            Activity

            Hide
            danielbeck Daniel Beck added a comment -

            How can this be reproduced? What cause is set up like this? Doesn't it lead to problems with XML serialization?

            Show
            danielbeck Daniel Beck added a comment - How can this be reproduced? What cause is set up like this? Doesn't it lead to problems with XML serialization?
            Hide
            dvdckl 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.

            Show
            dvdckl 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.
            Hide
            danielbeck Daniel Beck added a comment -

            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?

            Show
            danielbeck Daniel Beck added a comment - 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?
            Hide
            dvdckl 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?

            Show
            dvdckl 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?
            Hide
            jglick Jesse Glick added a comment -

            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.).

            Show
            jglick Jesse Glick added a comment - 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.).

              People

              • Assignee:
                Unassigned
                Reporter:
                dvdckl David Eckel
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated: