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

Base class setChannel does not handle exceptions from onOnline call

    XMLWordPrintable

Details

    • Jenkins 2.177, LTS 2.176.1

    Description

      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.

      Attachments

        Issue Links

          Activity

            bitwiseman Liam Newman created issue -
            bitwiseman Liam Newman made changes -
            Field Original Value New Value
            Link This issue causes JENKINS-34625 [ JENKINS-34625 ]
            bitwiseman Liam Newman made changes -
            Link This issue causes JENKINS-51863 [ JENKINS-51863 ]
            bitwiseman Liam Newman made changes -
            Link This issue causes JENKINS-55597 [ JENKINS-55597 ]
            bitwiseman Liam Newman made changes -
            Link This issue causes JENKINS-34044 [ JENKINS-34044 ]
            bitwiseman Liam Newman made changes -
            Description 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


            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.
            bitwiseman Liam Newman made changes -
            Description 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.
            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.
            bitwiseman Liam Newman made changes -
            Description 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.
            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.
            bitwiseman Liam Newman made changes -
            Description 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.
            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 made changes -
            Description 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.
            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:

            [jenkinsci/branch-api-plugin PR #142|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 made changes -
            Assignee Liam Newman [ bitwiseman ]
            bitwiseman Liam Newman made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            bitwiseman Liam Newman made changes -
            Remote Link This issue links to "PR-4015 (Web Link)" [ 22757 ]
            bitwiseman Liam Newman made changes -
            Status In Progress [ 3 ] In Review [ 10005 ]
            bitwiseman Liam Newman made changes -
            Resolution Fixed [ 1 ]
            Status In Review [ 10005 ] Fixed but Unreleased [ 10203 ]
            bitwiseman Liam Newman made changes -
            Status Fixed but Unreleased [ 10203 ] Resolved [ 5 ]
            bitwiseman Liam Newman made changes -
            Labels lts-candidate
            oleg_nenashev Oleg Nenashev made changes -
            Released As Jenkins 2.177
            olivergondza Oliver Gond┼ża made changes -
            Labels lts-candidate 2.176.1-fixed
            bitwiseman Liam Newman made changes -
            Released As Jenkins 2.177 Jenkins 2.177, LTS 2.176.1
            bitwiseman Liam Newman made changes -
            Status Resolved [ 5 ] Closed [ 6 ]

            People

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

              Dates

                Created:
                Updated:
                Resolved: