SOLR-4487: HttpSolrServer now includes the true http status code in SolrExceptions that it throws

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

solr/CHANGES.txt

@@ -203,6 +203,11 @@ Bug Fixes
 * SOLR-4710: You cannot delete a collection fully from ZooKeeper unless all nodes are up and 
   functioning correctly. (Mark Miller)
 
+* SOLR-4487: SolrExceptions thrown by HttpSolrServer will now contain the 
+  proper HTTP status code returned by the remote server, even if that status 
+  code is not something Solr itself returned -- eg: from the Servlet Container, 
+  or an intermediate HTTP Proxy (hossman)
+
 Optimizations
 ----------------------
 

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

@@ -369,10 +369,9 @@ public class HttpSolrServer extends SolrServer {
           }
           break;
         default:
-          throw new SolrException(SolrException.ErrorCode.getErrorCode(httpStatus), "Server at " + getBaseURL()
+          throw new RemoteSolrException(httpStatus, "Server at " + getBaseURL()
               + " returned non ok status:" + httpStatus + ", message:"
-              + response.getStatusLine().getReasonPhrase());
-          
+              + response.getStatusLine().getReasonPhrase(), null);
       }
       if (processor == null) {
         // no processor specified, return raw stream
@@ -400,8 +399,7 @@ public class HttpSolrServer extends SolrServer {
           msg.append("request: " + method.getURI());
           reason = java.net.URLDecoder.decode(msg.toString(), UTF_8);
         }
-        throw new SolrException(
-            SolrException.ErrorCode.getErrorCode(httpStatus), reason);
+        throw new RemoteSolrException(httpStatus, reason, null);
       }
       return rsp;
     } catch (ConnectException e) {
@@ -632,4 +630,20 @@ public class HttpSolrServer extends SolrServer {
   public void setUseMultiPartPost(boolean useMultiPartPost) {
     this.useMultiPartPost = useMultiPartPost;
   }
-}
+
+  /**
+   * Subclass of SolrException that allows us to capture an arbitrary HTTP
+   * status code that may have been returned by the remote server or a 
+   * proxy along the way.
+   */
+  protected static class RemoteSolrException extends SolrException {
+    /**
+     * @param code Arbitrary HTTP status code
+     * @param msg Exception Message
+     * @param th Throwable to wrap with this Exception
+     */
+    public RemoteSolrException(int code, String msg, Throwable th) {
+      super(code, msg, th);
+    }
+  }
+}

solr/solrj/src/java/org/apache/solr/common/SolrException.java

@@ -31,6 +31,9 @@ import org.slf4j.Logger;
 public class SolrException extends RuntimeException {
 
   /**
+   * This list of valid HTTP Status error codes that Solr may return in 
+   * the case of a "Server Side" error.
+   *
    * @since solr 1.2
    */
   public enum ErrorCode {
@@ -69,8 +72,28 @@ public class SolrException extends RuntimeException {
     super(th);
     this.code = code.code;
   }
+
+  /**
+   * Constructor that can set arbitrary http status code. Not for 
+   * use in Solr, but may be used by clients in subclasses to capture 
+   * errors returned by the servlet container or other HTTP proxies.
+   */
+  protected SolrException(int code, String msg, Throwable th) {
+    super(msg, th);
+    this.code = code;
+  }
   
   int code=0;
+
+  /**
+   * The HTTP Status code associated with this Exception.  For SolrExceptions 
+   * thrown by Solr "Server Side", this should valid {@link ErrorCode}, 
+   * however client side exceptions may contain an arbitrary error code based 
+   * on the behavior of the Servlet Container hosting Solr, or any HTTP 
+   * Proxies that may exist between the client and the server.
+   *
+   * @return The HTTP Status code associated with this Exception
+   */
   public int code() { return code; }
 
 

solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrServerTest.java

@@ -43,6 +43,8 @@ import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.client.solrj.request.UpdateRequest;
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.util.ExternalPaths;
@@ -74,12 +76,19 @@ public class BasicHttpSolrServerTest extends SolrJettyTestBase {
       lastMethod = null;
       headers = null;
       parameters = null;
+      errorCode = null;
     }
     
+    public static Integer errorCode = null;
     public static String lastMethod = null;
     public static HashMap<String,String> headers = null;
     public static Map<String,String[]> parameters = null;
     
+    public static void setErrorCode(Integer code) {
+      errorCode = code;
+    }
+    
+
     @Override
     protected void doGet(HttpServletRequest req, HttpServletResponse resp)
         throws ServletException, IOException {
@@ -110,6 +119,13 @@ public class BasicHttpSolrServerTest extends SolrJettyTestBase {
     private void recordRequest(HttpServletRequest req, HttpServletResponse resp) {
       setHeaders(req);
       setParameters(req);
+      if (null != errorCode) {
+        try { 
+          resp.sendError(errorCode); 
+        } catch (IOException e) {
+          throw new RuntimeException("sendError IO fail in DebugServlet", e);
+        }
+      }
     }
   }
   
@@ -160,6 +176,34 @@ public class BasicHttpSolrServerTest extends SolrJettyTestBase {
     server.shutdown();
   }
   
+  /**
+   * test that SolrExceptions thrown by HttpSolrServer can
+   * correctly encapsulate http status codes even when not on the list of
+   * ErrorCodes solr may return.
+   */
+  public void testSolrExceptionCodeNotFromSolr() throws IOException, SolrServerException {
+    final int status = 527;
+    assertEquals(status + " didn't generate an UNKNOWN error code, someone modified the list of valid ErrorCode's w/o changing this test to work a different way",
+                 ErrorCode.UNKNOWN, ErrorCode.getErrorCode(status));
+
+    HttpSolrServer server = new HttpSolrServer(jetty.getBaseUrl().toString() +
+                                               "/debug/foo");
+    try {
+      DebugServlet.setErrorCode(status);
+      try {
+        SolrQuery q = new SolrQuery("foo");
+        server.query(q, METHOD.GET);
+        fail("Didn't get excepted exception from oversided request");
+      } catch (SolrException e) {
+        System.out.println(e);
+        assertEquals("Unexpected exception status code", status, e.code());
+      }
+    } finally {
+      server.shutdown();
+      DebugServlet.clear();
+    }
+  }
+
   @Test
   public void testQuery(){
     DebugServlet.clear();