Solr
  1. Solr
  2. SOLR-3989

RuntimeException thrown by SolrZkClient should wrap cause, have a message, or be SolrException

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0
    • Fix Version/s: 4.1, 6.0
    • Component/s: clients - java
    • Labels:
      None

      Description

      In a few spots, but notably in the constructor for SolrZkClient, a try-catch block will catch Throwable and throw a new RuntimeException with no cause or message. Either the RuntimeException should wrap the Throwable that was caught, some sort of message should be added, or the type of the exception should be changed to SolrException so calling code can catch these exceptions without casting too broad of a net.

      Reproduce this by creating a CloudSolrServer that points to a URL that is valid, but has no server running:

      CloudSolrServer server = new CloudSolrServer("localhost:9983");

      server.connect();

        Activity

        Hide
        Audun Wilhelmsen added a comment -

        I see that the constructor dont wrap cause, fix to this is to wrap it like this: throw new RuntimeException(e);

        My trunk code is like this:

        public SolrZkClient(String zkServerAddress, int zkClientTimeout, ZkClientConnectionStrategy strat, final OnReconnect onReconnect, int clientConnectTimeout) {
        ...
        try

        { ... }

        catch (Throwable e)

        { ... throw new RuntimeException(); }

        ...
        }

        Show
        Audun Wilhelmsen added a comment - I see that the constructor dont wrap cause, fix to this is to wrap it like this: throw new RuntimeException(e); My trunk code is like this: public SolrZkClient(String zkServerAddress, int zkClientTimeout, ZkClientConnectionStrategy strat, final OnReconnect onReconnect, int clientConnectTimeout) { ... try { ... } catch (Throwable e) { ... throw new RuntimeException(); } ... }
        Hide
        Yonik Seeley added a comment -

        I committed the simplest/safest fix to just wrap the caught throwable in the RuntimeException.

        Show
        Yonik Seeley added a comment - I committed the simplest/safest fix to just wrap the caught throwable in the RuntimeException.
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Yonik Seeley
        http://svn.apache.org/viewvc?view=revision&revision=1408512

        SOLR-3989: SolrZkClient constructor dropped exception cause when throwing a new RuntimeException

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Yonik Seeley http://svn.apache.org/viewvc?view=revision&revision=1408512 SOLR-3989 : SolrZkClient constructor dropped exception cause when throwing a new RuntimeException

          People

          • Assignee:
            Yonik Seeley
            Reporter:
            Colin Bartolome
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development