SOLR-10406: SolrJ must throw exception if server throws an error

solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java

@@ -79,9 +79,8 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
     SolrRequest req = createAutoScalingRequest(SolrRequest.METHOD.POST, setPolicyCommand);
     NamedList<Object> response = null;
     try {
-      response = solrClient.request(req);
-      String errorMsg = (String) ((NamedList)response.get("error")).get("msg");
-      assertTrue(errorMsg.contains("cores is only allowed in 'cluster-policy'"));
+      solrClient.request(req);
+      fail("Adding a policy with 'cores' attribute should not have succeeded.");
     } catch (SolrServerException e) {
       // todo one of these catch blocks should not be needed after SOLR-10768
       if (e.getRootCause() instanceof HttpSolrClient.RemoteSolrException) {

solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java

@@ -27,6 +27,7 @@ import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.impl.BinaryResponseParser;
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.impl.XMLResponseParser;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.V2Request;
@@ -67,8 +68,13 @@ public class V2ApiIntegrationTest extends SolrCloudTestCase {
         .withPayload(payload)
         .build();
     v2Request.setResponseParser(responseParser);
-    V2Response response = v2Request.process(cluster.getSolrClient());
-    assertEquals(getStatus(response), expectedCode);
+    try {
+      v2Request.process(cluster.getSolrClient());
+      fail("expected an exception with error code: "+expectedCode);
+    } catch (HttpSolrClient.RemoteExecutionException e) {
+      assertEquals(expectedCode, e.code());
+
+    }
   }
 
   @Test

solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java

@@ -82,6 +82,8 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.MDC;
 
+import static org.apache.solr.common.util.Utils.getObjectByPath;
+
 /**
  * A SolrClient implementation that talks directly to a Solr server via HTTP
  */
@@ -575,6 +577,12 @@ 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) {
+          throw RemoteExecutionException.create(baseUrl, rsp);
+        }
+      }
       if (httpStatus != HttpStatus.SC_OK && !isV2Api) {
         NamedList<String> metadata = null;
         String reason = null;
@@ -758,6 +766,39 @@ public class HttpSolrClient extends SolrClient {
     }
   }
 
+  /**
+   * This should be thrown when a server has an error in executing the request and
+   * it sends a proper payload back to the client
+   */
+  public static class RemoteExecutionException extends RemoteSolrException {
+    private NamedList meta;
+
+    public RemoteExecutionException(String remoteHost, int code, String msg, NamedList meta) {
+      super(remoteHost, code, msg, null);
+      this.meta = meta;
+    }
+
+
+    public static RemoteExecutionException create(String host, NamedList errResponse) {
+      Object errObj = errResponse.get("error");
+      if (errObj != null) {
+        Number code = (Number) getObjectByPath(errObj, true, Collections.singletonList("code"));
+        String msg = (String) getObjectByPath(errObj, true, Collections.singletonList("msg"));
+        return new RemoteExecutionException(host, code == null ? ErrorCode.UNKNOWN.code : code.intValue(),
+            msg == null ? "Unknown Error" : msg, errResponse);
+
+      } else {
+        throw new RuntimeException("No error");
+      }
+
+    }
+
+    public NamedList getMetaData() {
+
+      return meta;
+    }
+  }
+
   /**
    * Constructs {@link HttpSolrClient} instances from provided configuration.
    */

solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java

@@ -43,6 +43,7 @@ import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteExecutionException;
 import org.apache.solr.client.solrj.request.IsUpdateRequest;
 import org.apache.solr.client.solrj.request.RequestWriter;
 import org.apache.solr.client.solrj.response.QueryResponse;
@@ -460,7 +461,9 @@ public class LBHttpSolrClient extends SolrClient {
       if (isZombie) {
         zombieServers.remove(zombieKey);
       }
-    } catch (SolrException e) {
+    } catch (RemoteExecutionException e){
+      throw e;
+    } catch(SolrException e) {
       // we retry on 404 or 403 or 503 or 500
       // unless it's an update - then we only retry on connect exception
       if (!isNonRetryable && RETRY_CODES.contains(e.code())) {

solr/solrj/src/java/org/apache/solr/common/util/Utils.java

@@ -213,15 +213,20 @@ public class Utils {
     }
   }
 
-  public static Object getObjectByPath(Map root, boolean onlyPrimitive, String hierarchy) {
+  public static Object getObjectByPath(Object root, boolean onlyPrimitive, String hierarchy) {
+    if(root == null) return null;
+    if(!isMapLike(root)) throw new RuntimeException("must be a Map or NamedList");
     List<String> parts = StrUtils.splitSmart(hierarchy, '/');
     if (parts.get(0).isEmpty()) parts.remove(0);
     return getObjectByPath(root, onlyPrimitive, parts);
   }
 
-  public static Object getObjectByPath(Map root, boolean onlyPrimitive, List<String> hierarchy) {
+
+
+  public static Object getObjectByPath(Object root, boolean onlyPrimitive, List<String> hierarchy) {
     if(root == null) return null;
-    Map obj = root;
+    if(!isMapLike(root)) throw new RuntimeException("must be a Map or NamedList");
+    Object obj = root;
     for (int i = 0; i < hierarchy.size(); i++) {
       int idx = -1;
       String s = hierarchy.get(i);
@@ -233,22 +238,22 @@ public class Utils {
         }
       }
       if (i < hierarchy.size() - 1) {
-        Object o = obj.get(s);
+        Object o = getVal(obj, s);
         if (o == null) return null;
         if (idx > -1) {
           List l = (List) o;
           o = idx < l.size() ? l.get(idx) : null;
         }
-        if (!(o instanceof Map)) return null;
-        obj = (Map) o;
+        if (!isMapLike(o)) return null;
+        obj = o;
       } else {
-        Object val = obj.get(s);
+        Object val = getVal(obj, s);
         if (val == null) return null;
         if (idx > -1) {
           List l = (List) val;
           val = idx < l.size() ? l.get(idx) : null;
         }
-        if (onlyPrimitive && val instanceof Map) {
+        if (onlyPrimitive && isMapLike(val)) {
           return null;
         }
         return val;
@@ -257,7 +262,17 @@ public class Utils {
 
     return false;
   }
-  
+
+  private static boolean isMapLike(Object o) {
+    return o instanceof Map || o instanceof NamedList;
+  }
+
+  private static Object getVal(Object obj, String key) {
+    if(obj instanceof NamedList) return ((NamedList) obj).get(key);
+    else if (obj instanceof Map) return ((Map) obj).get(key);
+    else throw new RuntimeException("must be a NamedList or Map");
+  }
+
   /**
    * If the passed entity has content, make sure it is fully
    * read and closed.