[GitHub] [jena] afs commented on a diff in pull request #1483: Improved http content type issue exception message

[GitBox <gi...@apache.org>]

afs commented on code in PR #1483:
URL: https://github.com/apache/jena/pull/1483#discussion_r956233801


##########
jena-arq/src/main/java/org/apache/jena/sparql/exec/http/QueryExecHTTP.java:
##########
@@ -190,10 +191,20 @@ private RowSet execRowSet() {
 
         // Map to lang, with pragmatic alternatives.
         Lang lang = WebContent.contentTypeToLangResultSet(actualContentType);
-        if ( lang == null )
-            throw new QueryException("Endpoint returned Content-Type: " + actualContentType + " which is not recognized for SELECT queries");
-        if ( !ResultSetReaderRegistry.isRegistered(lang) )
-            throw new QueryException("Endpoint returned Content-Type: " + actualContentType + " which is not supported for SELECT queries");
+        boolean unknownLang = lang == null;
+        boolean unsupportedFormat = !unknownLang && !ResultSetReaderRegistry.isRegistered(lang);
+
+        if ( unknownLang || unsupportedFormat ) {
+            int statusCode = response.statusCode();
+            String statusCodeMsg = HttpSC.getMessage(statusCode);
+            String bodyStr = IO.abbreviateAsUTF8(in, 1024, "...");
+            String errorTerm = unknownLang ? "recognized" : "supported";
+
+            throw new QueryException(String.format(
+                    "Endpoint returned Content-Type: %s which is not %s for SELECT queries. Status code %d %s, body: %s",
+                    actualContentType, errorTerm, statusCode, statusCodeMsg, bodyStr));

Review Comment:
   In practice, I think developers will often check the endpoint with tools such as curl and wget to get more information. The whole thing might be very large for a log item.
   
   (and, yes, Virtuoso does seem to return text/html - it seems to snoop the request to determine whether it is a programmatic query or now, and also users don't always find its endpoint but the SPARQL web UI.)
   
   > QueryExectipionHTTP
   
   I _think_ it is because it is not strictly an HTTP-level error. The HTTP mechanisms are fine. It's the application usage that is mismatched.
   
   



##########
jena-base/src/main/java/org/apache/jena/atlas/io/IO.java:
##########
@@ -441,6 +442,33 @@ public static String readWholeFileAsUTF8(InputStream in) {
         }
     }
 
+    /** Read a given number of bytes from a stream as UTF-8. Append a given marker such as "..." if
+     * more data remains in the stream.
+    *
+    * @param in           InputStream to be read
+    * @param maxWidth     Maximum number of bytes (rather than characters) to read from the input stream
+    * @param abbrevMarker A string to append to the result if the input stream was not fully consumed
+    * @return      String
+    */
+    public static String abbreviateAsUTF8(InputStream in, int maxWidth, String abbrevMarker) {
+        String result = IO.readWholeFileAsUTF8(new BoundedInputStream(in, maxWidth));
+
+        // Append the marker unless read successfully returns -1
+        boolean appendAbbrevMarker = true;
+        try {
+            appendAbbrevMarker = in.read() != -1;
+        } catch (Exception e) {

Review Comment:
   `Throwable` (UTF-8 encoding errors can be Errors as well as Exceptions)
   



##########
jena-base/src/main/java/org/apache/jena/atlas/io/IO.java:
##########
@@ -441,6 +442,33 @@ public static String readWholeFileAsUTF8(InputStream in) {
         }
     }
 
+    /** Read a given number of bytes from a stream as UTF-8. Append a given marker such as "..." if
+     * more data remains in the stream.
+    *
+    * @param in           InputStream to be read
+    * @param maxWidth     Maximum number of bytes (rather than characters) to read from the input stream
+    * @param abbrevMarker A string to append to the result if the input stream was not fully consumed
+    * @return      String
+    */
+    public static String abbreviateAsUTF8(InputStream in, int maxWidth, String abbrevMarker) {
+        String result = IO.readWholeFileAsUTF8(new BoundedInputStream(in, maxWidth));
+
+        // Append the marker unless read successfully returns -1
+        boolean appendAbbrevMarker = true;
+        try {
+            appendAbbrevMarker = in.read() != -1;
+        } catch (Exception e) {
+            // Ignored
+        }
+
+        if (appendAbbrevMarker) {
+            result += abbrevMarker;
+        }
+
+        return result;
+    }
+
+

Review Comment:
   One blank line. This is the case throughout (most of) Jena.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org