You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by kr...@apache.org on 2023/03/15 13:05:03 UTC

[solr] branch main updated: SOLR-16638: Fix Http2SolrClient's exception message when serverBaseUrl is null (#1356)

This is an automated email from the ASF dual-hosted git repository.

krisden pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 16109a48220 SOLR-16638: Fix Http2SolrClient's exception message when serverBaseUrl is null (#1356)
16109a48220 is described below

commit 16109a4822060fa08ccd21fa6af4914963d4d5f4
Author: Alex <st...@users.noreply.github.com>
AuthorDate: Wed Mar 15 06:04:57 2023 -0700

    SOLR-16638: Fix Http2SolrClient's exception message when serverBaseUrl is null (#1356)
---
 .../solr/client/solrj/impl/Http2SolrClient.java    | 37 ++++++++++++++--------
 .../client/solrj/impl/Http2SolrClientTest.java     | 32 +++++++++++++++++++
 2 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
index 8e463c3c7bc..78e191e696d 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -140,7 +140,7 @@ public class Http2SolrClient extends SolrClient {
   private List<HttpListenerFactory> listenerFactory = new ArrayList<>();
   private AsyncTracker asyncTracker = new AsyncTracker();
   /** The URL of the Solr server. */
-  private String serverBaseUrl;
+  private final String serverBaseUrl;
 
   private boolean closeClient;
   private ExecutorService executor;
@@ -159,6 +159,8 @@ public class Http2SolrClient extends SolrClient {
         serverBaseUrl = serverBaseUrl.substring(1, serverBaseUrl.length());
       }
       this.serverBaseUrl = serverBaseUrl;
+    } else {
+      this.serverBaseUrl = null;
     }
 
     if (builder.idleTimeoutMillis != null && builder.idleTimeoutMillis > 0) {
@@ -458,7 +460,8 @@ public class Http2SolrClient extends SolrClient {
                       assert ObjectReleaseTracker.track(is);
                       try {
                         NamedList<Object> body =
-                            processErrorsAndResponse(solrRequest, parser, response, is);
+                            processErrorsAndResponse(
+                                solrRequest, parser, response, is, req.getURI().toString());
                         mdcCopyHelper.onBegin(null);
                         log.debug("response processing success");
                         asyncListener.onSuccess(body);
@@ -504,7 +507,7 @@ public class Http2SolrClient extends SolrClient {
       Response response = listener.get(idleTimeoutMillis, TimeUnit.MILLISECONDS);
       InputStream is = listener.getInputStream();
       assert ObjectReleaseTracker.track(is);
-      return processErrorsAndResponse(solrRequest, parser, response, is);
+      return processErrorsAndResponse(solrRequest, parser, response, is, req.getURI().toString());
     } catch (InterruptedException e) {
       Thread.currentThread().interrupt();
       abortCause = e;
@@ -522,7 +525,7 @@ public class Http2SolrClient extends SolrClient {
         throw (SolrServerException) cause;
       } else if (cause instanceof IOException) {
         throw new SolrServerException(
-            "IOException occurred when talking to server at: " + getBaseURL(), cause);
+            "IOException occurred when talking to server at: " + req.getURI(), cause);
       }
       throw new SolrServerException(cause.getMessage(), cause);
     } catch (SolrServerException | RuntimeException sse) {
@@ -536,7 +539,11 @@ public class Http2SolrClient extends SolrClient {
   }
 
   private NamedList<Object> processErrorsAndResponse(
-      SolrRequest<?> solrRequest, ResponseParser parser, Response response, InputStream is)
+      SolrRequest<?> solrRequest,
+      ResponseParser parser,
+      Response response,
+      InputStream is,
+      String urlExceptionMessage)
       throws SolrServerException {
     String contentType = response.getHeaders().get(HttpHeader.CONTENT_TYPE);
     String mimeType = null;
@@ -546,7 +553,7 @@ public class Http2SolrClient extends SolrClient {
       encoding = MimeTypes.getCharsetFromContentType(contentType);
     }
     return processErrorsAndResponse(
-        response, parser, is, mimeType, encoding, isV2ApiRequest(solrRequest));
+        response, parser, is, mimeType, encoding, isV2ApiRequest(solrRequest), urlExceptionMessage);
   }
 
   private void setBasicAuthHeader(SolrRequest<?> solrRequest, Request req) {
@@ -778,7 +785,8 @@ public class Http2SolrClient extends SolrClient {
       InputStream is,
       String mimeType,
       String encoding,
-      final boolean isV2Api)
+      final boolean isV2Api,
+      String urlExceptionMessage)
       throws SolrServerException {
     boolean shouldClose = true;
     try {
@@ -794,13 +802,13 @@ public class Http2SolrClient extends SolrClient {
         case HttpStatus.MOVED_TEMPORARILY_302:
           if (!httpClient.isFollowRedirects()) {
             throw new SolrServerException(
-                "Server at " + getBaseURL() + " sent back a redirect (" + httpStatus + ").");
+                "Server at " + urlExceptionMessage + " sent back a redirect (" + httpStatus + ").");
           }
           break;
         default:
           if (processor == null || mimeType == null) {
             throw new RemoteSolrException(
-                serverBaseUrl,
+                urlExceptionMessage,
                 httpStatus,
                 "non ok status: " + httpStatus + ", message:" + response.getReason(),
                 null);
@@ -832,10 +840,10 @@ public class Http2SolrClient extends SolrClient {
             ByteArrayOutputStream body = new ByteArrayOutputStream();
             is.transferTo(body);
             throw new RemoteSolrException(
-                serverBaseUrl, httpStatus, prefix + body.toString(exceptionEncoding), null);
+                urlExceptionMessage, httpStatus, prefix + body.toString(exceptionEncoding), null);
           } catch (IOException e) {
             throw new RemoteSolrException(
-                serverBaseUrl,
+                urlExceptionMessage,
                 httpStatus,
                 "Could not parse response with encoding " + exceptionEncoding,
                 e);
@@ -847,14 +855,14 @@ public class Http2SolrClient extends SolrClient {
       try {
         rsp = processor.processResponse(is, encoding);
       } catch (Exception e) {
-        throw new RemoteSolrException(serverBaseUrl, httpStatus, e.getMessage(), e);
+        throw new RemoteSolrException(urlExceptionMessage, httpStatus, e.getMessage(), e);
       }
 
       Object error = rsp == null ? null : rsp.get("error");
       if (error != null
           && (String.valueOf(getObjectByPath(error, true, errPath))
               .endsWith("ExceptionWithErrObject"))) {
-        throw RemoteExecutionException.create(serverBaseUrl, rsp);
+        throw RemoteExecutionException.create(urlExceptionMessage, rsp);
       }
       if (httpStatus != HttpStatus.OK_200 && !isV2Api) {
         NamedList<String> metadata = null;
@@ -892,7 +900,8 @@ public class Http2SolrClient extends SolrClient {
               .append(response.getRequest().getMethod());
           reason = java.net.URLDecoder.decode(msg.toString(), FALLBACK_CHARSET);
         }
-        RemoteSolrException rss = new RemoteSolrException(serverBaseUrl, httpStatus, reason, null);
+        RemoteSolrException rss =
+            new RemoteSolrException(urlExceptionMessage, httpStatus, reason, null);
         if (metadata != null) rss.setMetadata(metadata);
         throw rss;
       }
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
index 733650aca56..e706f25908d 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
@@ -41,6 +41,7 @@ import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.client.solrj.request.SolrPing;
 import org.apache.solr.client.solrj.request.UpdateRequest;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
@@ -267,6 +268,37 @@ public class Http2SolrClientTest extends SolrJettyTestBase {
     }
   }
 
+  /**
+   * test that SolrExceptions thrown by HttpSolrClient can correctly encapsulate http status codes
+   * even when not on the list of ErrorCodes solr may return.
+   */
+  @Test
+  public void testSolrExceptionWithNullBaseurl() throws IOException, SolrServerException {
+    final int status = 527;
+    assertEquals(
+        status
+            + " didn't generate an UNKNOWN error code, someone modified the list of valid ErrorCode's w/o changing this test to work a different way",
+        SolrException.ErrorCode.UNKNOWN,
+        SolrException.ErrorCode.getErrorCode(status));
+
+    try (Http2SolrClient client = getHttp2SolrClient(null)) {
+      DebugServlet.setErrorCode(status);
+      try {
+        // if client base url is null, request url will be used in exception message
+        SolrPing ping = new SolrPing();
+        ping.setBasePath(jetty.getBaseUrl().toString() + "/debug/foo");
+        client.request(ping);
+
+        fail("Didn't get excepted exception from oversided request");
+      } catch (SolrException e) {
+        assertEquals("Unexpected exception status code", status, e.code());
+        assertTrue(e.getMessage().contains(jetty.getBaseUrl().toString()));
+      }
+    } finally {
+      DebugServlet.clear();
+    }
+  }
+
   @Test
   public void testQuery() throws Exception {
     DebugServlet.clear();