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

JenkinsAbstractProjectListener ::onNotifyCommit() totally fails if a runtime exception happens in one item

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      JenkinsAbstractProjectListener::onNotifyCommit() checks all projects with SCM triggers sequentially. Then it calls getSCMs() of the project and processes SCMs. If the method somehow fails (e.g. NPE), the method exists => one corrupted project may cause a total outage of Git SCM triggers

      Example of the issue: JENKINS-26761 (caused by workflow-plugin)

        Attachments

          Issue Links

            Activity

            Hide
            liketic Li Ke added a comment -

            Oleg NenashevThanks! Mark Waite I agree with you. I think it's low priority to fix this issue unless we know what's the best resolution for it. I think what we can do now is add more correct tests for this plugin for changes in the future. Thanks. 

            Show
            liketic Li Ke added a comment - Oleg Nenashev Thanks! Mark Waite I agree with you. I think it's low priority to fix this issue unless we know what's the best resolution for it. I think what we can do now is add more correct tests for this plugin for changes in the future. Thanks. 
            Hide
            markewaite Mark Waite added a comment - - edited

            Li Ke which method are you intending to refactor?

            If it is inside the git plugin (as in GitStatus or GitSCMSource), then your refactoring must include many tests to show current behavior.  Once you have tests that cover current behavior, then the refactoring can be attempted with the benefit of the tests to catch unexpected breaks.  With 120 000+ installations, there are very few branches in the git plugin which are not required for someone.

            If it is inside core Jenkins, others will need to decide if it needs tests first, or already has sufficient tests.  I've consistently found that there are not enough tests in the git plugin (and the git client plugin).

            I'm not especially persuaded that a null pointer exception in one of the triggers should be ignored.  That seems like we've detected undefined behavior, but will silently ignore it and continue operation.

            Show
            markewaite Mark Waite added a comment - - edited Li Ke which method are you intending to refactor? If it is inside the git plugin (as in GitStatus or GitSCMSource ), then your refactoring must include many tests to show current behavior.  Once you have tests that cover current behavior, then the refactoring can be attempted with the benefit of the tests to catch unexpected breaks.  With 120 000+ installations, there are very few branches in the git plugin which are not required for someone. If it is inside core Jenkins, others will need to decide if it needs tests first, or already has sufficient tests.  I've consistently found that there are not enough tests in the git plugin (and the git client plugin). I'm not especially persuaded that a null pointer exception in one of the triggers should be ignored.  That seems like we've detected undefined behavior, but will silently ignore it and continue operation.
            Hide
            oleg_nenashev Oleg Nenashev added a comment -

            Li Ke fine for me, but I would recommend syncing up with Mark Waite before it

            Show
            oleg_nenashev Oleg Nenashev added a comment - Li Ke fine for me, but I would recommend syncing up with Mark Waite before it
            Hide
            liketic Li Ke added a comment -

            Oleg Nenashev Would you mind me to refactor this method? Seems like it's really big and complex..

            Show
            liketic Li Ke added a comment - Oleg Nenashev Would you mind me to refactor this method? Seems like it's really big and complex..

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              oleg_nenashev Oleg Nenashev
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated: