From 94477033d559c7c60a763d1584ead645ea31d69c Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 14 Jun 2026 00:32:32 -0400 Subject: [PATCH 1/3] HttpSolrClient: improve error when text/html If most non-200 codes happen, and the Content-Type is text/html, we would throw an error relating to the content type being unexpected. We now don't mention the content type, and we simply throw a simpler exception. --- .../cloud/TestAuthenticationFramework.java | 13 ++++------ .../security/BasicAuthIntegrationTest.java | 2 +- .../solr/servlet/TestRequestRateLimiter.java | 17 ++++++++++--- .../client/solrj/impl/HttpSolrClient.java | 24 +++++++++---------- 4 files changed, 30 insertions(+), 26 deletions(-) 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..3a588e49e0d2 100644 --- a/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java +++ b/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java @@ -44,6 +44,9 @@ 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 +59,7 @@ import org.junit.Test; import org.mockito.Mockito; -@LuceneTestCase.AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-17810") +//@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 +72,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 +292,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 +432,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); } From ede7270abf0a39a57557c2c187484cdb0a042ec1 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 14 Jun 2026 00:58:30 -0400 Subject: [PATCH 2/3] changelog --- .../PR#4526-HttpSolrClient-error-message.yml | 33 +++++++++++++++++++ .../solr/servlet/TestRequestRateLimiter.java | 2 -- 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/PR#4526-HttpSolrClient-error-message.yml 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..2daae520a6d6 --- /dev/null +++ b/changelog/unreleased/PR#4526-HttpSolrClient-error-message.yml @@ -0,0 +1,33 @@ +# (DELETE ALL COMMENTS UP HERE AFTER FILLING THIS IN + +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc + +# If the change is minor, don't bother adding a changelog entry. +# For `other` type entries, the threshold to bother with a changelog entry should be even higher. + +# title: +# * The audience is end-users and administrators, not committers. +# * Be short and focused on the user impact. Multiple sentences is fine! +# * For technical/geeky details, prefer the commit message instead of changelog. +# * Reference JIRA issues like `SOLR-12345`, or if no JIRA but have a GitHub PR then `PR#12345`. + +# type: +# `added` for new features/improvements, opt-in by the user typically documented in the ref guide +# `changed` for improvements; not opt-in +# `fixed` for improvements that are deemed to have fixed buggy behavior +# `deprecated` for marking things deprecated +# `removed` for code removed +# `dependency_update` for updates to dependencies +# `other` for anything else, like large/significant refactorings, build changes, +# test infrastructure, or documentation. +# Most such changes are too small/minor to bother with a changelog entry. + +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/servlet/TestRequestRateLimiter.java b/solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java index 3a588e49e0d2..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,7 +38,6 @@ 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; @@ -59,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"; From 17a25b1d8f9394765cfed7bff6f0390fd7085e96 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 18 Jun 2026 11:27:30 -0400 Subject: [PATCH 3/3] fix changelog comment --- .../PR#4526-HttpSolrClient-error-message.yml | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/changelog/unreleased/PR#4526-HttpSolrClient-error-message.yml b/changelog/unreleased/PR#4526-HttpSolrClient-error-message.yml index 2daae520a6d6..ecf4bfc9a6dc 100644 --- a/changelog/unreleased/PR#4526-HttpSolrClient-error-message.yml +++ b/changelog/unreleased/PR#4526-HttpSolrClient-error-message.yml @@ -1,27 +1,3 @@ -# (DELETE ALL COMMENTS UP HERE AFTER FILLING THIS IN - -# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc - -# If the change is minor, don't bother adding a changelog entry. -# For `other` type entries, the threshold to bother with a changelog entry should be even higher. - -# title: -# * The audience is end-users and administrators, not committers. -# * Be short and focused on the user impact. Multiple sentences is fine! -# * For technical/geeky details, prefer the commit message instead of changelog. -# * Reference JIRA issues like `SOLR-12345`, or if no JIRA but have a GitHub PR then `PR#12345`. - -# type: -# `added` for new features/improvements, opt-in by the user typically documented in the ref guide -# `changed` for improvements; not opt-in -# `fixed` for improvements that are deemed to have fixed buggy behavior -# `deprecated` for marking things deprecated -# `removed` for code removed -# `dependency_update` for updates to dependencies -# `other` for anything else, like large/significant refactorings, build changes, -# test infrastructure, or documentation. -# Most such changes are too small/minor to bother with a changelog entry. - title: > HttpSolrClient: made error messages more consistent, avoiding complaining about the content-type when it's text/html