Solr
  1. Solr
  2. SOLR-204

Let solrconfig.xml configure the SolrDispatchFilter to handle /select

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.2
    • Component/s: None
    • Labels:
      None

      Description

      The major reason to make everythign use the SolrDispatchFilter is that we would have consistent error handling. Currently,

      SolrServlet spits back errors using:
      PrintWriter writer = response.getWriter();
      writer.write(msg);

      and the SolrDispatchFilter spits them back using:
      res.sendError( code, ex.getMessage() );

      Using "sendError" lets the servlet container format the code so it shows up ok in a browser. Without it, you may have to view source to see the error.

      Aditionaly, SolrDispatchFilter is more decerning about including stack trace. It only includes a stack trace of 500 or an unknown response code.

      Eventually, the error should probably be formatted in the requested format - SOLR-141.

      1. SOLR-204-HandleSelect.patch
        5 kB
        Ryan McKinley
      2. SOLR-204-HandleSelect.patch
        6 kB
        Ryan McKinley
      3. SOLR-204-HandleSelect.patch
        5 kB
        Ryan McKinley

        Activity

        Hide
        Ryan McKinley added a comment -

        this removes the "handle-select" init param from web.xml and adds:

        <requestDispatcher handleSelect="true" />

        to solrconfig.xml

        The one thing I would maybe change is to make the config more of a tree then a list, maybe we should have:

        <requestDispatcher handleSelect="true" >
        <requestParsers enableRemoteStreaming="false" multipartUploadLimitInKB="2048" />
        </requestDispatcher>

        rather then:

        <requestDispatcher handleSelect="true" />
        <requestParsers enableRemoteStreaming="false" multipartUploadLimitInKB="2048" />

        This would change a configuration that has been added since 1.1

        • - - - - -

        This patch also cleans up reading enableRemoteStreaming and multipartUploadLimitInKB, when i wrote it i did not know about the much cleaner:

        this.enableRemoteStreams = SolrConfig.config.getBool(
        "requestParsers/@enableRemoteStreaming", false );

        Show
        Ryan McKinley added a comment - this removes the "handle-select" init param from web.xml and adds: <requestDispatcher handleSelect="true" /> to solrconfig.xml The one thing I would maybe change is to make the config more of a tree then a list, maybe we should have: <requestDispatcher handleSelect="true" > <requestParsers enableRemoteStreaming="false" multipartUploadLimitInKB="2048" /> </requestDispatcher> rather then: <requestDispatcher handleSelect="true" /> <requestParsers enableRemoteStreaming="false" multipartUploadLimitInKB="2048" /> This would change a configuration that has been added since 1.1 - - - - - This patch also cleans up reading enableRemoteStreaming and multipartUploadLimitInKB, when i wrote it i did not know about the much cleaner: this.enableRemoteStreams = SolrConfig.config.getBool( "requestParsers/@enableRemoteStreaming", false );
        Hide
        Ryan McKinley added a comment -

        Updated version. This modifies solrconfig so it has a "requestDispatcher" section and moves requestParsers into that group.

        <requestDispatcher handleSelect="true" >
        <Unable to render embedded object: File (--Make sure your system has some authentication before enabling remote streaming) not found. -->
        <requestParsers enableRemoteStreaming="false" multipartUploadLimitInKB="2048" />
        </requestDispatcher>

        this means anyone using 'enableRemoteStreaming' since solr1.1 will need to modify solrconfig.xml.

        Show
        Ryan McKinley added a comment - Updated version. This modifies solrconfig so it has a "requestDispatcher" section and moves requestParsers into that group. <requestDispatcher handleSelect="true" > < Unable to render embedded object: File (--Make sure your system has some authentication before enabling remote streaming) not found. --> <requestParsers enableRemoteStreaming="false" multipartUploadLimitInKB="2048" /> </requestDispatcher> this means anyone using 'enableRemoteStreaming' since solr1.1 will need to modify solrconfig.xml.
        Hide
        Yonik Seeley added a comment -

        I wanted to try this out to see what sendError() output looks like, but the patch isn't applying cleanly.

        $ patch -p0 < c:/dl/SOLR-204*
        (Stripping trailing CRs from patch.)
        patching file src/test/test-files/solr/conf/solrconfig.xml
        (Stripping trailing CRs from patch.)
        patching file src/webapp/WEB-INF/web.xml
        (Stripping trailing CRs from patch.)
        patching file src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java
        Hunk #1 FAILED at 56.
        1 out of 1 hunk FAILED – saving rejects to file src/webapp/src/org/apache/solr/
        servlet/SolrDispatchFilter.java.rej
        (Stripping trailing CRs from patch.)
        patching file src/webapp/src/org/apache/solr/servlet/SolrRequestParsers.java
        (Stripping trailing CRs from patch.)
        patching file example/solr/conf/solrconfig.xml
        Hunk #1 succeeded at 231 (offset 8 lines).

        Show
        Yonik Seeley added a comment - I wanted to try this out to see what sendError() output looks like, but the patch isn't applying cleanly. $ patch -p0 < c:/dl/ SOLR-204 * (Stripping trailing CRs from patch.) patching file src/test/test-files/solr/conf/solrconfig.xml (Stripping trailing CRs from patch.) patching file src/webapp/WEB-INF/web.xml (Stripping trailing CRs from patch.) patching file src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java Hunk #1 FAILED at 56. 1 out of 1 hunk FAILED – saving rejects to file src/webapp/src/org/apache/solr/ servlet/SolrDispatchFilter.java.rej (Stripping trailing CRs from patch.) patching file src/webapp/src/org/apache/solr/servlet/SolrRequestParsers.java (Stripping trailing CRs from patch.) patching file example/solr/conf/solrconfig.xml Hunk #1 succeeded at 231 (offset 8 lines).
        Hide
        Ryan McKinley added a comment -

        applies cleanly with trunk

        Show
        Ryan McKinley added a comment - applies cleanly with trunk
        Hide
        Ryan McKinley added a comment -

        sendError lets the web app decide how to format the response body. Typically they put HTML with the status code, with a footer saying the "Jetty" or "Resin"

        This is what you get to configure with:

        <error-page>
        <exception-type>java.lang.Exception</exception-type>
        <location>/error</location>
        </error-page>
        <error-page><error-code>404</error-code><location>/error</location></error-page>
        etc

        Show
        Ryan McKinley added a comment - sendError lets the web app decide how to format the response body. Typically they put HTML with the status code, with a footer saying the "Jetty" or "Resin" This is what you get to configure with: <error-page> <exception-type>java.lang.Exception</exception-type> <location>/error</location> </error-page> <error-page><error-code>404</error-code><location>/error</location></error-page> etc
        Hide
        Yonik Seeley added a comment -

        OK cool, for something like an undefined field, it looks fine:
        "undefined field catdsfgsdg"

        But for something like a query parsing error, the only pointer to what the error is is in the stack trace, and you don't get that back. You just get: "Error parsing Lucene query"

        The logs show:
        SEVERE: org.apache.lucene.queryParser.ParseException: Cannot parse 'foo:': '' or '?' not allowed as first character in WildcardQuery
        at org.apache.lucene.queryParser.QueryParser.parse(QueryParser.java:149)
        at org.apache.solr.search.QueryParsing.parseQuery(QueryParsing.java:94)
        at org.apache.solr.request.StandardRequestHandler.handleRequestBody(StandardRequestHandler.java:85)
        at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:77)

        Hmmm, but I think this is an exception issue:

        In QueryParsing.java:
        } catch (ParseException e)

        { SolrCore.log(e); throw new SolrException(400,"Error parsing Lucene query",e); }

        should probably be something more like:
        throw new SolrException(400,"Query parsing error: " + e.getMessage() ,e);

        Show
        Yonik Seeley added a comment - OK cool, for something like an undefined field, it looks fine: "undefined field catdsfgsdg" But for something like a query parsing error, the only pointer to what the error is is in the stack trace, and you don't get that back. You just get: "Error parsing Lucene query" The logs show: SEVERE: org.apache.lucene.queryParser.ParseException: Cannot parse 'foo: ': ' ' or '?' not allowed as first character in WildcardQuery at org.apache.lucene.queryParser.QueryParser.parse(QueryParser.java:149) at org.apache.solr.search.QueryParsing.parseQuery(QueryParsing.java:94) at org.apache.solr.request.StandardRequestHandler.handleRequestBody(StandardRequestHandler.java:85) at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:77) Hmmm, but I think this is an exception issue: In QueryParsing.java: } catch (ParseException e) { SolrCore.log(e); throw new SolrException(400,"Error parsing Lucene query",e); } should probably be something more like: throw new SolrException(400,"Query parsing error: " + e.getMessage() ,e);
        Hide
        Ryan McKinley added a comment -

        >
        > should probably be something more like:
        > throw new SolrException(400,"Query parsing error: " + e.getMessage() ,e);
        >

        Yes, the other change is that errors for RequestDispatcher only print the stack trace if it is >=500, 400 (bad request) assumes the message will contain a user useful response.

        Show
        Ryan McKinley added a comment - > > should probably be something more like: > throw new SolrException(400,"Query parsing error: " + e.getMessage() ,e); > Yes, the other change is that errors for RequestDispatcher only print the stack trace if it is >=500, 400 (bad request) assumes the message will contain a user useful response.
        Show
        Ryan McKinley added a comment - added in: http://svn.apache.org/viewvc?view=rev&revision=533558

          People

          • Assignee:
            Ryan McKinley
            Reporter:
            Ryan McKinley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development