This is an automated email from the ASF dual-hosted git repository.
dsmiley 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 10a2cb044e0 SOLR-17316: fix ApiTool and PackageUtils to fail on
non-2xx responses (#4914)
10a2cb044e0 is described below
commit 10a2cb044e0320f0b63be7917f2742f5fff5e361
Author: Serhiy Bzhezytskyy <[email protected]>
AuthorDate: Thu Sep 17 03:12:10 2026 +0300
SOLR-17316: fix ApiTool and PackageUtils to fail on non-2xx responses
(#4914)
`bin/solr api` and the package manager's collection-agnostic API calls now
fail on a non-2xx response instead of printing it as if it had succeeded.
InputStreamResponseParser now incorporates a "reason" with the status. A
checkStatus() method helps callers handle errors.
Co-authored-by: Claude Sonnet 5 <[email protected]>
---
.../SOLR-17316-input-stream-status-check.yml | 10 +++++
.../core/src/java/org/apache/solr/cli/ApiTool.java | 5 ++-
.../solr/cloud/api/collections/SplitShardCmd.java | 1 +
.../apache/solr/packagemanager/PackageUtils.java | 5 ++-
.../client/solrj/io/stream/JSONTupleStream.java | 1 +
.../solr/client/solrj/impl/NodeValueFetcher.java | 1 +
.../solr/client/solrj/impl/HttpSolrClient.java | 2 +-
.../solrj/response/InputStreamResponseParser.java | 45 ++++++++++++++++++++++
.../response/InputStreamResponseParserTest.java | 32 +++++++++++++++
9 files changed, 99 insertions(+), 3 deletions(-)
diff --git a/changelog/unreleased/SOLR-17316-input-stream-status-check.yml
b/changelog/unreleased/SOLR-17316-input-stream-status-check.yml
new file mode 100644
index 00000000000..245d6342cf8
--- /dev/null
+++ b/changelog/unreleased/SOLR-17316-input-stream-status-check.yml
@@ -0,0 +1,10 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: >
+ `bin/solr api` and the package manager's collection-agnostic API calls now
fail on a non-2xx
+ response instead of printing it as if it had succeeded.
+type: fixed
+authors:
+ - name: Serhiy Bzhezytskyy
+links:
+ - name: SOLR-17316
+ url: https://issues.apache.org/jira/browse/SOLR-17316
diff --git a/solr/core/src/java/org/apache/solr/cli/ApiTool.java
b/solr/core/src/java/org/apache/solr/cli/ApiTool.java
index a86bf1f545d..51b883d507d 100644
--- a/solr/core/src/java/org/apache/solr/cli/ApiTool.java
+++ b/solr/core/src/java/org/apache/solr/cli/ApiTool.java
@@ -95,7 +95,10 @@ public class ApiTool extends ToolBase {
// Pass the server's JSON to the user as it came; parsing and
re-serialising it here only
// risks changing it.
req.setResponseParser(new InputStreamResponseParser("json"));
- return
InputStreamResponseParser.consumeResponseToString(solrClient.request(req));
+ var response = solrClient.request(req);
+ String body =
InputStreamResponseParser.consumeResponseToString(response);
+ InputStreamResponseParser.checkHttpStatus(response, url + ": " + body);
+ return body;
}
}
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
index 6740c5ba046..4787e81734c 100644
---
a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
+++
b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
@@ -883,6 +883,7 @@ public class SplitShardCmd implements
CollApiCmds.CollectionApiCommand {
NamedList<Object> resp =
httpClient.requestWithBaseUrl(parentShardLeader.getBaseUrl(), req,
null);
+ InputStreamResponseParser.checkHttpStatus(resp);
var indexSizeRef = new AtomicReference<Double>(-1.0);
var freeSizeRef = new AtomicReference<Double>(-1.0);
diff --git
a/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
b/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
index 804d636b4d5..c97bdab3123 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
@@ -168,7 +168,10 @@ public class PackageUtils {
new GenericSolrRequest(SolrRequest.METHOD.GET, path, params)
.setRequiresCollection(isCollectionApi);
request.setResponseParser(new InputStreamResponseParser("json"));
- return
InputStreamResponseParser.consumeResponseToString(client.request(request));
+ var response = client.request(request);
+ String body =
InputStreamResponseParser.consumeResponseToString(response);
+ InputStreamResponseParser.checkHttpStatus(response, path + ": " + body);
+ return body;
} catch (IOException | SolrServerException e) {
throw new RuntimeException(e);
}
diff --git
a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/JSONTupleStream.java
b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/JSONTupleStream.java
index 98f02cd274c..657d82e292d 100644
---
a/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/JSONTupleStream.java
+++
b/solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/JSONTupleStream.java
@@ -55,6 +55,7 @@ public class JSONTupleStream implements TupleStreamParser {
QueryRequest query = new QueryRequest(requestParams,
SolrRequest.METHOD.POST);
query.setResponseParser(new InputStreamResponseParser("json"));
NamedList<Object> genericResponse = server.request(query);
+ InputStreamResponseParser.checkHttpStatus(genericResponse);
InputStream stream = (InputStream)
genericResponse.get(InputStreamResponseParser.STREAM_KEY);
InputStreamReader reader = new InputStreamReader(stream,
StandardCharsets.UTF_8);
return new JSONTupleStream(reader);
diff --git
a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java
b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java
index 917eb59ceea..6a031c1bed1 100644
---
a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java
+++
b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java
@@ -181,6 +181,7 @@ public class NodeValueFetcher {
String baseUrl =
ctx.zkClientClusterStateProvider.getZkStateReader().getBaseUrlForNodeName(ctx.getNode());
NamedList<Object> response =
ctx.httpSolrClient().requestWithBaseUrl(baseUrl, req, null);
+ InputStreamResponseParser.checkHttpStatus(response);
// TODO come up with a better solution to stream this response instead
of loading in memory
try (InputStream prometheusStream = (InputStream)
response.get(STREAM_KEY)) {
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 35291fb6cce..55623337965 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
@@ -237,7 +237,7 @@ public abstract class HttpSolrClient extends SolrClient {
// Only case where stream should not be closed
shouldClose = false;
// no processor specified, return raw stream
- return
InputStreamResponseParser.createInputStreamNamedList(httpStatus, is);
+ return
InputStreamResponseParser.createInputStreamNamedList(httpStatus,
responseReason, is);
}
NamedList<Object> rsp;
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/InputStreamResponseParser.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/InputStreamResponseParser.java
index 77466ef1c26..6f35a319bcc 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/response/InputStreamResponseParser.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/response/InputStreamResponseParser.java
@@ -20,6 +20,7 @@ import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
+import java.util.Locale;
import java.util.Set;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.SimpleOrderedMap;
@@ -33,6 +34,7 @@ public class InputStreamResponseParser extends ResponseParser
{
public static String STREAM_KEY = "stream";
public static String HTTP_STATUS_KEY = "responseStatus";
+ public static String HTTP_REASON_KEY = "responseReason";
private final String writerType;
@@ -56,6 +58,36 @@ public class InputStreamResponseParser extends
ResponseParser {
return output;
}
+ /**
+ * Throws if the response's HTTP status was not 2xx.
+ *
+ * <p>{@code SolrClient}s skip their usual non-2xx check when an {@link
+ * InputStreamResponseParser} is in use, since the raw stream is handed back
regardless of
+ * status. Callers that read the stream under {@link #STREAM_KEY} directly
-- rather than via
+ * {@link #consumeResponseToString}, which does not check either -- should
call this first.
+ */
+ public static void checkHttpStatus(NamedList<Object> response) throws
IOException {
+ checkHttpStatus(response, null);
+ }
+
+ /**
+ * As {@link #checkHttpStatus(NamedList)}, appending {@code detail} to the
exception message
+ * when the status is not 2xx -- e.g. the request URL, or a body already
consumed for another
+ * purpose.
+ */
+ public static void checkHttpStatus(NamedList<Object> response, String detail)
+ throws IOException {
+ Object status = response.get(HTTP_STATUS_KEY);
+ if (status instanceof Integer httpStatus && (httpStatus < 200 ||
httpStatus >= 300)) {
+ Object reason = response.get(HTTP_REASON_KEY);
+ String msg =
+ reason instanceof String r && !r.isEmpty()
+ ? String.format(Locale.ROOT, "Unexpected HTTP status [%d %s] in
response", httpStatus, r)
+ : String.format(Locale.ROOT, "Unexpected HTTP status [%d] in
response", httpStatus);
+ throw new IOException(detail == null ? msg : msg + ": " + detail);
+ }
+ }
+
@Override
public String getWriterType() {
return writerType;
@@ -73,9 +105,22 @@ public class InputStreamResponseParser extends
ResponseParser {
public static NamedList<Object> createInputStreamNamedList(
int httpStatus, InputStream inputStream) {
+ return createInputStreamNamedList(httpStatus, null, inputStream);
+ }
+
+ /**
+ * As {@link #createInputStreamNamedList(int, InputStream)}, also recording
the HTTP reason
+ * phrase (e.g. "Bad Request") under {@link #HTTP_REASON_KEY}, when known,
so callers building
+ * an error message have more to go on than the bare status code.
+ */
+ public static NamedList<Object> createInputStreamNamedList(
+ int httpStatus, String reason, InputStream inputStream) {
final var nl = new SimpleOrderedMap<>();
nl.add(STREAM_KEY, inputStream);
nl.add(HTTP_STATUS_KEY, httpStatus);
+ if (reason != null) {
+ nl.add(HTTP_REASON_KEY, reason);
+ }
return nl;
}
}
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/response/InputStreamResponseParserTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/response/InputStreamResponseParserTest.java
index 68eaaf8f861..18a7bb49495 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/response/InputStreamResponseParserTest.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/response/InputStreamResponseParserTest.java
@@ -96,6 +96,38 @@ public class InputStreamResponseParserTest extends
SolrTestCaseJ4 {
assertEquals("1234", String.valueOf(solrDocument.getFieldValue("id")));
}
+ @Test
+ public void testCheckHttpStatusThrowsOnNon2xx() throws Exception {
+ try (InputStream is = getResponse()) {
+ NamedList<Object> response =
InputStreamResponseParser.createInputStreamNamedList(500, is);
+ IOException e =
+ assertThrows(
+ IOException.class, () ->
InputStreamResponseParser.checkHttpStatus(response));
+ assertTrue(e.getMessage().contains("500"));
+ }
+ }
+
+ @Test
+ public void testCheckHttpStatusIncludesReasonWhenPresent() throws Exception {
+ try (InputStream is = getResponse()) {
+ NamedList<Object> response =
+ InputStreamResponseParser.createInputStreamNamedList(400, "Bad
Request", is);
+ IOException e =
+ assertThrows(
+ IOException.class, () ->
InputStreamResponseParser.checkHttpStatus(response));
+ assertTrue(e.getMessage().contains("400"));
+ assertTrue(e.getMessage().contains("Bad Request"));
+ }
+ }
+
+ @Test
+ public void testCheckHttpStatusAllows2xx() throws Exception {
+ try (InputStream is = getResponse()) {
+ NamedList<Object> response =
InputStreamResponseParser.createInputStreamNamedList(200, is);
+ InputStreamResponseParser.checkHttpStatus(response); // should not throw
+ }
+ }
+
/** Parse response from java.io.InputStream. */
@Test
public void testInputStreamResponse() throws Exception {
