FrankChen021 commented on code in PR #20151:
URL: https://github.com/apache/druid/pull/20151#discussion_r3996192322
##########
server/src/main/java/org/apache/druid/client/DirectDruidClient.java:
##########
@@ -237,12 +274,139 @@ private InputStream dequeue() throws InterruptedException
return holder.getStream();
}
+ /**
+ * Scans past leading whitespace in {@code buffer} looking for the
first content byte, without consuming
+ * (advancing the reader index of) the buffer, and returns it. Once a
non-whitespace byte is found, the prefix
+ * is considered resolved (see {@link #bodyPrefixResolved}) and later
calls return null. If {@code buffer} is
+ * empty or entirely whitespace, the prefix remains unresolved (and
null is returned) so a later call, from a
+ * subsequent chunk, can retry the check; this matters for chunked
responses, where the initial
+ * {@link HttpResponse} can carry an empty body and the real content,
HTML or otherwise, only arrives via
+ * {@link #handleChunk}.
+ */
+ @Nullable
+ private Byte bodyPrefixByte(ByteBuf buffer)
+ {
+ if (bodyPrefixResolved.get()) {
+ return null;
+ }
+ final int readerIndex = buffer.readerIndex();
+ final int readable = buffer.readableBytes();
+ for (int i = 0; i < readable; i++) {
+ byte b = buffer.getByte(readerIndex + i);
+ if (b == ' ' || b == '\n' || b == '\r' || b == '\t') {
+ continue;
+ }
+ bodyPrefixResolved.set(true);
+ return b;
+ }
+ return null;
+ }
+
+ /**
+ * Classifies the body prefix in {@code buffer} (see {@link
#bodyPrefixByte}) and fails the query if it is not
+ * JSON (or Smile, when the request was sent as Smile per {@link
#isSmile}). HTML always fails; any other
+ * non-JSON/non-Smile body fails only when the status is 429/503,
since Druid itself never sends such a body
+ * with those statuses but proxies routinely do (an HTML error page
from nginx or a load balancer, a
+ * plain-text "upstream connect error" from Envoy). A structured body
in the request's own format, whatever
+ * the status, is left alone so that the normal parse path can surface
the server's own structured error.
+ *
+ * @param contentType Content-Type header of the initial response,
possibly null; a text/html value fails the
+ * query regardless of the body prefix
+ * @param chunkNum 0 for the initial response body, else the chunk
number
+ */
+ private void failIfNonJsonBody(String contentType, ByteBuf buffer,
long chunkNum)
+ {
+ final boolean isHtmlContentType =
+ contentType != null &&
StringUtils.toLowerCase(contentType).contains("text/html");
+ final Byte prefix = bodyPrefixByte(buffer);
+ final boolean isHtml = isHtmlContentType || (prefix != null &&
prefix == '<');
+ // A data server negotiates its response format from the request
(ResourceIOReaderWriterFactory#factorize),
+ // so a Smile request gets a Smile response, error bodies included;
those begin with the Smile format
+ // header's 0x3a byte rather than JSON's '{'/'['. Checking only
'{'/'[' here would misclassify every
+ // structured Smile 429/503 body as non-JSON and discard the
server's real error.
+ final boolean isNonJson = isSmile
+ ? prefix != null && prefix !=
SmileConstants.HEADER_BYTE_1
+ : prefix != null && prefix != '{' &&
prefix != '[';
+ final int statusCode = responseStatusCode;
+ if (isHtml || (isNonJson && (statusCode == 429 || statusCode ==
503))) {
Review Comment:
[P2] Classify empty 429/503 bodies as capacity errors
When a proxy returns 429/503 with an empty or whitespace-only body,
`bodyPrefixByte` returns null and this condition never throws. Netty also skips
`handleChunk` for an empty `LastHttpContent`, so `done()` completes a stream
that `JsonParserIterator` later reports as a generic EOF/parse error instead of
`QueryCapacityExceededException`. Please finalize the unresolved prefix when
the response ends (or otherwise treat an unresolved 429/503 body as non-JSON)
while preserving the existing behavior for successful empty responses.
--
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]
