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

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

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


##########
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 the code the limit is hard coded to 1024 characters; may example only showed an excerpt of the excerpt. Also, the threshold could be made configurable via the context.
   What I want to avoid is unconditionally loading everything into a string because for large incorrect responses this could again make investigation difficult.
   
   I was also thinking that QueryExceptionHTTP would be more appropriate - but the original code used QueryExeception; so I thought maybe @afs had something in mind when he did not use the former (such as that a non-handleable response content type is not really an HTTP issue).
   
   Furthermore, there seems to be more strange things going on with Jena over Virtuoso. In my specific case the problem is that Virtuoso sometimes returns plain text/html for queries - rather than some virtuoso error. But with this PR at least this becomes visible. So it's seems to be related to that Virtuoso instance (DBpedia) rather than Jena.
   
   I also intend to extend this PR with logging the HTTP request headers on error during this week - because some of my queries sometimes hang - but without knowing the exact headers I can't reproduce it - if I paste the query into the Web form it works.
   
   



-- 
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