You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ry...@apache.org on 2012/03/06 22:59:31 UTC

svn commit: r1297749 - in /lucene/dev/trunk/solr: ./ core/src/java/org/apache/solr/client/solrj/embedded/ core/src/java/org/apache/solr/servlet/ solrj/src/java/org/apache/solr/client/solrj/impl/ solrj/src/java/org/apache/solr/client/solrj/request/ solr...

Author: ryan
Date: Tue Mar  6 21:59:31 2012
New Revision: 1297749

URL: http://svn.apache.org/viewvc?rev=1297749&view=rev
Log:
SOLR-141: Errors and Exceptions are formated by ResponseWriter.

Modified:
    lucene/dev/trunk/solr/CHANGES.txt
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
    lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CommonsHttpSolrServer.java
    lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java
    lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/SolrException.java
    lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java

Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1297749&r1=1297748&r2=1297749&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Tue Mar  6 21:59:31 2012
@@ -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
 ----------------------
 

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java?rev=1297749&r1=1297748&r2=1297749&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java Tue Mar  6 21:59:31 2012
@@ -155,6 +155,9 @@ public class EmbeddedSolrServer extends 
       
       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 
     catch( IOException iox ) {
       throw iox;
     }
+    catch( SolrException sx ) {
+      throw sx;
+    }
     catch( Exception ex ) {
       throw new SolrServerException( ex );
     }

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java?rev=1297749&r1=1297748&r2=1297749&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java Tue Mar  6 21:59:31 2012
@@ -49,6 +49,7 @@ import org.apache.solr.common.cloud.Slic
 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 implemen
       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 implemen
             }
             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 implemen
     // 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,43 +363,38 @@ public class SolrDispatchFilter implemen
   private void writeResponse(SolrQueryResponse solrRsp, ServletResponse response,
                              QueryResponseWriter responseWriter, SolrQueryRequest solrReq, Method reqMethod)
           throws IOException {
+
+    // Now write it out
+    final String ct = responseWriter.getContentType(solrReq, solrRsp);
+    // don't call setContentType on null
+    if (null != ct) response.setContentType(ct); 
+
     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) {
-        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();
-        }
+      NamedList info = new SimpleOrderedMap();
+      int code = getErrorInfo(solrRsp.getException(),info);
+      solrRsp.add("error", info);
+      ((HttpServletResponse) response).setStatus(code);
+    }
+    
+    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
     }
+    //else http HEAD request, nothing to write out, waited this long just to get ContentType
   }
-
-  protected void execute( HttpServletRequest req, SolrRequestHandler handler, SolrQueryRequest sreq, SolrQueryResponse rsp) {
-    // a custom filter could add more stuff to the request before passing it on.
-    // for example: sreq.getContext().put( "HttpServletRequest", req );
-    // used for logging query stats in SolrCore.execute()
-    sreq.getContext().put( "webapp", req.getContextPath() );
-    sreq.getCore().execute( handler, sreq, rsp );
-  }
-
-  protected void sendError(HttpServletResponse res, Throwable ex) throws IOException {
+  
+  protected int getErrorInfo(Throwable ex, NamedList info) {
     int code=500;
-    String trace = "";
     if( ex instanceof SolrException ) {
       code = ((SolrException)ex).code();
     }
@@ -418,14 +404,16 @@ public class SolrDispatchFilter implemen
       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));
-      trace = "\n\n"+sw.toString();
-
       SolrException.log(log, null, ex);
+      info.add("trace", sw.toString());
 
       // non standard codes have undefined results with various servers
       if( code < 100 ) {
@@ -433,8 +421,45 @@ public class SolrDispatchFilter implemen
         code = 500;
       }
     }
+    info.add("code", new Integer(code));
+    return code;
+  }
+
+  protected void execute( HttpServletRequest req, SolrRequestHandler handler, SolrQueryRequest sreq, SolrQueryResponse rsp) {
+    // a custom filter could add more stuff to the request before passing it on.
+    // for example: sreq.getContext().put( "HttpServletRequest", req );
+    // used for logging query stats in SolrCore.execute()
+    sreq.getContext().put( "webapp", req.getContextPath() );
+    sreq.getCore().execute( handler, sreq, rsp );
+  }
 
-    res.sendError( code, msg + trace );
+  protected void sendError(SolrCore core, 
+      SolrQueryRequest req, 
+      ServletRequest request, 
+      HttpServletResponse response, 
+      Throwable ex) throws IOException {
+    try {
+      SolrQueryResponse solrResp = new SolrQueryResponse();
+      if(ex instanceof Exception) {
+        solrResp.setException((Exception)ex);
+      }
+      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() );
+    }
   }
 
   //---------------------------------------------------------------------

Modified: lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CommonsHttpSolrServer.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CommonsHttpSolrServer.java?rev=1297749&r1=1297748&r2=1297749&view=diff
==============================================================================
--- lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CommonsHttpSolrServer.java (original)
+++ lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CommonsHttpSolrServer.java Tue Mar  6 21:59:31 2012
@@ -423,17 +423,7 @@ public class CommonsHttpSolrServer exten
     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 exten
           }
         }
       }
-      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);

Modified: lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java?rev=1297749&r1=1297748&r2=1297749&view=diff
==============================================================================
--- lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java (original)
+++ lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/QueryRequest.java Tue Mar  6 21:59:31 2012
@@ -21,6 +21,7 @@ import org.apache.solr.client.solrj.Solr
 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 SolrRe
       return res;
     } catch (SolrServerException e){
       throw e;
+    } catch (SolrException s){
+      throw s;
     } catch (Exception e) {
       throw new SolrServerException("Error executing query", e);
     }

Modified: lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/SolrException.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/SolrException.java?rev=1297749&r1=1297748&r2=1297749&view=diff
==============================================================================
--- lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/SolrException.java (original)
+++ lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/SolrException.java Tue Mar  6 21:59:31 2012
@@ -56,7 +56,8 @@ public class SolrException extends Runti
   };
 
   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 Runti
   }
 
   public SolrException(ErrorCode code, Throwable th) {
-    this(code, null, th);
+    super(th);
+    this.code = code.code;
   }
   
   int code=0;

Modified: lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java?rev=1297749&r1=1297748&r2=1297749&view=diff
==============================================================================
--- lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java (original)
+++ lucene/dev/trunk/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java Tue Mar  6 21:59:31 2012
@@ -48,9 +48,11 @@ import org.apache.solr.client.solrj.resp
 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 e
     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