SOLR-14345: return correct err msg when non-binary resp parser is used

* This adds support to parse error properly in case of non-binary
  resp parser but the problem still exists for noopResponseParser

solr/CHANGES.txt

@@ -233,6 +233,8 @@ Bug Fixes
 
 * SOLR-14550: Fix duplicates issue in Atomic updates with add-distinct (Thomas Corthals, Munendra S N)
 
+* SOLR-14345: Return proper error message when non-BinaryResponseParser is used in solrJ (Munendra S N)
+
 Other Changes
 ---------------------
 * SOLR-14197: SolrResourceLoader: marked many methods as deprecated, and in some cases rerouted exiting logic to avoid

solr/core/src/test/org/apache/solr/search/json/TestJsonRequest.java

@@ -32,6 +32,8 @@ import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import static org.hamcrest.core.StringContains.containsString;
+
 
 @LuceneTestCase.SuppressCodecs({"Lucene3x","Lucene40","Lucene41","Lucene42","Lucene45","Appending"})
 public class TestJsonRequest extends SolrTestCaseHS {
@@ -84,6 +86,8 @@ public class TestJsonRequest extends SolrTestCaseHS {
   public static void doJsonRequest(Client client, boolean isDistrib) throws Exception {
     addDocs(client);
 
+    ignoreException("Expected JSON");
+
     // test json param
     client.testJQ( params("json","{query:'cat_s:A'}")
         , "response/numFound==2"
@@ -92,6 +96,7 @@ public class TestJsonRequest extends SolrTestCaseHS {
     // invalid value
     SolrException ex = expectThrows(SolrException.class, () -> client.testJQ(params("q", "*:*", "json", "5")));
     assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, ex.code());
+    assertThat(ex.getMessage(), containsString("Expected JSON Object but got Long=5"));
 
     // this is to verify other json params are not affected
     client.testJQ( params("q", "cat_s:A", "json.limit", "1"),
@@ -389,23 +394,19 @@ public class TestJsonRequest extends SolrTestCaseHS {
         , "response/numFound==3", isDistrib? "" :  "response/docs==[{id:'4'},{id:'1'},{id:'5'}]"
     );
 
-    try {
+    // TODO: this seems like a reasonable capability that we would want to support in the future.  It should be OK to make this pass.
-      client.testJQ(params("json", "{query:{'lucene':'foo_s:ignore_exception'}}"));  // TODO: this seems like a reasonable capability that we would want to support in the future.  It should be OK to make this pass.
+    Exception e = expectThrows(Exception.class, () -> {
-      fail();
+      client.testJQ(params("json", "{query:{'lucene':'foo_s:ignore_exception'}}"));
-    } catch (Exception e) {
+    });
-      assertTrue(e.getMessage().contains("foo_s"));
+    assertThat(e.getMessage(), containsString("foo_s"));
-    }
 
-    try {
+    // test failure on unknown parameter
-      // test failure on unknown parameter
+    e = expectThrows(Exception.class, () -> {
-      client.testJQ(params("json", "{query:'cat_s:A', foobar_ignore_exception:5}")
+      client.testJQ(params("json", "{query:'cat_s:A', foobar_ignore_exception:5}"), "response/numFound==2");
-          , "response/numFound==2"
+    });
-      );
+    assertThat(e.getMessage(), containsString("foobar"));
-      fail();
-    } catch (Exception e) {
-      assertTrue(e.getMessage().contains("foobar"));
-    }
 
+    resetExceptionIgnores();
   }
 
   private static void doParamRefDslTest(Client client) throws Exception {

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

@@ -69,6 +69,7 @@ import org.apache.solr.common.util.ExecutorUtil;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.ObjectReleaseTracker;
 import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.common.util.Utils;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.client.HttpClientTransport;
 import org.eclipse.jetty.client.ProtocolHandlers;
@@ -629,6 +630,7 @@ public class Http2SolrClient extends SolrClient {
     return processor == null || processor instanceof InputStreamResponseParser;
   }
 
+  @SuppressWarnings({"unchecked"})
   private NamedList<Object> processErrorsAndResponse(Response response,
                                                      final ResponseParser processor,
                                                      InputStream is,
@@ -701,13 +703,24 @@ public class Http2SolrClient extends SolrClient {
         NamedList<String> metadata = null;
         String reason = null;
         try {
-          NamedList err = (NamedList) rsp.get("error");
+          if (error != null) {
-          if (err != null) {
+            reason = (String) Utils.getObjectByPath(error, false, Collections.singletonList("msg"));
-            reason = (String) err.get("msg");
+            if(reason == null) {
-            if (reason == null) {
+              reason = (String) Utils.getObjectByPath(error, false, Collections.singletonList("trace"));
-              reason = (String) err.get("trace");
+            }
+            Object metadataObj = Utils.getObjectByPath(error, false, Collections.singletonList("metadata"));
+            if  (metadataObj instanceof NamedList) {
+              metadata = (NamedList<String>) metadataObj;
+            } else if (metadataObj instanceof List) {
+              // NamedList parsed as List convert to NamedList again
+              List<Object> list = (List<Object>) metadataObj;
+              metadata = new NamedList<>(list.size()/2);
+              for (int i = 0; i < list.size(); i+=2) {
+                metadata.add((String)list.get(i), (String) list.get(i+1));
+              }
+            } else if (metadataObj instanceof Map) {
+              metadata = new NamedList((Map) metadataObj);
             }
-            metadata = (NamedList<String>) err.get("metadata");
           }
         } catch (Exception ex) {}
         if (reason == null) {

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

@@ -533,6 +533,7 @@ public class HttpSolrClient extends BaseHttpSolrClient {
 
   private static final List<String> errPath = Arrays.asList("metadata", "error-class");//Utils.getObjectByPath(err, false,"metadata/error-class")
 
+  @SuppressWarnings({"unchecked"})
   protected NamedList<Object> executeMethod(HttpRequestBase method, Principal userPrincipal, final ResponseParser processor, final boolean isV2Api) throws SolrServerException {
     method.addHeader("User-Agent", AGENT);
  
@@ -643,13 +644,24 @@ public class HttpSolrClient extends BaseHttpSolrClient {
         NamedList<String> metadata = null;
         String reason = null;
         try {
-          NamedList err = (NamedList) rsp.get("error");
+          if (error != null) {
-          if (err != null) {
+            reason = (String) Utils.getObjectByPath(error, false, Collections.singletonList("msg"));
-            reason = (String) err.get("msg");
             if(reason == null) {
-              reason = (String) err.get("trace");
+              reason = (String) Utils.getObjectByPath(error, false, Collections.singletonList("trace"));
+            }
+            Object metadataObj = Utils.getObjectByPath(error, false, Collections.singletonList("metadata"));
+            if  (metadataObj instanceof NamedList) {
+              metadata = (NamedList<String>) metadataObj;
+            } else if (metadataObj instanceof List) {
+              // NamedList parsed as List convert to NamedList again
+              List<Object> list = (List<Object>) metadataObj;
+              metadata = new NamedList<>(list.size()/2);
+              for (int i = 0; i < list.size(); i+=2) {
+                metadata.add((String)list.get(i), (String) list.get(i+1));
+              }
+            } else if (metadataObj instanceof Map) {
+              metadata = new NamedList((Map) metadataObj);
             }
-            metadata = (NamedList<String>)err.get("metadata");
           }
         } catch (Exception ex) {}
         if (reason == null) {

solr/test-framework/src/java/org/apache/solr/SolrTestCaseHS.java

@@ -45,12 +45,14 @@ import org.apache.solr.client.solrj.embedded.JettySolrRunner;
 import org.apache.solr.client.solrj.impl.NoOpResponseParser;
 import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.response.DelegationTokenResponse;
 import org.apache.solr.client.solrj.response.UpdateResponse;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.CoreDescriptor;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.schema.IndexSchema;
@@ -225,12 +227,15 @@ public class SolrTestCaseHS extends SolrTestCaseJ4 {
       query.setPath(path);
     }
 
-    query.setResponseParser(new NoOpResponseParser(wt));
+    if ("json".equals(wt)) {
-    NamedList<Object> rsp = client.request(query);
+      query.setResponseParser(new DelegationTokenResponse.JsonMapResponseParser());
-
+      NamedList<Object> rsp = client.request(query);
-    String raw = (String)rsp.get("response");
+      return Utils.toJSONString(rsp);
-
+    } else {
-    return raw;
+      query.setResponseParser(new NoOpResponseParser(wt));
+      NamedList<Object> rsp = client.request(query);
+      return  (String)rsp.get("response");
+    }
   }
 
   public static String getQueryResponse(String wt, SolrParams params) throws Exception {