Flume
  1. Flume
  2. FLUME-1160

ComponentConfigurationFactory catches NullPointerException

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: v1.2.0
    • Fix Version/s: v1.2.0
    • Component/s: Configuration
    • Labels:
      None

      Description

      Catching NPE and then returning null is super hard to debug when there is an actual bug. We should put the appropriate null checks in place so we don't have to catch NPE.

      1. FLUME-1160.patch
        0.8 kB
        Hari Shreedharan

        Activity

        Hide
        Hudson added a comment -

        Integrated in flume-trunk #189 (See https://builds.apache.org/job/flume-trunk/189/)
        FLUME-1160: ComponentConfigurationFactory catches NullPointerException

        (Hari Shreedharan via Brock Noland) (Revision 1331916)

        Result = SUCCESS
        brock : http://svn.apache.org/viewvc/?view=rev&rev=1331916
        Files :

        • /incubator/flume/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java
        Show
        Hudson added a comment - Integrated in flume-trunk #189 (See https://builds.apache.org/job/flume-trunk/189/ ) FLUME-1160 : ComponentConfigurationFactory catches NullPointerException (Hari Shreedharan via Brock Noland) (Revision 1331916) Result = SUCCESS brock : http://svn.apache.org/viewvc/?view=rev&rev=1331916 Files : /incubator/flume/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java
        Hide
        Brock Noland added a comment -

        Committed in 1331916.

        Thank you for your patch Hari!!

        Show
        Brock Noland added a comment - Committed in 1331916. Thank you for your patch Hari!!
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-04-28 23:27:37, Brock Noland wrote:

        > lgtm, attache the patch to the jira if not already there

        Thanks Brock! Already attached.

        • Hari

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4927/#review7366
        -----------------------------------------------------------

        On 2012-04-28 21:36:08, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4927/

        -----------------------------------------------------------

        (Updated 2012-04-28 21:36:08)

        Review request for Flume and Brock Noland.

        Summary

        -------

        Actually nothing in that try block would throw an NPE, so removing it. To clarify:

        * a check for type being null is done before entering this code.

        * valueOf throws an IllegalArgumentException if the type cannot be found(which is caught and rethrown)

        * All of the getConfiguration functions will return a configuration object(or throw) and not return null.

        This addresses bug FLUME-1160.

        https://issues.apache.org/jira/browse/FLUME-1160

        Diffs

        -----

        flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java c495cde

        Diff: https://reviews.apache.org/r/4927/diff

        Testing

        -------

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-04-28 23:27:37, Brock Noland wrote: > lgtm, attache the patch to the jira if not already there Thanks Brock! Already attached. Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4927/#review7366 ----------------------------------------------------------- On 2012-04-28 21:36:08, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4927/ ----------------------------------------------------------- (Updated 2012-04-28 21:36:08) Review request for Flume and Brock Noland. Summary ------- Actually nothing in that try block would throw an NPE, so removing it. To clarify: * a check for type being null is done before entering this code. * valueOf throws an IllegalArgumentException if the type cannot be found(which is caught and rethrown) * All of the getConfiguration functions will return a configuration object(or throw) and not return null. This addresses bug FLUME-1160 . https://issues.apache.org/jira/browse/FLUME-1160 Diffs ----- flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java c495cde Diff: https://reviews.apache.org/r/4927/diff Testing ------- Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4927/#review7366
        -----------------------------------------------------------

        Ship it!

        lgtm, attache the patch to the jira if not already there

        • Brock

        On 2012-04-28 21:36:08, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4927/

        -----------------------------------------------------------

        (Updated 2012-04-28 21:36:08)

        Review request for Flume and Brock Noland.

        Summary

        -------

        Actually nothing in that try block would throw an NPE, so removing it. To clarify:

        * a check for type being null is done before entering this code.

        * valueOf throws an IllegalArgumentException if the type cannot be found(which is caught and rethrown)

        * All of the getConfiguration functions will return a configuration object(or throw) and not return null.

        This addresses bug FLUME-1160.

        https://issues.apache.org/jira/browse/FLUME-1160

        Diffs

        -----

        flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java c495cde

        Diff: https://reviews.apache.org/r/4927/diff

        Testing

        -------

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4927/#review7366 ----------------------------------------------------------- Ship it! lgtm, attache the patch to the jira if not already there Brock On 2012-04-28 21:36:08, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4927/ ----------------------------------------------------------- (Updated 2012-04-28 21:36:08) Review request for Flume and Brock Noland. Summary ------- Actually nothing in that try block would throw an NPE, so removing it. To clarify: * a check for type being null is done before entering this code. * valueOf throws an IllegalArgumentException if the type cannot be found(which is caught and rethrown) * All of the getConfiguration functions will return a configuration object(or throw) and not return null. This addresses bug FLUME-1160 . https://issues.apache.org/jira/browse/FLUME-1160 Diffs ----- flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java c495cde Diff: https://reviews.apache.org/r/4927/diff Testing ------- Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4927/
        -----------------------------------------------------------

        (Updated 2012-04-28 21:36:08.179731)

        Review request for Flume and Brock Noland.

        Changes
        -------

        Update description.

        Summary (updated)
        -------

        Actually nothing in that try block would throw an NPE, so removing it. To clarify:

        • a check for type being null is done before entering this code.
        • valueOf throws an IllegalArgumentException if the type cannot be found(which is caught and rethrown)
        • All of the getConfiguration functions will return a configuration object(or throw) and not return null.

        This addresses bug FLUME-1160.
        https://issues.apache.org/jira/browse/FLUME-1160

        Diffs


        flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java c495cde

        Diff: https://reviews.apache.org/r/4927/diff

        Testing
        -------

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4927/ ----------------------------------------------------------- (Updated 2012-04-28 21:36:08.179731) Review request for Flume and Brock Noland. Changes ------- Update description. Summary (updated) ------- Actually nothing in that try block would throw an NPE, so removing it. To clarify: a check for type being null is done before entering this code. valueOf throws an IllegalArgumentException if the type cannot be found(which is caught and rethrown) All of the getConfiguration functions will return a configuration object(or throw) and not return null. This addresses bug FLUME-1160 . https://issues.apache.org/jira/browse/FLUME-1160 Diffs flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java c495cde Diff: https://reviews.apache.org/r/4927/diff Testing ------- Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4927/
        -----------------------------------------------------------

        Review request for Flume and Brock Noland.

        Summary
        -------

        Actually nothing in that try block would throw an NPE, so removing it.

        This addresses bug FLUME-1160.
        https://issues.apache.org/jira/browse/FLUME-1160

        Diffs


        flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java c495cde

        Diff: https://reviews.apache.org/r/4927/diff

        Testing
        -------

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4927/ ----------------------------------------------------------- Review request for Flume and Brock Noland. Summary ------- Actually nothing in that try block would throw an NPE, so removing it. This addresses bug FLUME-1160 . https://issues.apache.org/jira/browse/FLUME-1160 Diffs flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java c495cde Diff: https://reviews.apache.org/r/4927/diff Testing ------- Thanks, Hari

          People

          • Assignee:
            Hari Shreedharan
            Reporter:
            Brock Noland
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development