Skip to content

fix(druid): sniff HTML error body before JSON parse (second leg) #20152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shoemoney wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(druid): sniff HTML error body before JSON parse (second leg) #20152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17b5863
fix(druid): sniff HTML error body before JSON parse (second leg)
shoemoney Aug 25, 2026
d1f40ca
fix(druid): propagate handler exceptions through the future, restrict…
shoemoney Aug 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion processing/src/main/java/org/apache/druid/java/util/http/client/NettyHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,12 @@ public void abort()
catch (Exception ex) {
log.warn(ex, "[%s] Exception thrown while processing message, closing channel.", requestDesc);

// Complete the future with the exception itself rather than null: a handler (e.g. handleResponse)
// may throw a specific, meaningful exception (query capacity exceeded, interrupted, etc.) and
// completing with null discards it, leaving callers with a successful-looking null result instead
// of the real failure.
if (!retVal.isDone()) {
retVal.set(null);
retVal.setException(ex);
}
channel.close();
channelResourceContainer.returnResource();
Expand Down
65 changes: 65 additions & 0 deletions server/src/main/java/org/apache/druid/client/DirectDruidClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.apache.druid.java.util.http.client.response.StatusResponseHolder;
import org.apache.druid.query.Queries;
import org.apache.druid.query.Query;
import org.apache.druid.query.QueryCapacityExceededException;
import org.apache.druid.query.QueryContext;
import org.apache.druid.query.QueryMetrics;
import org.apache.druid.query.QueryPlus;
Expand Down Expand Up @@ -243,6 +244,70 @@ 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;
}
}
// A 503 is only treated as capacity-exceeded when the body is confirmed HTML/non-JSON; a 503 carrying a
// proper JSON error body falls through to the normal JSON error handling below.
if (statusCode == 429 || (statusCode == 503 && (isHtmlContentType || isHtmlBody))) {
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);

@FrankChen021 FrankChen021 Aug 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

}
if (isHtmlContentType || isHtmlBody) {
int len = Math.min(contentBuffer.readableBytes(), 512);
byte[] previewBytes = new byte[len];
if (len > 0) {
contentBuffer.getBytes(contentBuffer.readerIndex(), previewBytes);
}
String preview = len > 0 ? StringUtils.fromUtf8(previewBytes) : "";
preview = preview.substring(0, Math.min(preview.length(), 256));
throw new org.apache.druid.query.QueryInterruptedException(
org.apache.druid.query.QueryException.UNKNOWN_EXCEPTION_ERROR_CODE,
StringUtils.format(
"Query[%s] url[%s] returned HTML response instead of JSON with status[%s] contentType[%s] preview[%s]",
query.getId(),
url,
statusCode,
contentType,
preview
),
org.apache.druid.query.QueryInterruptedException.class.getName(),
host
);
}
checkTotalBytesLimit(response.getContent().readableBytes());

log.debug("Initial response from url[%s] for queryId[%s]", url, query.getId());
Expand Down