SOLR-141: Errors and Exceptions are formated by ResponseWriter.

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

solr/CHANGES.txt

@@ -1821,6 +1821,10 @@ Other Changes
   servlet containers was therefore removed and is now ignored if set.
   Output is always UTF-8.  (uschindler, yonik, rmuir)
 
+* SOLR-141: Errors and Exceptions are formated by ResponseWriter.
+  (Mike Sokolov, Rich Cariens, Daniel Naber, ryan)
+
+
 Build
 ----------------------
 

solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java

@@ -155,6 +155,9 @@ public class EmbeddedSolrServer extends SolrServer
       
       core.execute( handler, req, rsp );
       if( rsp.getException() != null ) {
+        if(rsp.getException() instanceof SolrException) {
+          throw rsp.getException();
+        }
         throw new SolrServerException( rsp.getException() );
       }
       
@@ -219,6 +222,9 @@ public class EmbeddedSolrServer extends SolrServer
     catch( IOException iox ) {
       throw iox;
     }
+    catch( SolrException sx ) {
+      throw sx;
+    }
     catch( Exception ex ) {
       throw new SolrServerException( ex );
     }

solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java

@@ -49,6 +49,7 @@ import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.util.FastWriter;
 import org.apache.solr.common.util.ContentStreamBase;
 import org.apache.solr.core.*;
@@ -134,13 +135,13 @@ public class SolrDispatchFilter implements Filter
       return;
     }
     CoreContainer cores = this.cores;
+    SolrCore core = null;
+    SolrQueryRequest solrReq = null;
     
     if( request instanceof HttpServletRequest) {
       HttpServletRequest req = (HttpServletRequest)request;
       HttpServletResponse resp = (HttpServletResponse)response;
       SolrRequestHandler handler = null;
-      SolrQueryRequest solrReq = null;
-      SolrCore core = null;
       String corename = "";
       try {
         // put the core container in request attribute
@@ -269,21 +270,11 @@ public class SolrDispatchFilter implements Filter
             }
             return; // we are done with a valid handler
           }
-          // otherwise (we have a core), let's ensure the core is in the SolrCore request attribute so
-          // a servlet/jsp can retrieve it
-          else {
-            req.setAttribute("org.apache.solr.SolrCore", core);
-            // Modify the request so each core gets its own /admin
-            if( path.startsWith( "/admin" ) ) {
-              req.getRequestDispatcher( pathPrefix == null ? path : pathPrefix + path ).forward( request, response );
-              return;
-            }
-          }
         }
         log.debug("no handler or core retrieved for " + path + ", follow through...");
       } 
       catch (Throwable ex) {
-        sendError( (HttpServletResponse)response, ex );
+        sendError( core, solrReq, request, (HttpServletResponse)response, ex );
         return;
       } 
       finally {
@@ -300,7 +291,7 @@ public class SolrDispatchFilter implements Filter
     // Otherwise let the webapp handle the request
     chain.doFilter(request, response);
   }
-
+  
   private SolrCore getCoreByCollection(CoreContainer cores, String corename, String path) {
     String collection = corename;
     ZkStateReader zkStateReader = cores.getZkController().getZkStateReader();
@@ -372,30 +363,66 @@ public class SolrDispatchFilter implements Filter
   private void writeResponse(SolrQueryResponse solrRsp, ServletResponse response,
                              QueryResponseWriter responseWriter, SolrQueryRequest solrReq, Method reqMethod)
           throws IOException {
-    if (solrRsp.getException() != null) {
-      sendError((HttpServletResponse) response, solrRsp.getException());
-    } else {
-      // Now write it out
-      final String ct = responseWriter.getContentType(solrReq, solrRsp);
-      // don't call setContentType on null
-      if (null != ct) response.setContentType(ct); 
 
-      if (Method.HEAD != reqMethod) {
+    // Now write it out
-        if (responseWriter instanceof BinaryQueryResponseWriter) {
+    final String ct = responseWriter.getContentType(solrReq, solrRsp);
-          BinaryQueryResponseWriter binWriter = (BinaryQueryResponseWriter) responseWriter;
+    // don't call setContentType on null
-          binWriter.write(response.getOutputStream(), solrReq, solrRsp);
+    if (null != ct) response.setContentType(ct); 
-        } else {
+
-          String charset = ContentStreamBase.getCharsetFromContentType(ct);
+    if (solrRsp.getException() != null) {
-          Writer out = (charset == null || charset.equalsIgnoreCase("UTF-8"))
+      NamedList info = new SimpleOrderedMap();
-            ? new OutputStreamWriter(response.getOutputStream(), UTF8)
+      int code = getErrorInfo(solrRsp.getException(),info);
-            : new OutputStreamWriter(response.getOutputStream(), charset);
+      solrRsp.add("error", info);
-          out = new FastWriter(out);
+      ((HttpServletResponse) response).setStatus(code);
-          responseWriter.write(out, solrReq, solrRsp);
-          out.flush();
-        }
-      }
-      //else http HEAD request, nothing to write out, waited this long just to get ContentType
     }
+    
+    if (Method.HEAD != reqMethod) {
+      if (responseWriter instanceof BinaryQueryResponseWriter) {
+        BinaryQueryResponseWriter binWriter = (BinaryQueryResponseWriter) responseWriter;
+        binWriter.write(response.getOutputStream(), solrReq, solrRsp);
+      } else {
+        String charset = ContentStreamBase.getCharsetFromContentType(ct);
+        Writer out = (charset == null || charset.equalsIgnoreCase("UTF-8"))
+          ? new OutputStreamWriter(response.getOutputStream(), UTF8)
+          : new OutputStreamWriter(response.getOutputStream(), charset);
+        out = new FastWriter(out);
+        responseWriter.write(out, solrReq, solrRsp);
+        out.flush();
+      }
+    }
+    //else http HEAD request, nothing to write out, waited this long just to get ContentType
+  }
+  
+  protected int getErrorInfo(Throwable ex, NamedList info) {
+    int code=500;
+    if( ex instanceof SolrException ) {
+      code = ((SolrException)ex).code();
+    }
+
+    String msg = null;
+    for (Throwable th = ex; th != null; th = th.getCause()) {
+      msg = th.getMessage();
+      if (msg != null) break;
+    }
+    if(msg != null) {
+      info.add("msg", msg);
+    }
+    
+    // For any regular code, don't include the stack trace
+    if( code == 500 || code < 100 ) {
+      StringWriter sw = new StringWriter();
+      ex.printStackTrace(new PrintWriter(sw));
+      SolrException.log(log, null, ex);
+      info.add("trace", sw.toString());
+
+      // non standard codes have undefined results with various servers
+      if( code < 100 ) {
+        log.warn( "invalid return code: "+code );
+        code = 500;
+      }
+    }
+    info.add("code", new Integer(code));
+    return code;
   }
 
   protected void execute( HttpServletRequest req, SolrRequestHandler handler, SolrQueryRequest sreq, SolrQueryResponse rsp) {
@@ -406,35 +433,33 @@ public class SolrDispatchFilter implements Filter
     sreq.getCore().execute( handler, sreq, rsp );
   }
 
-  protected void sendError(HttpServletResponse res, Throwable ex) throws IOException {
+  protected void sendError(SolrCore core, 
-    int code=500;
+      SolrQueryRequest req, 
-    String trace = "";
+      ServletRequest request, 
-    if( ex instanceof SolrException ) {
+      HttpServletResponse response, 
-      code = ((SolrException)ex).code();
+      Throwable ex) throws IOException {
-    }
+    try {
-
+      SolrQueryResponse solrResp = new SolrQueryResponse();
-    String msg = null;
+      if(ex instanceof Exception) {
-    for (Throwable th = ex; th != null; th = th.getCause()) {
+        solrResp.setException((Exception)ex);
-      msg = th.getMessage();
-      if (msg != null) break;
-    }
-
-    // For any regular code, don't include the stack trace
-    if( code == 500 || code < 100 ) {
-      StringWriter sw = new StringWriter();
-      ex.printStackTrace(new PrintWriter(sw));
-      trace = "\n\n"+sw.toString();
-
-      SolrException.log(log, null, ex);
-
-      // non standard codes have undefined results with various servers
-      if( code < 100 ) {
-        log.warn( "invalid return code: "+code );
-        code = 500;
       }
+      else {
+        solrResp.setException(new RuntimeException(ex));
+      }
+      if(core==null) {
+        core = cores.getCore(""); // default core
+      }
+      if(req==null) {
+        req = new SolrQueryRequestBase(core,new ServletSolrParams(request)) {};
+      }
+      QueryResponseWriter writer = core.getQueryResponseWriter(req);
+      writeResponse(solrResp, response, writer, req, Method.GET);
+    }
+    catch( Throwable t ) { // This error really does not matter
+      SimpleOrderedMap info = new SimpleOrderedMap();
+      int code=getErrorInfo(ex, info);
+      response.sendError( code, info.toString() );
     }
-
-    res.sendError( code, msg + trace );
   }
 
   //---------------------------------------------------------------------

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

@@ -423,17 +423,7 @@ public class CommonsHttpSolrServer extends SolrServer
     try {
       // Execute the method.
       //System.out.println( "EXECUTE:"+method.getURI() );
-
       int statusCode = _httpClient.executeMethod(method);
-      if (statusCode != HttpStatus.SC_OK) {
-        StringBuilder msg = new StringBuilder();
-        msg.append( method.getStatusLine().getReasonPhrase() );
-        msg.append( "\n\n" );
-        msg.append( method.getStatusText() );
-        msg.append( "\n\n" );
-        msg.append( "request: "+method.getURI() );
-        throw new SolrException(SolrException.ErrorCode.getErrorCode(statusCode), java.net.URLDecoder.decode(msg.toString(), "UTF-8") );
-      }
 
       // Read the contents
       String charset = "UTF-8";
@@ -474,7 +464,30 @@ public class CommonsHttpSolrServer extends SolrServer
           }
         }
       }
-      return processor.processResponse(respBody, charset);
+      
+      NamedList<Object> rsp = processor.processResponse(respBody, charset);
+      if (statusCode != HttpStatus.SC_OK) {
+        String reason = null;
+        try {
+          NamedList err = (NamedList)rsp.get("error");
+          if(err!=null) {
+            reason = (String)err.get("msg");
+            // TODO? get the trace?
+          }
+        }
+        catch(Exception ex) {}
+        if(reason == null) {
+          StringBuilder msg = new StringBuilder();
+          msg.append( method.getStatusLine().getReasonPhrase() );
+          msg.append( "\n\n" );
+          msg.append( method.getStatusText() );
+          msg.append( "\n\n" );
+          msg.append( "request: "+method.getURI() );
+          reason = java.net.URLDecoder.decode(msg.toString(), "UTF-8");
+        }
+        throw new SolrException(SolrException.ErrorCode.getErrorCode(statusCode), reason );
+      }
+      return rsp;
     }
     catch (HttpException e) {
       throw new SolrServerException(getBaseURL(), e);

solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java

@@ -21,6 +21,7 @@ import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServer;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.ContentStream;
@@ -91,6 +92,8 @@ public class QueryRequest extends SolrRequest
       return res;
     } catch (SolrServerException e){
       throw e;
+    } catch (SolrException s){
+      throw s;
     } catch (Exception e) {
       throw new SolrServerException("Error executing query", e);
     }

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

@@ -56,7 +56,8 @@ public class SolrException extends RuntimeException {
   };
 
   public SolrException(ErrorCode code, String msg) {
-    this(code, msg, null);
+    super(msg);
+    this.code = code.code;
   }
   public SolrException(ErrorCode code, String msg, Throwable th) {
     super(msg, th);
@@ -64,7 +65,8 @@ public class SolrException extends RuntimeException {
   }
 
   public SolrException(ErrorCode code, Throwable th) {
-    this(code, null, th);
+    super(th);
+    this.code = code.code;
   }
   
   int code=0;

solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java

@@ -48,9 +48,11 @@ import org.apache.solr.client.solrj.response.UpdateResponse;
 import org.apache.solr.client.solrj.util.ClientUtils;
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.util.XML;
 import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.params.AnalysisParams;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.FacetParams;
 import org.junit.Test;
@@ -463,6 +465,42 @@ abstract public class SolrExampleTests extends SolrJettyTestBase
     Assert.fail("commitWithin failed to commit");
   }
 
+  @Test
+  public void testErrorHandling() throws Exception
+  {    
+    SolrServer server = getSolrServer();
+
+    SolrQuery query = new SolrQuery();
+    query.set(CommonParams.QT, "/analysis/field");
+    query.set(AnalysisParams.FIELD_TYPE, "int");
+    query.set(AnalysisParams.FIELD_VALUE, "hello");
+    try {
+      server.query( query );
+      Assert.fail("should have a number format exception");
+    }
+    catch(SolrException ex) {
+      assertEquals(400, ex.code());
+      assertEquals("Invalid Number: hello", ex.getMessage());  // The reason should get passed through
+    }
+    catch(Throwable t) {
+      t.printStackTrace();
+      Assert.fail("should have thrown a SolrException! not: "+t);
+    }
+    
+    try {
+      server.deleteByQuery( "??::??" ); // query syntax error
+      Assert.fail("should have a number format exception");
+    }
+    catch(SolrException ex) {
+      assertEquals(400, ex.code());
+      assertTrue(ex.getMessage().indexOf("??::??")>0);  // The reason should get passed through
+    }
+    catch(Throwable t) {
+      t.printStackTrace();
+      Assert.fail("should have thrown a SolrException! not: "+t);
+    }
+  }
+
 
   @Test
   public void testAugmentFields() throws Exception