Solr
  1. Solr
  2. SOLR-5940

Make post.jar report back detailed error in case of 400 responses

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.7
    • Fix Version/s: 4.9, Trunk
    • Component/s: scripts and tools
    • Labels:
      None

      Description

      Currently post.jar does not print detailed error message that is encountered during indexing. In certain use cases, it's helpful to see the error message so that clients can take appropriate actions.

      In 4.7, here's what gets shown if there is an error during indexing:

      SimplePostTool: WARNING: Solr returned an error #400 Bad Request
      SimplePostTool: WARNING: IOException while reading response: java.io.IOException: Server returned HTTP response code: 400 for URL: http://localhost:8983/solr/update

      It would be helpful to print out the "msg" that is returned from Solr.

      1. solr-5940.patch
        1 kB
        Sameer Maggon
      2. solr-5940.patch
        1 kB
        Shalin Shekhar Mangar
      3. solr-5940.patch
        2 kB
        Uwe Schindler
      4. solr-5940.patch
        2 kB
        Shalin Shekhar Mangar
      5. solr-5940.patch
        4 kB
        Uwe Schindler

        Activity

        Sameer Maggon created issue -
        Hide
        Sameer Maggon added a comment -

        Patch that adds the error returned by solr and prints out as a warning

        Show
        Sameer Maggon added a comment - Patch that adds the error returned by solr and prints out as a warning
        Sameer Maggon made changes -
        Field Original Value New Value
        Attachment solr-5940.patch [ 12638057 ]
        Shalin Shekhar Mangar made changes -
        Assignee Shalin Shekhar Mangar [ shalinmangar ]
        Hide
        Sameer Maggon added a comment -

        Thanks Shalin for picking it up - let me know of any feedback!

        Show
        Sameer Maggon added a comment - Thanks Shalin for picking it up - let me know of any feedback!
        Hide
        Shalin Shekhar Mangar added a comment -

        Thanks Sameer. I have added "UTF-8" charset in the InputStreamReader constructor and put the whole thing inside a try-with-resources block.

        I'll commit this shortly.

        Show
        Shalin Shekhar Mangar added a comment - Thanks Sameer. I have added "UTF-8" charset in the InputStreamReader constructor and put the whole thing inside a try-with-resources block. I'll commit this shortly.
        Shalin Shekhar Mangar made changes -
        Attachment solr-5940.patch [ 12649819 ]
        Hide
        Uwe Schindler added a comment - - edited

        If you are reading messages from a HTTP server, the default encoding is ISO-8859-1 (according to HTTP/1.1 standards). Only if the server returns a different charset in its Content-Type header, it must be changed.

        From HTTP spec:

        The "charset" parameter is used with some media types to define the character set (section 3.4) of the data. When no explicit charset parameter is provided by the sender, media subtypes of the "text" type are defined to have a default charset value of "ISO-8859-1" when received via HTTP. Data in character sets other than "ISO-8859-1" or its subsets MUST be labeled with an appropriate charset value. See section 3.4.1 for compatibility problems.

        Show
        Uwe Schindler added a comment - - edited If you are reading messages from a HTTP server, the default encoding is ISO-8859-1 (according to HTTP/1.1 standards). Only if the server returns a different charset in its Content-Type header, it must be changed. From HTTP spec: The "charset" parameter is used with some media types to define the character set (section 3.4) of the data. When no explicit charset parameter is provided by the sender, media subtypes of the "text" type are defined to have a default charset value of "ISO-8859-1" when received via HTTP. Data in character sets other than "ISO-8859-1" or its subsets MUST be labeled with an appropriate charset value. See section 3.4.1 for compatibility problems.
        Hide
        Uwe Schindler added a comment -

        In addition, in Java 7, you should use the StandardCharsets class from the JDK. This spares one lookup of the charset by name and is bullet-proof.

        Show
        Uwe Schindler added a comment - In addition, in Java 7, you should use the StandardCharsets class from the JDK. This spares one lookup of the charset by name and is bullet-proof.
        Hide
        Shalin Shekhar Mangar added a comment -

        Thanks Uwe. I'll fix.

        Show
        Shalin Shekhar Mangar added a comment - Thanks Uwe. I'll fix.
        Hide
        Uwe Schindler added a comment -

        Patch to handle all according HTTP spec. It also uses StringBuilder and uses try-with-resources also for the standard InputStream.

        Show
        Uwe Schindler added a comment - Patch to handle all according HTTP spec. It also uses StringBuilder and uses try-with-resources also for the standard InputStream.
        Uwe Schindler made changes -
        Attachment solr-5940.patch [ 12649823 ]
        Hide
        Shalin Shekhar Mangar added a comment -

        Awesome! Thanks Uwe! I'll commit this in a bit.

        Show
        Shalin Shekhar Mangar added a comment - Awesome! Thanks Uwe! I'll commit this in a bit.
        Hide
        Uwe Schindler added a comment -

        I would only change the whitespace in the StringBuilder (.append(' ')) to a .append(\n'). I tested the patch by providing a fake URL that always returns an error, and the formatting of the response was unreadable.

        Show
        Uwe Schindler added a comment - I would only change the whitespace in the StringBuilder ( .append(' ') ) to a .append(\n') . I tested the patch by providing a fake URL that always returns an error, and the formatting of the response was unreadable.
        Hide
        Shalin Shekhar Mangar added a comment -

        Done, thanks.

        Show
        Shalin Shekhar Mangar added a comment - Done, thanks.
        Shalin Shekhar Mangar made changes -
        Attachment solr-5940.patch [ 12649826 ]
        Hide
        Uwe Schindler added a comment -

        I found out that the error reporting is missing for non-POST requests. I will provide a new patch that also reports errors the same way on GET requests (e.g. when committing)

        Show
        Uwe Schindler added a comment - I found out that the error reporting is missing for non-POST requests. I will provide a new patch that also reports errors the same way on GET requests (e.g. when committing)
        Hide
        Shalin Shekhar Mangar added a comment -

        It's best if you take issue up Uwe since you're doing all the work anyway

        Many thanks!

        Show
        Shalin Shekhar Mangar added a comment - It's best if you take issue up Uwe since you're doing all the work anyway Many thanks!
        Shalin Shekhar Mangar made changes -
        Assignee Shalin Shekhar Mangar [ shalinmangar ] Uwe Schindler [ thetaphi ]
        Hide
        Uwe Schindler added a comment -

        New patch:

        • Also report server response on errors when doing GET requests
        • Use try-with-resources also for output stream
        • report only for HTTP status codes >= 400. HTTP 200 is not the only successful response code.

        I take the issue.

        Show
        Uwe Schindler added a comment - New patch: Also report server response on errors when doing GET requests Use try-with-resources also for output stream report only for HTTP status codes >= 400. HTTP 200 is not the only successful response code. I take the issue.
        Uwe Schindler made changes -
        Attachment solr-5940.patch [ 12649830 ]
        Uwe Schindler made changes -
        Fix Version/s 4.9 [ 12326731 ]
        Fix Version/s 5.0 [ 12321664 ]
        Hide
        ASF subversion and git services added a comment -

        Commit 1601970 from Uwe Schindler in branch 'dev/trunk'
        [ https://svn.apache.org/r1601970 ]

        SOLR-5940: post.jar reports back detailed error in case of error responses

        Show
        ASF subversion and git services added a comment - Commit 1601970 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1601970 ] SOLR-5940 : post.jar reports back detailed error in case of error responses
        Hide
        ASF subversion and git services added a comment -

        Commit 1601973 from Uwe Schindler in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1601973 ]

        Merged revision(s) 1601970 from lucene/dev/trunk:
        SOLR-5940: post.jar reports back detailed error in case of error responses

        Show
        ASF subversion and git services added a comment - Commit 1601973 from Uwe Schindler in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1601973 ] Merged revision(s) 1601970 from lucene/dev/trunk: SOLR-5940 : post.jar reports back detailed error in case of error responses
        Hide
        Uwe Schindler added a comment -

        Committed. Thanks Sameer!

        I fixed the code to not NPE if we get no error response (on stuff like disallowed redirects on POST, this might happen). NULL is a valid return value of getErrorStream().

        Show
        Uwe Schindler added a comment - Committed. Thanks Sameer! I fixed the code to not NPE if we get no error response (on stuff like disallowed redirects on POST, this might happen). NULL is a valid return value of getErrorStream().
        Uwe Schindler made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Sameer Maggon added a comment -

        Awesome! Thanks guys!

        Show
        Sameer Maggon added a comment - Awesome! Thanks guys!
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        71d 23h 2m 1 Uwe Schindler 11/Jun/14 18:52

          People

          • Assignee:
            Uwe Schindler
            Reporter:
            Sameer Maggon
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development