lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From no...@apache.org
Subject lucene-solr:branch_7_0: SOLR-11178: Change error handling in AutoScalingHandler to be consistent w/ other APIs
Date Fri, 04 Aug 2017 07:55:44 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/branch_7_0 1f00dff09 -> 96ca14708


SOLR-11178: Change error handling in AutoScalingHandler to be consistent w/ other APIs


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/96ca1470
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/96ca1470
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/96ca1470

Branch: refs/heads/branch_7_0
Commit: 96ca14708fb07e5733e13a3e2c8666b71c4ad694
Parents: 1f00dff
Author: Noble Paul <noble@apache.org>
Authored: Fri Aug 4 17:25:11 2017 +0930
Committer: Noble Paul <noble@apache.org>
Committed: Fri Aug 4 17:25:11 2017 +0930

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  2 +
 .../cloud/autoscaling/AutoScalingHandler.java   | 41 +++++++++++++++-----
 .../org/apache/solr/servlet/ResponseUtils.java  | 10 ++---
 .../autoscaling/AutoScalingHandlerTest.java     | 25 +++++++++++-
 .../solr/client/solrj/impl/HttpSolrClient.java  | 11 +++---
 .../java/org/apache/solr/common/util/Utils.java |  2 +-
 6 files changed, 68 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/96ca1470/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index e9b305e..2df3941 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -505,6 +505,8 @@ Other Changes
 * SOLR-10033: When attempting to facet with facet.mincount=0 over points fields, raise mincount
to 1
   and log a warning. (Steve Rowe)
 
+* SOLR-11178: Change error handling in AutoScalingHandler to be consistent w/ other APIs
(noble)
+
 ==================  6.7.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/96ca1470/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
b/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
index 356ce37..5ed5f8e 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -167,16 +168,31 @@ public class AutoScalingHandler extends RequestHandlerBase implements
Permission
   private void handleSetClusterPolicy(SolrQueryRequest req, SolrQueryResponse rsp, CommandOperation
op) throws KeeperException, InterruptedException, IOException {
     List clusterPolicy = (List) op.getCommandData();
     if (clusterPolicy == null || !(clusterPolicy instanceof List)) {
-      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "A list of cluster policies
was not found");
+      op.addError("A list of cluster policies was not found");
+      checkErr(op);
+    }
+
+    try {
+      zkSetClusterPolicy(container.getZkController().getZkStateReader(), clusterPolicy);
+    } catch (Exception e) {
+      log.warn("error persisting policies");
+      op.addError(e.getMessage());
+      checkErr(op);
+
     }
-    zkSetClusterPolicy(container.getZkController().getZkStateReader(), clusterPolicy);
     rsp.getValues().add("result", "success");
   }
 
+  private void checkErr(CommandOperation op) {
+    if (!op.hasError()) return;
+    throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST, "Error in
command payload", CommandOperation.captureErrors(Collections.singletonList(op)));
+  }
+
   private void handleSetClusterPreferences(SolrQueryRequest req, SolrQueryResponse rsp, CommandOperation
op) throws KeeperException, InterruptedException, IOException {
     List preferences = (List) op.getCommandData();
     if (preferences == null || !(preferences instanceof List)) {
-      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "A list of cluster preferences
not found");
+      op.addError("A list of cluster preferences not found");
+      checkErr(op);
     }
     zkSetPreferences(container.getZkController().getZkStateReader(), preferences);
     rsp.getValues().add("result", "success");
@@ -185,15 +201,13 @@ public class AutoScalingHandler extends RequestHandlerBase implements
Permission
   private void handleRemovePolicy(SolrQueryRequest req, SolrQueryResponse rsp, CommandOperation
op) throws KeeperException, InterruptedException, IOException {
     String policyName = (String) op.getCommandData();
 
-    if (policyName.trim().length() == 0) {
-      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "The policy name cannot
be empty");
-    }
+    if (op.hasError()) checkErr(op);
     Map<String, Object> autoScalingConf = zkReadAutoScalingConf(container.getZkController().getZkStateReader());
     Map<String, Object> policies = (Map<String, Object>) autoScalingConf.get("policies");
     if (policies == null || !policies.containsKey(policyName)) {
-      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "No policy exists with
name: " + policyName);
+      op.addError("No policy exists with name: " + policyName);
     }
-
+    checkErr(op);
     zkSetPolicies(container.getZkController().getZkStateReader(), policyName, null);
     rsp.getValues().add("result", "success");
   }
@@ -203,11 +217,18 @@ public class AutoScalingHandler extends RequestHandlerBase implements
Permission
     for (Map.Entry<String, Object> policy : policies.entrySet()) {
       String policyName = policy.getKey();
       if (policyName == null || policyName.trim().length() == 0) {
-        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "The policy name cannot
be null or empty");
+        op.addError("The policy name cannot be null or empty");
       }
     }
+    checkErr(op);
 
-    zkSetPolicies(container.getZkController().getZkStateReader(), null, policies);
+    try {
+      zkSetPolicies(container.getZkController().getZkStateReader(), null, policies);
+    } catch (Exception e) {
+      log.warn("error persisting policies", e);
+      op.addError(e.getMessage());
+      checkErr(op);
+    }
 
     rsp.getValues().add("result", "success");
   }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/96ca1470/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java b/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
index 90ca968..228bca8 100644
--- a/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
+++ b/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
@@ -15,15 +15,15 @@
  * limitations under the License.
  */
 package org.apache.solr.servlet;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+
 import org.apache.solr.api.ApiBag;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.NamedList;
-import org.apache.solr.common.util.CommandOperation;
 import org.slf4j.Logger;
 
-import java.io.PrintWriter;
-import java.io.StringWriter;
-
 /**
  * Response helper methods.
  */
@@ -52,7 +52,7 @@ public class ResponseUtils {
       info.add("metadata", errorMetadata);
       if (ex instanceof ApiBag.ExceptionWithErrObject) {
         ApiBag.ExceptionWithErrObject exception = (ApiBag.ExceptionWithErrObject) ex;
-        info.add(CommandOperation.ERR_MSGS, exception.getErrs() );
+        info.add("details", exception.getErrs() );
       }
     }
     

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/96ca1470/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
b/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
index 197801a..9250e8c 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
@@ -81,9 +81,11 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
     try {
       solrClient.request(req);
       fail("Adding a policy with 'cores' attribute should not have succeeded.");
-    } catch (HttpSolrClient.RemoteSolrException e)  {
+    } catch (HttpSolrClient.RemoteExecutionException e)  {
+      String message = String.valueOf(Utils.getObjectByPath(e.getMetaData(), true, "error/details[0]/errorMessages[0]"));
+
       // expected
-      assertTrue(e.getMessage().contains("cores is only allowed in 'cluster-policy'"));
+      assertTrue(message.contains("cores is only allowed in 'cluster-policy'"));
     }
 
     setPolicyCommand =  "{'set-policy': {" +
@@ -175,6 +177,25 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
     assertNotNull(clusterPolicy);
     assertEquals(3, clusterPolicy.size());
   }
+  public void testErrorHandling() throws Exception {
+    CloudSolrClient solrClient = cluster.getSolrClient();
+
+    String setClusterPolicyCommand = "{" +
+        " 'set-cluster-policy': [" +
+        "      {'cores':'<10', 'node':'#ANY'}," +
+        "      {'shard': '#EACH', 'node': '#ANY'}," +
+        "      {'nodeRole':'overseer', 'replica':0}" +
+        "    ]" +
+        "}";
+    try {
+      SolrRequest req = createAutoScalingRequest(SolrRequest.METHOD.POST, setClusterPolicyCommand);
+      solrClient.request(req);
+      fail("expect exception");
+    } catch (HttpSolrClient.RemoteExecutionException e) {
+      String message = String.valueOf(Utils.getObjectByPath(e.getMetaData(), true, "error/details[0]/errorMessages[0]"));
+      assertTrue(message.contains("replica is required in"));
+    }
+  }
 
   @Test
   public void testReadApi() throws Exception  {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/96ca1470/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
index 7566ba0..02d7c1a 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
@@ -23,6 +23,7 @@ import java.lang.invoke.MethodHandles;
 import java.net.ConnectException;
 import java.net.SocketTimeoutException;
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
@@ -497,7 +498,9 @@ public class HttpSolrClient extends SolrClient {
     throw new SolrServerException("Unsupported method: " + request.getMethod());
 
   }
-  
+
+  private static final List<String> errPath = Arrays.asList("metadata", "error-class");//Utils.getObjectByPath(err,
false,"metadata/error-class")
+
   protected NamedList<Object> executeMethod(HttpRequestBase method, final ResponseParser
processor, final boolean isV2Api) throws SolrServerException {
     method.addHeader("User-Agent", AGENT);
  
@@ -596,11 +599,9 @@ public class HttpSolrClient extends SolrClient {
       } catch (Exception e) {
         throw new RemoteSolrException(baseUrl, httpStatus, e.getMessage(), e);
       }
-      if (isV2Api) {
-        Object err = rsp.get("error");
-        if (err != null) {
+      Object error = rsp == null ? null : rsp.get("error");
+      if (error != null && (isV2Api || String.valueOf(getObjectByPath(error, true,
errPath)).endsWith("ExceptionWithErrObject"))) {
           throw RemoteExecutionException.create(baseUrl, rsp);
-        }
       }
       if (httpStatus != HttpStatus.SC_OK && !isV2Api) {
         NamedList<String> metadata = null;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/96ca1470/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 3be565c..08c9dca 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -283,7 +283,7 @@ public class Utils {
 
   public static Object getObjectByPath(Object root, boolean onlyPrimitive, List<String>
hierarchy) {
     if(root == null) return null;
-    if(!isMapLike(root)) throw new RuntimeException("must be a Map or NamedList");
+    if(!isMapLike(root)) return null;
     Object obj = root;
     for (int i = 0; i < hierarchy.size(); i++) {
       int idx = -1;


Mime
View raw message