HttpSolrClient: improve error when text/html#4526
Conversation
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.
| 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") |
| return new CloudJettySolrClient.Builder( | ||
| new ZkClientClusterStateProvider(cluster.getZkServer().getZkAddress())) | ||
| .withDefaultCollection(collection) | ||
| .withHttpClientBuilder(new HttpJettySolrClient.Builder().useHttp1_1(true)) |
There was a problem hiding this comment.
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.
| } | ||
| } | ||
|
|
||
| checkContentType(processor, is, mimeType, encoding, httpStatus, urlExceptionMessage); |
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 (processor == null || mimeType == null) { | ||
| if (mimeType == null || mimeType.equals("text/html")) { |
There was a problem hiding this comment.
the meat of the change.
| 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); |
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
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.
Not sure if worth a JIRA but I'll add a changelog