HttpSolrClient: improve error when text/html #4526
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,11 +23,11 @@ | |
| import java.lang.invoke.MethodHandles; | ||
| import java.lang.reflect.Constructor; | ||
| import java.net.MalformedURLException; | ||
| 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<Object> processErrorsAndResponse( | |
| String responseMethod, | ||
| final ResponseParser processor, | ||
| InputStream is, | ||
| 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<Object> processErrorsAndResponse( | |
| } | ||
| break; | ||
| default: | ||
| if (mimeType == null || mimeType.equals("text/html")) { | ||
|
Comment on lines
-212
to
+213
Contributor
Author
There was a problem hiding this comment. the meat of the change. |
||
| throw new RemoteSolrException( | ||
| urlExceptionMessage, | ||
| httpStatus, | ||
|
|
@@ -218,15 +219,15 @@ protected NamedList<Object> processErrorsAndResponse( | |
| } | ||
| } | ||
|
|
||
| checkContentType(processor, is, mimeType, encoding, httpStatus, urlExceptionMessage); | ||
|
Contributor
Author
There was a problem hiding this comment. moved this above wantStream / InputStreamResponseParser because I think it makes sense to check this fairly early. Someone could subclass ISRP and specify the content types that should be checked. |
||
|
|
||
| if (wantStream(processor)) { | ||
| // Only case where stream should not be closed | ||
| shouldClose = false; | ||
| // no processor specified, return raw stream | ||
| return InputStreamResponseParser.createInputStreamNamedList(httpStatus, is); | ||
| } | ||
|
|
||
| NamedList<Object> rsp; | ||
| try { | ||
| rsp = processor.processResponse(is, encoding); | ||
|
|
@@ -240,14 +241,11 @@ protected NamedList<Object> processErrorsAndResponse( | |
| } | ||
| if (httpStatus != 200 && !isV2Api) { | ||
| if (error == null) { | ||
| throw new RemoteSolrException( | ||
| urlExceptionMessage, | ||
| httpStatus, | ||
| "non ok status: " + httpStatus + ", message:" + responseReason, | ||
| null); | ||
|
Comment on lines
-243
to
+248
Contributor
Author
There was a problem hiding this comment. @HoustonPutman , you added this in SOLR-17998 #1657 I didn't see much sense in this particular string so I changed it to match the message string above line 217 |
||
| } else { | ||
| throw new RemoteSolrException(urlExceptionMessage, httpStatus, error); | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claude's root cause, and suggested this fix:
When 350 concurrent queries hit the rate limiter and get rejected as 429s, the server sends rapid RST_STREAM frames on the multiplexed HTTP/2 connection. This triggers HTTP/2's ENHANCE_YOUR_CALM protection, which tears down the entire connection. Subsequent requests then fail with IOException, which the CloudSolrClient sees as a communication error → "No live SolrServers."
The old Apache HTTP client used HTTP/1.1 (separate connections per request), so this never happened. The Jetty client defaults to HTTP/2.