diff --git a/changelog/unreleased/PR#4526-HttpSolrClient-error-message.yml b/changelog/unreleased/PR#4526-HttpSolrClient-error-message.yml new file mode 100644 index 000000000000..ecf4bfc9a6dc --- /dev/null +++ b/changelog/unreleased/PR#4526-HttpSolrClient-error-message.yml @@ -0,0 +1,9 @@ +title: > + HttpSolrClient: made error messages more consistent, avoiding complaining about the content-type + when it's text/html +type: changed +authors: + - name: David Smiley +links: + - name: PR#4526 + url: https://github.com/apache/solr/pull/4526 diff --git a/solr/core/src/test/org/apache/solr/cloud/TestAuthenticationFramework.java b/solr/core/src/test/org/apache/solr/cloud/TestAuthenticationFramework.java index 130e288ee960..18060fcd016a 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestAuthenticationFramework.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestAuthenticationFramework.java @@ -21,7 +21,7 @@ import jakarta.servlet.http.HttpServletResponse; import java.lang.invoke.MethodHandles; import java.util.Map; -import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.RemoteSolrException; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.SolrQuery; @@ -43,9 +43,6 @@ public class TestAuthenticationFramework extends SolrCloudTestCase { private static final String configName = "solrCloudCollectionConfig"; private static final String collectionName = "testcollection"; - static String requestUsername = MockAuthenticationPlugin.expectedUsername; - static String requestPassword = MockAuthenticationPlugin.expectedPassword; - @Override public void setUp() throws Exception { setupAuthenticationPlugin(); @@ -70,11 +67,9 @@ public void testBasics() throws Exception { // Should fail with 401 try { - SolrServerException e = - expectThrows(SolrServerException.class, this::collectionCreateSearchDeleteTwice); - assertTrue( - "Should've returned a 401 error", - e.getMessage().contains("401") || e.getMessage().contains("Authentication")); + RemoteSolrException e = + expectThrows(RemoteSolrException.class, this::collectionCreateSearchDeleteTwice); + assertEquals(401, e.code()); // Authentication failed } finally { MockAuthenticationPlugin.expectedUsername = null; MockAuthenticationPlugin.expectedPassword = null; diff --git a/solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java b/solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java index 166b9f440c61..9684d5447cf7 100644 --- a/solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java @@ -259,7 +259,7 @@ public void testBasicAuth() throws Exception { () -> { new UpdateRequest().deleteByQuery("*:*").process(aNewClient, COLLECTION); }); - assertTrue(e.getMessage(), e.getMessage().contains("Authentication failed")); + assertEquals(401, e.code()); // Authentication failed } finally { aNewClient.close(); cluster.stopJettySolrRunner(aNewJetty); diff --git a/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java b/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java index 87ec473b13a0..0054cd1ad196 100644 --- a/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java +++ b/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java @@ -38,12 +38,14 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.LongAdder; -import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.RemoteSolrException; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider; +import org.apache.solr.client.solrj.jetty.CloudJettySolrClient; +import org.apache.solr.client.solrj.jetty.HttpJettySolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.SolrQuery; import org.apache.solr.client.solrj.response.QueryResponse; @@ -56,7 +58,6 @@ import org.junit.Test; import org.mockito.Mockito; -@LuceneTestCase.AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-17810") public class TestRequestRateLimiter extends SolrCloudTestCase { private static final String FIRST_COLLECTION = "c1"; private static final String SECOND_COLLECTION = "c2"; @@ -69,7 +70,7 @@ public static void setupCluster() throws Exception { @Test public void testConcurrentQueries() throws Exception { - try (CloudSolrClient client = cluster.newSolrClient(FIRST_COLLECTION)) { + try (CloudSolrClient client = newHttp1SolrClient(FIRST_COLLECTION)) { CollectionAdminRequest.createCollection(FIRST_COLLECTION, 1, 1).process(client); cluster.waitForActiveCollection(FIRST_COLLECTION, 1, 1); @@ -289,7 +290,7 @@ private static HttpServletRequest newRequest(String type, String ctx) { @Nightly public void testSlotBorrowing() throws Exception { - try (CloudSolrClient client = cluster.newSolrClient(SECOND_COLLECTION)) { + try (CloudSolrClient client = newHttp1SolrClient(SECOND_COLLECTION)) { CollectionAdminRequest.createCollection(SECOND_COLLECTION, 1, 1).process(client); cluster.waitForActiveCollection(SECOND_COLLECTION, 1, 1); @@ -429,6 +430,14 @@ public SlotReservation allowSlotBorrowing() throws InterruptedException { } } + private CloudSolrClient newHttp1SolrClient(String collection) { + return new CloudJettySolrClient.Builder( + new ZkClientClusterStateProvider(cluster.getZkServer().getZkAddress())) + .withDefaultCollection(collection) + .withHttpClientBuilder(new HttpJettySolrClient.Builder().useHttp1_1(true)) + .build(); + } + private static class MockBuilder extends RateLimitManager.Builder { private final RequestRateLimiter queryRequestRateLimiter; private final RequestRateLimiter indexRequestRateLimiter; diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java index 93ae9e332f15..d1e69255bab4 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java @@ -23,11 +23,11 @@ import java.lang.invoke.MethodHandles; import java.lang.reflect.Constructor; import java.net.MalformedURLException; -import java.net.URLDecoder; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.Collection; +import java.util.Objects; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; @@ -188,11 +188,12 @@ protected NamedList processErrorsAndResponse( String responseMethod, final ResponseParser processor, InputStream is, - String mimeType, + String mimeType, // aka contentType String encoding, final boolean isV2Api, String urlExceptionMessage) throws SolrServerException { + Objects.requireNonNull(processor); boolean shouldClose = true; try { // handle some http level checks before trying to parse the response @@ -209,7 +210,7 @@ protected NamedList processErrorsAndResponse( } break; default: - if (processor == null || mimeType == null) { + if (mimeType == null || mimeType.equals("text/html")) { throw new RemoteSolrException( urlExceptionMessage, httpStatus, @@ -218,6 +219,8 @@ protected NamedList processErrorsAndResponse( } } + checkContentType(processor, is, mimeType, encoding, httpStatus, urlExceptionMessage); + if (wantStream(processor)) { // Only case where stream should not be closed shouldClose = false; @@ -225,8 +228,6 @@ protected NamedList processErrorsAndResponse( return InputStreamResponseParser.createInputStreamNamedList(httpStatus, is); } - checkContentType(processor, is, mimeType, encoding, httpStatus, urlExceptionMessage); - NamedList rsp; try { rsp = processor.processResponse(is, encoding); @@ -240,14 +241,11 @@ protected NamedList processErrorsAndResponse( } if (httpStatus != 200 && !isV2Api) { if (error == null) { - StringBuilder msg = - new StringBuilder() - .append(responseReason) - .append("\n") - .append("request: ") - .append(responseMethod); - String reason = URLDecoder.decode(msg.toString(), FALLBACK_CHARSET); - throw new RemoteSolrException(urlExceptionMessage, httpStatus, reason, null); + throw new RemoteSolrException( + urlExceptionMessage, + httpStatus, + "non ok status: " + httpStatus + ", message:" + responseReason, + null); } else { throw new RemoteSolrException(urlExceptionMessage, httpStatus, error); }