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

Base class setChannel does not handle exceptions from onOnline call

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • Jenkins 2.177, LTS 2.176.1

      Throwing an exception in ComputerListener.onOnline should not take a node offline:

      core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164

      However, there a number of issues filed that show exceptions in onOnline not being handled, the latest being one in branch-api:

      jenkinsci/branch-api-plugin PR #142

      Here is the code used in on the Master node and the related test:

      core/src/main/java/jenkins/model/Jenkins.java#L979-L993
      test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487

      Here is the code in SlaveComputer.setChannel() and the test file showing no test:
      core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698
      test/src/test/java/hudson/slaves/SlaveComputerTest.java

      NOTE: the Jenkins.java code is not correct either since it will swallow more than the contract specifies - including NullPointerException for example. Also, it should probably be a WARNING rather than SEVERE.

          [JENKINS-57111] Base class setChannel does not handle exceptions from onOnline call

          Liam Newman created issue -

          Liam Newman added a comment -

          Linked issues are likely all due to this one.

          Liam Newman added a comment - Linked issues are likely all due to this one.
          Liam Newman made changes -
          Link New: This issue causes JENKINS-34625 [ JENKINS-34625 ]
          Liam Newman made changes -
          Link New: This issue causes JENKINS-51863 [ JENKINS-51863 ]
          Liam Newman made changes -
          Link New: This issue causes JENKINS-55597 [ JENKINS-55597 ]
          Liam Newman made changes -
          Link New: This issue causes JENKINS-34044 [ JENKINS-34044 ]
          Liam Newman made changes -
          Description Original: Throwing an exception in ComputerListener.onOnline should not take a node offline:

          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164

          However, there a number of issues filed that show exceptions in onOnline not being handled, the latest being one in branch-api:
          https://github.com/jenkinsci/branch-api-plugin/pull/142

          Here is the code used in on the Master node and the related test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/jenkins/model/Jenkins.java#L979-L993
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487


          Here is the code in hudson.slaves.SlaveComputer.setChannel and the test file showing no test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/hudson/slaves/SlaveComputerTest.java#L45


          New: Throwing an exception in ComputerListener.onOnline should not take a node offline:

          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164

          However, there a number of issues filed that show exceptions in onOnline not being handled, the latest being one in branch-api:
          https://github.com/jenkinsci/branch-api-plugin/pull/142

          Here is the code used in on the Master node and the related test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/jenkins/model/Jenkins.java#L979-L993
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487

          Here is the code in hudson.slaves.SlaveComputer.setChannel and the test file showing no test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/hudson/slaves/SlaveComputerTest.java#L45

          NOTE: the Jenkins.java code is not correct either since it will swallow more than the contract specifies - including NullPointerException for example.
          Liam Newman made changes -
          Description Original: Throwing an exception in ComputerListener.onOnline should not take a node offline:

          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164

          However, there a number of issues filed that show exceptions in onOnline not being handled, the latest being one in branch-api:
          https://github.com/jenkinsci/branch-api-plugin/pull/142

          Here is the code used in on the Master node and the related test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/jenkins/model/Jenkins.java#L979-L993
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487

          Here is the code in hudson.slaves.SlaveComputer.setChannel and the test file showing no test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/hudson/slaves/SlaveComputerTest.java#L45

          NOTE: the Jenkins.java code is not correct either since it will swallow more than the contract specifies - including NullPointerException for example.
          New: Throwing an exception in ComputerListener.onOnline should not take a node offline:

          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164

          However, there a number of issues filed that show exceptions in onOnline not being handled, the latest being one in branch-api:
          https://github.com/jenkinsci/branch-api-plugin/pull/142

          Here is the code used in on the Master node and the related test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/jenkins/model/Jenkins.java#L979-L993
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487

          Here is the code in hudson.slaves.SlaveComputer.setChannel and the test file showing no test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/hudson/slaves/SlaveComputerTest.java#L45

          NOTE: the Jenkins.java code is not correct either since it will swallow more than the contract specifies - including NullPointerException for example. Also, it should probably be a WARNING rather than SEVERE.
          Liam Newman made changes -
          Description Original: Throwing an exception in ComputerListener.onOnline should not take a node offline:

          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164

          However, there a number of issues filed that show exceptions in onOnline not being handled, the latest being one in branch-api:
          https://github.com/jenkinsci/branch-api-plugin/pull/142

          Here is the code used in on the Master node and the related test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/jenkins/model/Jenkins.java#L979-L993
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487

          Here is the code in hudson.slaves.SlaveComputer.setChannel and the test file showing no test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/hudson/slaves/SlaveComputerTest.java#L45

          NOTE: the Jenkins.java code is not correct either since it will swallow more than the contract specifies - including NullPointerException for example. Also, it should probably be a WARNING rather than SEVERE.
          New: Throwing an exception in ComputerListener.onOnline should not take a node offline:

          [https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164|core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164]

          However, there a number of issues filed that show exceptions in onOnline not being handled, the latest being one in branch-api:
          https://github.com/jenkinsci/branch-api-plugin/pull/142

          Here is the code used in on the Master node and the related test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/jenkins/model/Jenkins.java#L979-L993
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487

          Here is the code in hudson.slaves.SlaveComputer.setChannel and the test file showing no test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/hudson/slaves/SlaveComputerTest.java#L45

          NOTE: the Jenkins.java code is not correct either since it will swallow more than the contract specifies - including NullPointerException for example. Also, it should probably be a WARNING rather than SEVERE.
          Liam Newman made changes -
          Description Original: Throwing an exception in ComputerListener.onOnline should not take a node offline:

          [https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164|core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164]

          However, there a number of issues filed that show exceptions in onOnline not being handled, the latest being one in branch-api:
          https://github.com/jenkinsci/branch-api-plugin/pull/142

          Here is the code used in on the Master node and the related test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/jenkins/model/Jenkins.java#L979-L993
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487

          Here is the code in hudson.slaves.SlaveComputer.setChannel and the test file showing no test:
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698
          https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/hudson/slaves/SlaveComputerTest.java#L45

          NOTE: the Jenkins.java code is not correct either since it will swallow more than the contract specifies - including NullPointerException for example. Also, it should probably be a WARNING rather than SEVERE.
          New: Throwing an exception in ComputerListener.onOnline should not take a node offline:

          [core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164|https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/ComputerListener.java#L135-L164]

          However, there a number of issues filed that show exceptions in onOnline not being handled, the latest being one in branch-api:

          [https://github.com/jenkinsci/branch-api-plugin/pull/142]

          Here is the code used in on the Master node and the related test:

          [core/src/main/java/jenkins/model/Jenkins.java#L979-L993|https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/jenkins/model/Jenkins.java#L979-L993]
          [test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487|https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/jenkins/model/JenkinsTest.java#L473-L487]

          Here is the code in SlaveComputer.setChannel() and the test file showing no test:
          [core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698|https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/core/src/main/java/hudson/slaves/SlaveComputer.java#L696-L698]
          [test/src/test/java/hudson/slaves/SlaveComputerTest.java|https://github.com/jenkinsci/jenkins/blob/2767b00146ce2ff2738b7fd7c6db95a26b8f9f39/test/src/test/java/hudson/slaves/SlaveComputerTest.java]

          NOTE: the Jenkins.java code is not correct either since it will swallow more than the contract specifies - including NullPointerException for example. Also, it should probably be a WARNING rather than SEVERE.

            bitwiseman Liam Newman
            bitwiseman Liam Newman
            Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: