SOLR-5291: Solrj does not propagate the root cause to the user for many errors.

SOLR-3530: Better error messages / Content-Type validation in SolrJ.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1527547 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -304,6 +304,9 @@ Bug Fixes
 * SOLR-5279: Implicit properties don't seem to exist on core RELOAD
   (elyograg, hossman, Steve Rowe) 
 
+* SOLR-5291: Solrj does not propagate the root cause to the user for many errors.
+  (Mark Miller)
+
 Optimizations
 ----------------------
 
@@ -318,6 +321,8 @@ Optimizations
 * SOLR-4816: CloudSolrServer now uses multiple threads to send updates by default.  
   (Joel Bernstein via Mark Miller)
 
+* SOLR-3530: Better error messages / Content-Type validation in SolrJ. (Mark Miller, hossman)
+
 Other Changes
 ----------------------
 

solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java

@@ -49,7 +49,10 @@ import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
+
 import static org.apache.lucene.util.IOUtils.CHARSET_UTF_8;
+
+import org.apache.solr.client.solrj.impl.BinaryResponseParser;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.params.CommonParams;
@@ -1002,7 +1005,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
 
       @Override
       public String getContentType(SolrQueryRequest request, SolrQueryResponse response) {
-        return "application/octet-stream";
+        return BinaryResponseParser.BINARY_CONTENT_TYPE;
       }
 
       @Override

solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java

@@ -19,11 +19,10 @@ package org.apache.solr.response;
 import java.io.*;
 import java.util.*;
 
-import org.apache.lucene.document.Document;
-import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.index.StorableField;
 import org.apache.lucene.index.StoredDocument;
 import org.apache.lucene.util.BytesRef;
+import org.apache.solr.client.solrj.impl.BinaryResponseParser;
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.util.JavaBinCodec;
@@ -59,7 +58,7 @@ public class BinaryResponseWriter implements BinaryQueryResponseWriter {
 
   @Override
   public String getContentType(SolrQueryRequest request, SolrQueryResponse response) {
-    return "application/octet-stream";
+    return BinaryResponseParser.BINARY_CONTENT_TYPE;
   }
 
   @Override

solr/core/src/java/org/apache/solr/response/SchemaXmlResponseWriter.java

@@ -20,6 +20,7 @@ package org.apache.solr.response;
 import java.io.Writer;
 import java.io.IOException;
 
+import org.apache.solr.client.solrj.impl.XMLResponseParser;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.request.SolrQueryRequest;
 
@@ -44,6 +45,6 @@ public class SchemaXmlResponseWriter implements QueryResponseWriter {
 
   @Override
   public String getContentType(SolrQueryRequest request, SolrQueryResponse response) {
-    return CONTENT_TYPE_XML_UTF8;
+    return XMLResponseParser.XML_CONTENT_TYPE;
   }
 }

solr/core/src/java/org/apache/solr/response/XMLResponseWriter.java

@@ -20,6 +20,7 @@ package org.apache.solr.response;
 import java.io.Writer;
 import java.io.IOException;
 
+import org.apache.solr.client.solrj.impl.XMLResponseParser;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.request.SolrQueryRequest;
 
@@ -44,6 +45,6 @@ public class XMLResponseWriter implements QueryResponseWriter {
 
   @Override
   public String getContentType(SolrQueryRequest request, SolrQueryResponse response) {
-    return CONTENT_TYPE_XML_UTF8;
+    return XMLResponseParser.XML_CONTENT_TYPE;
   }
 }

solr/core/src/test/org/apache/solr/schema/TestCloudManagedSchema.java

@@ -120,7 +120,7 @@ public class TestCloudManagedSchema extends AbstractFullDistribZkTestBase {
 
   protected final void assertFileNotInZooKeeper(String fileName) throws Exception {
     // Stolen from AbstractBadConfigTestBase
-    String errString = "returned non ok status:404, message:Not Found";
+    String errString = "Not Found";
     ignoreException(Pattern.quote(errString));
     String rawContent = null;
     try {

solr/solrj/src/java/org/apache/solr/client/solrj/ResponseParser.java

@@ -34,6 +34,15 @@ public abstract class ResponseParser
 
   public abstract NamedList<Object> processResponse(Reader reader);
   
+  /**
+   * A well behaved ResponseParser will return it's content-type.
+   * 
+   * @return the content-type this parser expects to parse
+   */
+  public String getContentType() {
+    return null;
+  }
+  
   /**
    * @return the version param passed to solr
    */

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

@@ -30,6 +30,8 @@ import java.io.Reader;
  * @since solr 1.3
  */
 public class BinaryResponseParser extends ResponseParser {
+  public static final String BINARY_CONTENT_TYPE = "application/octet-stream";
+  
   @Override
   public String getWriterType() {
     return "javabin";
@@ -45,6 +47,10 @@ public class BinaryResponseParser extends ResponseParser {
     }
   }
 
+  @Override
+  public String getContentType() {
+    return BINARY_CONTENT_TYPE;
+  }
 
   @Override
   public String getVersion() {

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

@@ -26,6 +26,7 @@ import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 
+import org.apache.commons.io.IOUtils;
 import org.apache.http.Header;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
@@ -265,7 +266,7 @@ public class HttpSolrServer extends SolrServer {
                 for (ContentStream content : streams) {
                   String contentType = content.getContentType();
                   if(contentType==null) {
-                    contentType = "application/octet-stream"; // default
+                    contentType = BinaryResponseParser.BINARY_CONTENT_TYPE; // default
                   }
                   String name = content.getName();
                   if(name==null) {
@@ -367,6 +368,13 @@ public class HttpSolrServer extends SolrServer {
       
       // Read the contents
       respBody = response.getEntity().getContent();
+      Header ctHeader = response.getLastHeader("content-type");
+      String contentType;
+      if (ctHeader != null) {
+        contentType = ctHeader.getValue();
+      } else {
+        contentType = "";
+      }
       
       // handle some http level checks before trying to parse the response
       switch (httpStatus) {
@@ -382,11 +390,15 @@ public class HttpSolrServer extends SolrServer {
           }
           break;
         default:
-          throw new RemoteSolrException(httpStatus, "Server at " + getBaseURL()
+          if (processor == null) {
-              + " returned non ok status:" + httpStatus + ", message:"
+            throw new RemoteSolrException(httpStatus, "Server at "
-              + response.getStatusLine().getReasonPhrase(), null);
+                + getBaseURL() + " returned non ok status:" + httpStatus
+                + ", message:" + response.getStatusLine().getReasonPhrase(),
+                null);
+          }
       }
       if (processor == null) {
+        
         // no processor specified, return raw stream
         NamedList<Object> rsp = new NamedList<Object>();
         rsp.add("stream", respBody);
@@ -395,6 +407,17 @@ public class HttpSolrServer extends SolrServer {
         return rsp;
       }
       
+      String procCt = processor.getContentType();
+      if (procCt != null) {
+        if (!contentType.equals(procCt)) {
+          // unexpected content type
+          String msg = "Expected content type " + procCt + " but got " + contentType + ".";
+          RemoteSolrException e = new RemoteSolrException(httpStatus, msg + " " +
+              IOUtils.toString(respBody), null);
+          throw e;
+        }
+      }
+      
 //      if(true) {
 //        ByteArrayOutputStream copy = new ByteArrayOutputStream();
 //        IOUtils.copy(respBody, copy);
@@ -403,8 +426,13 @@ public class HttpSolrServer extends SolrServer {
 //        respBody = new ByteArrayInputStream(copy.toByteArray());
 //      }
       
+      NamedList<Object> rsp = null;
       String charset = EntityUtils.getContentCharSet(response.getEntity());
-      NamedList<Object> rsp = processor.processResponse(respBody, charset);
+      try {
+        rsp = processor.processResponse(respBody, charset);
+      } catch (Exception e) {
+        throw new RemoteSolrException(httpStatus, e.getMessage(), e);
+      }
       if (httpStatus != HttpStatus.SC_OK) {
         String reason = null;
         try {

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

@@ -32,6 +32,7 @@ import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLStreamConstants;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamReader;
+
 import java.io.InputStream;
 import java.io.Reader;
 import java.util.ArrayList;
@@ -46,6 +47,7 @@ import java.util.Locale;
  */
 public class XMLResponseParser extends ResponseParser
 {
+  public static final String XML_CONTENT_TYPE = "application/xml; charset=UTF-8";
   public static Logger log = LoggerFactory.getLogger(XMLResponseParser.class);
   private static final XMLErrorLogger xmllog = new XMLErrorLogger(log);
 
@@ -79,6 +81,11 @@ public class XMLResponseParser extends ResponseParser
     return "xml";
   }
   
+  @Override
+  public String getContentType() {
+    return XML_CONTENT_TYPE;
+  }
+
   @Override
   public NamedList<Object> processResponse(Reader in) {
     XMLStreamReader parser = null;