FrankChen021 commented on code in PR #20152:
URL: https://github.com/apache/druid/pull/20152#discussion_r3862683821
##########
server/src/main/java/org/apache/druid/client/DirectDruidClient.java:
##########
@@ -243,6 +244,68 @@ public ClientResponse<InputStream>
handleResponse(HttpResponse response, Traffic
{
trafficCopRef.set(trafficCop);
checkQueryTimeout();
+ // Handle 429/503 HTML before JSON parse to avoid JsonParseException
0x3c ('<')
+ final int statusCode = response.getStatus().getCode();
+ final String contentType =
response.headers().get(HttpHeaders.Names.CONTENT_TYPE);
+ final ChannelBuffer contentBuffer = response.getContent();
+ boolean isHtmlContentType = contentType != null &&
StringUtils.toLowerCase(contentType).contains("text/html");
+ boolean isHtmlBody = false;
+ if (contentBuffer.readableBytes() > 0) {
+ int readerIndex = contentBuffer.readerIndex();
+ int readable = contentBuffer.readableBytes();
+ for (int i = 0; i < readable; i++) {
+ byte b = contentBuffer.getByte(readerIndex + i);
+ if (b == ' ' || b == '\n' || b == '\r' || b == '\t') {
+ continue;
+ }
+ if (b == '<') {
+ isHtmlBody = true;
+ } else if (b != '{' && b != '[') {
+ // Not JSON start, but only treat '<' as HTML indicator
+ }
+ break;
+ }
+ }
+ if (statusCode == 429 || statusCode == 503) {
+ String msg = StringUtils.format(
+ "Query[%s] url[%s] failed with status[%s] [%s]",
+ query.getId(),
+ url,
+ statusCode,
+ response.getStatus().getReasonPhrase()
+ );
+ if (contentBuffer.readableBytes() > 0) {
+ int len = Math.min(contentBuffer.readableBytes(), 512);
+ byte[] previewBytes = new byte[len];
+ contentBuffer.getBytes(contentBuffer.readerIndex(),
previewBytes);
+ String preview = StringUtils.fromUtf8(previewBytes);
+ preview = preview.substring(0, Math.min(preview.length(), 256));
+ msg = StringUtils.format("%s: %s", msg, preview);
+ }
+ throw
QueryCapacityExceededException.withErrorMessageAndResolvedHost(msg);
Review Comment:
[P1] Handler exceptions complete the Netty future with null
With production NettyHttpClient, this throw occurs before the handler
response is assigned. Netty then completes the future successfully with null
and closes the channel, so JsonParserIterator treats the result as a possible
scatter-gather limit and raises ResourceLimitExceededException. The intended
capacity/interruption error is therefore lost. Propagate the exception through
the future or return an error-bearing response, and test with the real Netty
client.
##########
server/src/main/java/org/apache/druid/client/DirectDruidClient.java:
##########
@@ -243,6 +244,68 @@ public ClientResponse<InputStream>
handleResponse(HttpResponse response, Traffic
{
trafficCopRef.set(trafficCop);
checkQueryTimeout();
+ // Handle 429/503 HTML before JSON parse to avoid JsonParseException
0x3c ('<')
+ final int statusCode = response.getStatus().getCode();
+ final String contentType =
response.headers().get(HttpHeaders.Names.CONTENT_TYPE);
+ final ChannelBuffer contentBuffer = response.getContent();
+ boolean isHtmlContentType = contentType != null &&
StringUtils.toLowerCase(contentType).contains("text/html");
+ boolean isHtmlBody = false;
+ if (contentBuffer.readableBytes() > 0) {
+ int readerIndex = contentBuffer.readerIndex();
+ int readable = contentBuffer.readableBytes();
+ for (int i = 0; i < readable; i++) {
+ byte b = contentBuffer.getByte(readerIndex + i);
+ if (b == ' ' || b == '\n' || b == '\r' || b == '\t') {
+ continue;
+ }
+ if (b == '<') {
+ isHtmlBody = true;
+ } else if (b != '{' && b != '[') {
+ // Not JSON start, but only treat '<' as HTML indicator
+ }
+ break;
+ }
+ }
+ if (statusCode == 429 || statusCode == 503) {
Review Comment:
[P2] All 503 responses are mapped to capacity errors
This branch depends only on the status code, so a JSON 503
service-unavailable response is replaced with QueryCapacityExceededException,
which maps to HTTP 429 and discards structured error details. Restrict 503
conversion to confirmed HTML/non-JSON responses or preserve the existing JSON
error path; add 503 JSON and HTML coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
