Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.7.0
    • Component/s: core
    • Labels:
      None

      Description

      ScriptBasedClusterAction is using NPE for flow control.

      public Cluster execute(ClusterSpec clusterSpec, Cluster cluster) throws IOException, InterruptedException {
      ...
      for (String role : instanceTemplate.getRoles()) {
      try

      { handlerMap.get(role).beforeAction(event); }

      catch (NullPointerException e)

      { throw new IllegalArgumentException("No handler for role " + role); }

      }
      ...
      }

      I just lost about an hour trying to find out why the actionhandler was not being registered while the NPE came from the beforeAction() call.

      1. WHIRR-394.patch
        8 kB
        Andrei Savu
      2. WHIRR-394.patch
        6 kB
        Andrei Savu
      3. WHIRR-394.patch
        0.9 kB
        David Alves

        Activity

        Hide
        David Alves added a comment -

        Simple fix that tests if there is a handler first (not catching an NPE)

        Show
        David Alves added a comment - Simple fix that tests if there is a handler first (not catching an NPE)
        Hide
        Andrei Savu added a comment -

        +1 - too bad we've missed this in previous reviews.

        Show
        Andrei Savu added a comment - +1 - too bad we've missed this in previous reviews.
        Hide
        Andrei Savu added a comment -

        It seems like the ComputingMap returned by HandleMapFactory.create does not work with null values. I have updated the code to throw an exception with a relevant error message. All unit tests are passing with this change.

        http://code.google.com/p/google-collections/issues/detail?id=166

        Show
        Andrei Savu added a comment - It seems like the ComputingMap returned by HandleMapFactory.create does not work with null values. I have updated the code to throw an exception with a relevant error message. All unit tests are passing with this change. http://code.google.com/p/google-collections/issues/detail?id=166
        Hide
        David Alves added a comment -

        There is another instance of the bug a bit further down in ScriptBasedCluster action.
        Andrei could you update that too?
        (more of a code quality thing as there is no way an NPE would be thrown there).

        Show
        David Alves added a comment - There is another instance of the bug a bit further down in ScriptBasedCluster action. Andrei could you update that too? (more of a code quality thing as there is no way an NPE would be thrown there).
        Hide
        Andrei Savu added a comment -

        In this update I have extracted a method for handling this error condition. All unit tests are passing.

        Show
        Andrei Savu added a comment - In this update I have extracted a method for handling this error condition. All unit tests are passing.
        Hide
        Andrei Savu added a comment -

        I just did a search and I think this is the only place were we are catching NullPointerException.

        Show
        Andrei Savu added a comment - I just did a search and I think this is the only place were we are catching NullPointerException.
        Hide
        Andrei Savu added a comment -

        Good to go?

        Show
        Andrei Savu added a comment - Good to go?
        Hide
        David Alves added a comment -

        +1 applies cleanly unit test pass.

        Show
        David Alves added a comment - +1 applies cleanly unit test pass.
        Hide
        Andrei Savu added a comment -

        Committed. Thanks David for reviewing.

        Show
        Andrei Savu added a comment - Committed. Thanks David for reviewing.

          People

          • Assignee:
            David Alves
            Reporter:
            David Alves
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development