This is an automated email from the ASF dual-hosted git repository.
kfaraz pushed a commit to branch 38.0.0
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/38.0.0 by this push:
new 0bfde5a7f4f fix: ensure native query statusCode is 403 for blocklist
(#19745)
0bfde5a7f4f is described below
commit 0bfde5a7f4ff5fa3454f147bd3cd5b41f9fbd448
Author: jtuglu1 <[email protected]>
AuthorDate: Wed Jul 29 22:32:31 2026 -0700
fix: ensure native query statusCode is 403 for blocklist (#19745)
This makes sure that any DruidExceptions thrown during query processing are
correctly propagated with the correct status code for native queries.
(cherry picked from commit afcee5a075bfc64179488e9ad22ce30b15ca0d85)
---
.../server/EmbeddedBrokerDynamicConfigTest.java | 51 +++
.../exception/ErrorResponseTransformStrategy.java | 18 +-
.../PersonaBasedErrorTransformStrategy.java | 2 +-
.../PersonaBasedErrorTransformStrategyTest.java | 31 +-
.../apache/druid/server/BrokerQueryResource.java | 5 +-
.../org/apache/druid/server/QueryLifecycle.java | 5 +-
.../org/apache/druid/server/QueryResource.java | 56 ++-
.../org/apache/druid/server/QueryResultPusher.java | 7 +-
.../druid/server/initialization/ServerConfig.java | 6 +
.../org/apache/druid/server/QueryResourceTest.java | 504 ++++++++++++++-------
.../org/apache/druid/sql/avatica/ErrorHandler.java | 7 +-
.../org/apache/druid/sql/http/SqlResource.java | 52 ++-
12 files changed, 519 insertions(+), 225 deletions(-)
diff --git
a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/server/EmbeddedBrokerDynamicConfigTest.java
b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/server/EmbeddedBrokerDynamicConfigTest.java
index ac48e2c5346..dab43be7a45 100644
---
a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/server/EmbeddedBrokerDynamicConfigTest.java
+++
b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/server/EmbeddedBrokerDynamicConfigTest.java
@@ -23,6 +23,7 @@ import org.apache.druid.audit.AuditInfo;
import org.apache.druid.common.config.JacksonConfigManager;
import org.apache.druid.common.utils.IdUtils;
import org.apache.druid.indexing.common.task.TaskBuilder;
+import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.query.QueryContext;
import org.apache.druid.server.DefaultQueryBlocklistRule;
import org.apache.druid.server.QueryBlocklistRule;
@@ -43,6 +44,11 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.time.Duration;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -127,6 +133,51 @@ public class EmbeddedBrokerDynamicConfigTest extends
EmbeddedClusterTestBase
Assertions.assertFalse(finalResult.isBlank());
}
+ /**
+ * A blocklisted query must be rejected with 403.
+ */
+ @Test
+ @Timeout(30)
+ public void testBlocklistedQueryReturnsForbiddenOnBothEndpoints() throws
Exception
+ {
+ updateBrokerDynamicConfig(
+ BrokerDynamicConfig.builder()
+ .withQueryBlocklist(List.of(
+ new
DefaultQueryBlocklistRule("block-test-datasource", Set.of(dataSource), null,
null)
+ ))
+ .build()
+ );
+
+ try {
+ final String nativeQuery = StringUtils.format(
+
"{\"queryType\":\"timeseries\",\"dataSource\":\"%s\",\"granularity\":\"all\","
+ +
"\"intervals\":[\"2000/3000\"],\"aggregations\":[{\"type\":\"count\",\"name\":\"rows\"}]}",
+ dataSource
+ );
+ Assertions.assertEquals(403, postToBroker("/druid/v2",
nativeQuery).statusCode());
+
+ final String sqlQuery = StringUtils.format("{\"query\":\"SELECT COUNT(*)
FROM \\\"%s\\\"\"}", dataSource);
+ Assertions.assertEquals(403, postToBroker("/druid/v2/sql",
sqlQuery).statusCode());
+ }
+ finally {
+ updateBrokerDynamicConfig(BrokerDynamicConfig.builder().build());
+ }
+ }
+
+ private HttpResponse<String> postToBroker(String path, String body) throws
Exception
+ {
+ final HttpRequest request = HttpRequest
+ .newBuilder(URI.create(getServerUrl(broker) + path))
+ .header("Content-Type", "application/json")
+ .timeout(Duration.ofSeconds(10))
+ .POST(HttpRequest.BodyPublishers.ofString(body))
+ .build();
+
+ try (HttpClient client = HttpClient.newHttpClient()) {
+ return client.send(request, HttpResponse.BodyHandlers.ofString());
+ }
+ }
+
@Test
@Timeout(30)
public void testDynamicQueryContextTimeoutCausesQueryToFail()
diff --git
a/processing/src/main/java/org/apache/druid/common/exception/ErrorResponseTransformStrategy.java
b/processing/src/main/java/org/apache/druid/common/exception/ErrorResponseTransformStrategy.java
index b48ba97ca90..807926f6f69 100644
---
a/processing/src/main/java/org/apache/druid/common/exception/ErrorResponseTransformStrategy.java
+++
b/processing/src/main/java/org/apache/druid/common/exception/ErrorResponseTransformStrategy.java
@@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import org.apache.druid.error.DruidException;
+import javax.annotation.Nullable;
import javax.validation.constraints.NotNull;
import java.util.Optional;
import java.util.function.Function;
@@ -50,11 +51,26 @@ public interface ErrorResponseTransformStrategy
* It is the callers responsibility to do so. Returns Optional.empty() if no
transformation was applied.
* The errorId is provided to be used in the transformed Exception if needed.
*/
- default Optional<Exception> maybeTransform(DruidException exception,
Optional<String> errorId)
+ default Optional<DruidException> maybeTransform(DruidException exception,
Optional<String> errorId)
{
return Optional.empty();
}
+ /**
+ * Applies {@link #maybeTransform} and returns the exception to hand back to
the caller, or {@code exception} unchanged
+ * if this strategy does not transform it.
+ * <p>
+ * A transformed exception carries only {@code errorId}, not the original
message, so callers are responsible for
+ * logging {@code exception} against {@code errorId}. Note also that the
transformed exception carries its own category,
+ * so the status code the caller sees may differ from {@code
exception.getStatusCode()}.
+ *
+ * @param errorId id echoed back to the caller by the transformed exception;
a random one is used if null
+ */
+ default DruidException sanitizeForClient(DruidException exception, @Nullable
String errorId)
+ {
+ return maybeTransform(exception,
Optional.ofNullable(errorId)).orElse(exception);
+ }
+
/**
* Return a function for checking and transforming the error message if
needed.
* Function can return null if error message needs to be omitted or return
String to be use instead.
diff --git
a/processing/src/main/java/org/apache/druid/common/exception/PersonaBasedErrorTransformStrategy.java
b/processing/src/main/java/org/apache/druid/common/exception/PersonaBasedErrorTransformStrategy.java
index a0122204f79..3f83dc592f1 100644
---
a/processing/src/main/java/org/apache/druid/common/exception/PersonaBasedErrorTransformStrategy.java
+++
b/processing/src/main/java/org/apache/druid/common/exception/PersonaBasedErrorTransformStrategy.java
@@ -42,7 +42,7 @@ public class PersonaBasedErrorTransformStrategy implements
ErrorResponseTransfor
* exception was modified. Returns an empty optional if no transformation
was performed.
*/
@Override
- public Optional<Exception> maybeTransform(DruidException druidException,
Optional<String> optionalErrorId)
+ public Optional<DruidException> maybeTransform(DruidException
druidException, Optional<String> optionalErrorId)
{
if (druidException.getTargetPersona() == DruidException.Persona.USER) {
return Optional.empty();
diff --git
a/processing/src/test/java/org/apache/druid/common/exception/PersonaBasedErrorTransformStrategyTest.java
b/processing/src/test/java/org/apache/druid/common/exception/PersonaBasedErrorTransformStrategyTest.java
index 068fb30ab9c..fc6f446b18c 100644
---
a/processing/src/test/java/org/apache/druid/common/exception/PersonaBasedErrorTransformStrategyTest.java
+++
b/processing/src/test/java/org/apache/druid/common/exception/PersonaBasedErrorTransformStrategyTest.java
@@ -22,6 +22,7 @@ package org.apache.druid.common.exception;
import nl.jqno.equalsverifier.EqualsVerifier;
import org.apache.druid.error.DruidException;
import org.apache.druid.error.DruidExceptionMatcher;
+import org.hamcrest.MatcherAssert;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -52,13 +53,31 @@ public class PersonaBasedErrorTransformStrategyTest
{
DruidException druidException = DruidException.defensive().build("Test
Defensive exception");
- DruidExceptionMatcher druidExceptionMatcher = new DruidExceptionMatcher(
- DruidException.Persona.USER,
- druidException.getCategory(),
- druidException.getErrorCode()
- ).expectMessageContains("Could not process the query, please contact your
administrator with Error ID");
+ MatcherAssert.assertThat(
+ target.maybeTransform(druidException, Optional.of("the-error")).get(),
+ new DruidExceptionMatcher(
+ DruidException.Persona.USER,
+ DruidException.Category.RUNTIME_FAILURE,
+ "general"
+ ).expectMessageIs(
+ "Internal server error, please contact your administrator with
Error ID [the-error] if the issue persists."
+ )
+ );
+ }
+
+ @Test
+ public void testErrorIdIsGeneratedWhenAbsent()
+ {
+ DruidException druidException = DruidException.defensive().build("Test
Defensive exception");
- druidExceptionMatcher.matches(target.maybeTransform(druidException,
Optional.of("the-error")).get());
+ MatcherAssert.assertThat(
+ target.maybeTransform(druidException, Optional.empty()).get(),
+ new DruidExceptionMatcher(
+ DruidException.Persona.USER,
+ DruidException.Category.RUNTIME_FAILURE,
+ "general"
+ ).expectMessageContains("please contact your administrator with Error
ID [")
+ );
}
@Test
diff --git
a/server/src/main/java/org/apache/druid/server/BrokerQueryResource.java
b/server/src/main/java/org/apache/druid/server/BrokerQueryResource.java
index c5638561ed2..ad3b18b9ec0 100644
--- a/server/src/main/java/org/apache/druid/server/BrokerQueryResource.java
+++ b/server/src/main/java/org/apache/druid/server/BrokerQueryResource.java
@@ -31,6 +31,7 @@ import org.apache.druid.query.Query;
import org.apache.druid.query.QueryContexts;
import org.apache.druid.query.planning.ExecutionVertex;
import org.apache.druid.server.http.security.StateResourceFilter;
+import org.apache.druid.server.initialization.ServerConfig;
import org.apache.druid.server.security.AuthorizerMapper;
import javax.annotation.Nullable;
@@ -62,6 +63,7 @@ public class BrokerQueryResource extends QueryResource
AuthorizerMapper authorizerMapper,
QueryResourceQueryResultPusherFactory queryResultPusherFactory,
ResourceIOReaderWriterFactory resourceIOReaderWriterFactory,
+ ServerConfig serverConfig,
TimelineServerView brokerServerView
)
{
@@ -71,7 +73,8 @@ public class BrokerQueryResource extends QueryResource
queryScheduler,
authorizerMapper,
queryResultPusherFactory,
- resourceIOReaderWriterFactory
+ resourceIOReaderWriterFactory,
+ serverConfig
);
this.brokerServerView = brokerServerView;
}
diff --git a/server/src/main/java/org/apache/druid/server/QueryLifecycle.java
b/server/src/main/java/org/apache/druid/server/QueryLifecycle.java
index 9a514b85e2d..fc7d896df83 100644
--- a/server/src/main/java/org/apache/druid/server/QueryLifecycle.java
+++ b/server/src/main/java/org/apache/druid/server/QueryLifecycle.java
@@ -389,6 +389,10 @@ public class QueryLifecycle
Preconditions.checkNotNull(authenticationResult, "authenticationResult");
Preconditions.checkNotNull(authorizationResult, "authorizationResult");
+ // Authentication has already happened, so record the identity before
anything below can throw. Note this means
+ // a set authenticationResult implies only that authorization was
attempted, not that it succeeded.
+ this.authenticationResult = authenticationResult;
+
if (!authorizationResult.allowBasicAccess()) {
// Not authorized; go straight to Jail, do not pass Go.
transition(State.AUTHORIZING, State.UNAUTHORIZED);
@@ -405,7 +409,6 @@ public class QueryLifecycle
));
}
- this.authenticationResult = authenticationResult;
return authorizationResult;
}
diff --git a/server/src/main/java/org/apache/druid/server/QueryResource.java
b/server/src/main/java/org/apache/druid/server/QueryResource.java
index 80ea2895b97..ad96c20f32e 100644
--- a/server/src/main/java/org/apache/druid/server/QueryResource.java
+++ b/server/src/main/java/org/apache/druid/server/QueryResource.java
@@ -25,8 +25,11 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.jaxrs.smile.SmileMediaTypes;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.inject.Inject;
+import org.apache.druid.error.DruidException;
+import org.apache.druid.error.QueryExceptionCompat;
import org.apache.druid.guice.LazySingleton;
import org.apache.druid.guice.annotations.Json;
import org.apache.druid.java.util.common.jackson.JacksonUtils;
@@ -38,6 +41,7 @@ import org.apache.druid.query.QueryException;
import org.apache.druid.query.QueryInterruptedException;
import org.apache.druid.query.context.ResponseContext;
import org.apache.druid.query.context.ResponseContext.Keys;
+import org.apache.druid.server.initialization.ServerConfig;
import org.apache.druid.server.metrics.QueryCountStatsProvider;
import org.apache.druid.server.security.AuthorizationResult;
import org.apache.druid.server.security.AuthorizationUtils;
@@ -91,6 +95,7 @@ public class QueryResource implements QueryCountStatsProvider
protected final QueryScheduler queryScheduler;
protected final AuthorizerMapper authorizerMapper;
+ private final ServerConfig serverConfig;
private final QueryResourceQueryResultPusherFactory queryResultPusherFactory;
protected final ResourceIOReaderWriterFactory resourceIOReaderWriterFactory;
@@ -107,7 +112,8 @@ public class QueryResource implements
QueryCountStatsProvider
QueryScheduler queryScheduler,
AuthorizerMapper authorizerMapper,
QueryResourceQueryResultPusherFactory queryResultPusherFactory,
- ResourceIOReaderWriterFactory resourceIOReaderWriterFactory
+ ResourceIOReaderWriterFactory resourceIOReaderWriterFactory,
+ ServerConfig serverConfig
)
{
this.queryLifecycleFactory = queryLifecycleFactory;
@@ -116,6 +122,7 @@ public class QueryResource implements
QueryCountStatsProvider
this.authorizerMapper = authorizerMapper;
this.queryResultPusherFactory = queryResultPusherFactory;
this.resourceIOReaderWriterFactory = resourceIOReaderWriterFactory;
+ this.serverConfig = serverConfig;
}
@DELETE
@@ -182,15 +189,7 @@ public class QueryResource implements
QueryCountStatsProvider
authResult = queryLifecycle.authorize(req);
}
catch (RuntimeException e) {
- final QueryException qe;
-
- if (e instanceof QueryException) {
- qe = (QueryException) e;
- } else {
- qe = new QueryInterruptedException(e);
- }
-
- return
io.getResponseWriter().buildNonOkResponse(qe.getFailType().getExpectedStatus(),
qe);
+ return handleAuthorizeFailure(queryLifecycle, io, req, e);
}
if (!authResult.allowBasicAccess()) {
@@ -239,6 +238,43 @@ public class QueryResource implements
QueryCountStatsProvider
}
}
+ /**
+ * Builds the response for a query that failed during {@link
QueryLifecycle#authorize}, before a
+ * {@link QueryResultPusher} exists to do it. The pusher is what normally
records the failure, so this has to emit the
+ * logs, metrics and counters itself; otherwise the caller gets a response
but nothing is recorded server-side.
+ * <p>
+ * A {@link DruidException} is reported under its own category. Everything
else goes through the {@link QueryException}
+ * conversion below, which resolves anything it does not recognise to
+ * {@link QueryException#UNKNOWN_EXCEPTION_ERROR_CODE} and therefore a 500.
+ */
+ private Response handleAuthorizeFailure(
+ final QueryLifecycle queryLifecycle,
+ final ResourceIOReaderWriterFactory.ResourceIOReaderWriter io,
+ final HttpServletRequest req,
+ final RuntimeException e
+ ) throws IOException
+ {
+ // Logs the exception with the query id, which doubles as the error id
below.
+ queryLifecycle.emitLogsAndMetrics(e, req.getRemoteAddr(), -1);
+
+ if (e instanceof DruidException) {
+ final DruidException druidException = (DruidException) e;
+ QueryResultPusher.incrementQueryCounterForException(counter,
druidException);
+
+ final String queryId = queryLifecycle.getQueryId();
+ return QueryResultPusher.handleDruidExceptionBeforeResponseStarted(
+
serverConfig.getErrorResponseTransformStrategy().sanitizeForClient(druidException,
queryId),
+ MediaType.valueOf(io.getResponseWriter().getResponseType()),
+ ImmutableMap.of(QUERY_ID_RESPONSE_HEADER, queryId)
+ );
+ }
+
+ final QueryException qe = e instanceof QueryException ? (QueryException) e
: new QueryInterruptedException(e);
+ // Converted only to reuse the category mapping; the response body stays
in the legacy QueryException format.
+ QueryResultPusher.incrementQueryCounterForException(counter,
DruidException.fromFailure(new QueryExceptionCompat(qe)));
+ return
io.getResponseWriter().buildNonOkResponse(qe.getFailType().getExpectedStatus(),
qe);
+ }
+
public interface QueryMetricCounter
{
void incrementSuccess();
diff --git
a/server/src/main/java/org/apache/druid/server/QueryResultPusher.java
b/server/src/main/java/org/apache/druid/server/QueryResultPusher.java
index ee9849a7b11..cb958ea9857 100644
--- a/server/src/main/java/org/apache/druid/server/QueryResultPusher.java
+++ b/server/src/main/java/org/apache/druid/server/QueryResultPusher.java
@@ -228,7 +228,10 @@ public abstract class QueryResultPusher
return handleDruidException(resultsWriter, DruidException.fromFailure(new
QueryExceptionCompat(e)));
}
- private void incrementQueryCounterForException(final DruidException e)
+ static void incrementQueryCounterForException(
+ final QueryResource.QueryMetricCounter counter,
+ final DruidException e
+ )
{
switch (e.getCategory()) {
case INVALID_INPUT:
@@ -251,7 +254,7 @@ public abstract class QueryResultPusher
private Response handleDruidException(ResultsWriter resultsWriter,
DruidException e)
{
- incrementQueryCounterForException(e);
+ incrementQueryCounterForException(counter, e);
if (resultsWriter != null) {
final long bytesWritten = accumulator != null ?
accumulator.getNumBytesSent() : 0;
diff --git
a/server/src/main/java/org/apache/druid/server/initialization/ServerConfig.java
b/server/src/main/java/org/apache/druid/server/initialization/ServerConfig.java
index a7206ca9cf7..92fa8dfd0a5 100644
---
a/server/src/main/java/org/apache/druid/server/initialization/ServerConfig.java
+++
b/server/src/main/java/org/apache/druid/server/initialization/ServerConfig.java
@@ -124,6 +124,12 @@ public class ServerConfig
this.enableQueryRequestsQueuing = enableQueryRequestsQueuing;
}
+ @VisibleForTesting
+ public ServerConfig(@NotNull ErrorResponseTransformStrategy
errorResponseTransformStrategy)
+ {
+ this.errorResponseTransformStrategy = errorResponseTransformStrategy;
+ }
+
@JsonProperty
@Min(1)
private int numThreads = getDefaultNumThreads();
diff --git
a/server/src/test/java/org/apache/druid/server/QueryResourceTest.java
b/server/src/test/java/org/apache/druid/server/QueryResourceTest.java
index d7a611bfc32..5d4ac54b99c 100644
--- a/server/src/test/java/org/apache/druid/server/QueryResourceTest.java
+++ b/server/src/test/java/org/apache/druid/server/QueryResourceTest.java
@@ -31,6 +31,8 @@ import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.inject.Injector;
import com.google.inject.Key;
+import org.apache.druid.client.BrokerViewOfBrokerConfig;
+import org.apache.druid.common.exception.ErrorResponseTransformStrategy;
import org.apache.druid.error.DruidException;
import org.apache.druid.error.DruidExceptionMatcher;
import org.apache.druid.error.ErrorResponse;
@@ -40,6 +42,7 @@ import org.apache.druid.guice.annotations.Smile;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.concurrent.Execs;
import org.apache.druid.java.util.common.guava.Accumulator;
import org.apache.druid.java.util.common.guava.BaseSequence;
@@ -74,6 +77,7 @@ import org.apache.druid.query.filter.NullFilter;
import org.apache.druid.query.policy.NoopPolicyEnforcer;
import org.apache.druid.query.policy.RowFilterPolicy;
import org.apache.druid.query.timeboundary.TimeBoundaryResultValue;
+import org.apache.druid.server.broker.BrokerDynamicConfig;
import org.apache.druid.server.initialization.ServerConfig;
import org.apache.druid.server.log.TestRequestLogger;
import org.apache.druid.server.metrics.NoopServiceEmitter;
@@ -103,9 +107,12 @@ import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.internal.matchers.ThrowableMessageMatcher;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mockito;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
@@ -122,12 +129,14 @@ import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
+import java.util.function.Function;
import java.util.stream.Collectors;
public class QueryResourceTest
@@ -261,33 +270,68 @@ public class QueryResourceTest
queryResource = createQueryResource(ResponseContextConfig.newConfig(true));
}
+ private QueryLifecycleFactory createQueryLifecycleFactory()
+ {
+ return new QueryLifecycleFactory(
+ CONGLOMERATE,
+ TEST_SEGMENT_WALKER,
+ new DefaultGenericQueryMetricsFactory(),
+ emitter,
+ testRequestLogger,
+ new AuthConfig(),
+ NoopPolicyEnforcer.instance(),
+ AuthTestUtils.TEST_AUTHORIZER_MAPPER,
+ new DefaultQueryConfig(Map.of()),
+ null
+ );
+ }
+
private QueryResource createQueryResource(ResponseContextConfig
responseContextConfig)
+ {
+ return createQueryResource(
+ createQueryLifecycleFactory(),
+ null,
+ queryScheduler,
+ responseContextConfig,
+ smileMapper,
+ new ServerConfig()
+ );
+ }
+
+ private QueryResource createQueryResource(QueryLifecycleFactory
queryLifecycleFactory)
+ {
+ return createQueryResource(
+ queryLifecycleFactory,
+ null,
+ queryScheduler,
+ ResponseContextConfig.newConfig(true),
+ smileMapper,
+ new ServerConfig()
+ );
+ }
+
+ /**
+ * Every {@link QueryResource} under test is built here, so a change to its
constructor touches one call site.
+ *
+ * @param responseMapper mapper backing the response writer, i.e. the smile
mapper unless the test needs json
+ */
+ private QueryResource createQueryResource(
+ final QueryLifecycleFactory queryLifecycleFactory,
+ @Nullable final AuthorizerMapper authorizerMapper,
+ final QueryScheduler queryScheduler,
+ final ResponseContextConfig responseContextConfig,
+ final ObjectMapper responseMapper,
+ final ServerConfig serverConfig
+ )
{
return new QueryResource(
- new QueryLifecycleFactory(
- CONGLOMERATE,
- TEST_SEGMENT_WALKER,
- new DefaultGenericQueryMetricsFactory(),
- emitter,
- testRequestLogger,
- new AuthConfig(),
- NoopPolicyEnforcer.instance(),
- AuthTestUtils.TEST_AUTHORIZER_MAPPER,
- new DefaultQueryConfig(Map.of()),
- null
- ),
+ queryLifecycleFactory,
jsonMapper,
queryScheduler,
- null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- responseContextConfig,
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
- )
+ authorizerMapper,
+ new QueryResourceQueryResultPusherFactory(jsonMapper,
responseContextConfig, DRUID_NODE),
+ new ResourceIOReaderWriterFactory(jsonMapper, responseMapper),
+ serverConfig
);
}
@@ -313,7 +357,7 @@ public class QueryResourceTest
final String overrideConfigKey = "priority";
final String overrideConfigValue = "678";
DefaultQueryConfig overrideConfig = new
DefaultQueryConfig(ImmutableMap.of(overrideConfigKey, overrideConfigValue));
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
TEST_SEGMENT_WALKER,
@@ -325,18 +369,6 @@ public class QueryResourceTest
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
overrideConfig,
null
- ),
- jsonMapper,
- queryScheduler,
- null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
)
);
@@ -367,13 +399,13 @@ public class QueryResourceTest
);
}
- @Test
- public void testGoodQueryThrowsDruidExceptionFromLifecycleExecute() throws
IOException
+ /**
+ * A {@link QueryResource} whose walker throws once the query is already
executing, so the failure surfaces through
+ * {@link QueryResultPusher} rather than before it.
+ */
+ private QueryResource createQueryResourceFailingInExecute(final
DefaultQueryConfig queryConfig)
{
- String overrideConfigKey = "priority";
- String overrideConfigValue = "678";
- DefaultQueryConfig overrideConfig = new
DefaultQueryConfig(ImmutableMap.of(overrideConfigKey, overrideConfigValue));
- queryResource = new QueryResource(
+ return createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
new QuerySegmentWalker()
@@ -404,22 +436,19 @@ public class QueryResourceTest
new AuthConfig(),
NoopPolicyEnforcer.instance(),
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
- overrideConfig,
+ queryConfig,
null
- ),
- jsonMapper,
- queryScheduler,
- null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
)
);
+ }
+
+ @Test
+ public void testGoodQueryThrowsDruidExceptionFromLifecycleExecute() throws
IOException
+ {
+ String overrideConfigKey = "priority";
+ String overrideConfigValue = "678";
+ DefaultQueryConfig overrideConfig = new
DefaultQueryConfig(ImmutableMap.of(overrideConfigKey, overrideConfigValue));
+ queryResource = createQueryResourceFailingInExecute(overrideConfig);
expectPermissiveHappyPathAuth();
@@ -452,7 +481,7 @@ public class QueryResourceTest
@Test
public void testResponseWithIncludeTrailerHeader() throws IOException
{
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
new QuerySegmentWalker()
@@ -507,16 +536,7 @@ public class QueryResourceTest
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
new DefaultQueryConfig(Map.of()),
null
- ),
- jsonMapper,
- queryScheduler,
- null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(jsonMapper, smileMapper)
+ )
);
expectPermissiveHappyPathAuth();
@@ -543,7 +563,7 @@ public class QueryResourceTest
@Test
public void testResponseWithMidFlightExceptions() throws IOException
{
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
new QuerySegmentWalker()
@@ -593,16 +613,7 @@ public class QueryResourceTest
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
new DefaultQueryConfig(Map.of()),
null
- ),
- jsonMapper,
- queryScheduler,
- null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(jsonMapper, smileMapper)
+ )
);
expectPermissiveHappyPathAuth();
@@ -630,7 +641,7 @@ public class QueryResourceTest
Intervals.of("2025-01-01/P1D"), "0", 1
);
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
new QuerySegmentWalker()
@@ -692,18 +703,6 @@ public class QueryResourceTest
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
new DefaultQueryConfig(Map.of()),
null
- ),
- jsonMapper,
- queryScheduler,
- null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
)
);
@@ -758,7 +757,7 @@ public class QueryResourceTest
}
};
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(null, null, null, null, null, null,
NoopPolicyEnforcer.instance(), null, overrideConfig, null)
{
@@ -789,19 +788,7 @@ public class QueryResourceTest
}
};
}
- },
- jsonMapper,
- queryScheduler,
- null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
- )
+ }
);
expectPermissiveHappyPathAuth();
@@ -828,7 +815,7 @@ public class QueryResourceTest
String overrideConfigKey = "priority";
String overrideConfigValue = "678";
DefaultQueryConfig overrideConfig = new
DefaultQueryConfig(ImmutableMap.of(overrideConfigKey, overrideConfigValue));
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
TEST_SEGMENT_WALKER,
@@ -840,18 +827,6 @@ public class QueryResourceTest
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
overrideConfig,
null
- ),
- jsonMapper,
- queryScheduler,
- null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
)
);
@@ -1100,7 +1075,7 @@ public class QueryResourceTest
}
};
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
TEST_SEGMENT_WALKER,
@@ -1113,18 +1088,11 @@ public class QueryResourceTest
new DefaultQueryConfig(Map.of()),
null
),
- jsonMapper,
- queryScheduler,
authMapper,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
- )
+ queryScheduler,
+ ResponseContextConfig.newConfig(true),
+ smileMapper,
+ new ServerConfig()
);
@@ -1182,7 +1150,7 @@ public class QueryResourceTest
}
};
- final QueryResource timeoutQueryResource = new QueryResource(
+ final QueryResource timeoutQueryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
timeoutSegmentWalker,
@@ -1195,18 +1163,11 @@ public class QueryResourceTest
new DefaultQueryConfig(Map.of()),
null
),
- jsonMapper,
- queryScheduler,
null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- jsonMapper
- )
+ queryScheduler,
+ ResponseContextConfig.newConfig(true),
+ jsonMapper,
+ new ServerConfig()
);
expectPermissiveHappyPathAuth();
@@ -1287,7 +1248,7 @@ public class QueryResourceTest
}
};
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
TEST_SEGMENT_WALKER,
@@ -1300,18 +1261,11 @@ public class QueryResourceTest
new DefaultQueryConfig(Map.of()),
null
),
- jsonMapper,
- queryScheduler,
authMapper,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
- )
+ queryScheduler,
+ ResponseContextConfig.newConfig(true),
+ smileMapper,
+ new ServerConfig()
);
final String queryString = "{\"queryType\":\"timeBoundary\",
\"dataSource\":\"allow\","
@@ -1401,7 +1355,7 @@ public class QueryResourceTest
}
};
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
TEST_SEGMENT_WALKER,
@@ -1414,18 +1368,11 @@ public class QueryResourceTest
new DefaultQueryConfig(Map.of()),
null
),
- jsonMapper,
- queryScheduler,
authMapper,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
- )
+ queryScheduler,
+ ResponseContextConfig.newConfig(true),
+ smileMapper,
+ new ServerConfig()
);
final String queryString = "{\"queryType\":\"timeBoundary\",
\"dataSource\":\"allow\","
@@ -1791,7 +1738,7 @@ public class QueryResourceTest
}
};
- queryResource = new QueryResource(
+ queryResource = createQueryResource(
new QueryLifecycleFactory(
CONGLOMERATE,
texasRanger,
@@ -1804,18 +1751,11 @@ public class QueryResourceTest
new DefaultQueryConfig(Map.of()),
null
),
- jsonMapper,
- scheduler,
null,
- new QueryResourceQueryResultPusherFactory(
- jsonMapper,
- ResponseContextConfig.newConfig(true),
- DRUID_NODE
- ),
- new ResourceIOReaderWriterFactory(
- jsonMapper,
- smileMapper
- )
+ scheduler,
+ ResponseContextConfig.newConfig(true),
+ smileMapper,
+ new ServerConfig()
);
}
@@ -1835,6 +1775,218 @@ public class QueryResourceTest
});
}
+ @Test
+ public void testBlocklistedQueryReturnsForbidden() throws IOException
+ {
+ expectPermissiveHappyPathAuth();
+
+ final QueryResource blockingQueryResource =
createQueryResourceWithBlocklist(
+ new ServerConfig(),
+ new DefaultQueryBlocklistRule("block-mmx",
ImmutableSet.of("mmx_metrics"), null, null)
+ );
+
+ final Response response = blockingQueryResource.doPost(
+ new
ByteArrayInputStream(SIMPLE_TIMESERIES_QUERY.getBytes(StandardCharsets.UTF_8)),
+ null /*pretty*/,
+ testServletRequest
+ );
+
+ Assert.assertNotNull(response);
+ Assert.assertEquals(Status.FORBIDDEN.getStatusCode(),
response.getStatus());
+
Assert.assertNotNull(response.getMetadata().getFirst(QueryResource.QUERY_ID_RESPONSE_HEADER));
+
+ MatcherAssert.assertThat(
+ ((ErrorResponse) response.getEntity()).getUnderlyingException(),
+ DruidExceptionMatcher.forbidden().expectMessageContains("blocked by
rule[block-mmx]")
+ );
+
+ // Blocked queries are still recorded in metrics and the request log.
FORBIDDEN maps to no query counter, the
+ // same as when the exception surfaces through QueryResultPusher.
+ Assert.assertEquals(0, blockingQueryResource.getFailedQueryCount());
+ Assert.assertEquals(0, blockingQueryResource.getInterruptedQueryCount());
+ Assert.assertEquals(1, testRequestLogger.getNativeQuerylogs().size());
+ final Map<String, Object> stats =
testRequestLogger.getNativeQuerylogs().get(0).getQueryStats().getStats();
+ Assert.assertEquals(false, stats.get("success"));
+ // The blocklist throws mid-authorization, so the identity is only present
if it was recorded before that point.
+ Assert.assertEquals(AUTHENTICATION_RESULT.getIdentity(),
stats.get("identity"));
+ Assert.assertEquals(Status.FORBIDDEN.getStatusCode(),
stats.get(DruidMetrics.STATUS_CODE));
+ }
+
+ @Test
+ public void
testBlocklistedQueryIsSanitizedByErrorResponseTransformStrategy() throws
IOException
+ {
+ expectPermissiveHappyPathAuth();
+
+ // Always-transforming strategy, so the test does not depend on any
particular strategy's persona rules.
+ final ErrorResponseTransformStrategy strategy = new
ErrorResponseTransformStrategy()
+ {
+ @Override
+ public Optional<DruidException> maybeTransform(DruidException exception,
Optional<String> errorId)
+ {
+ return Optional.of(
+ DruidException.forPersona(DruidException.Persona.USER)
+ .ofCategory(DruidException.Category.RUNTIME_FAILURE)
+ .build("sanitized[%s]", errorId.orElse(null))
+ );
+ }
+
+ @Override
+ public Function<String, String> getErrorMessageTransformFunction()
+ {
+ throw new UnsupportedOperationException();
+ }
+ };
+
+ final QueryResource blockingQueryResource =
createQueryResourceWithBlocklist(
+ new ServerConfig(strategy),
+ new DefaultQueryBlocklistRule("block-mmx",
ImmutableSet.of("mmx_metrics"), null, null)
+ );
+
+ final Response response = blockingQueryResource.doPost(
+ new
ByteArrayInputStream(SIMPLE_TIMESERIES_QUERY.getBytes(StandardCharsets.UTF_8)),
+ null /*pretty*/,
+ testServletRequest
+ );
+
+ Assert.assertNotNull(response);
+ // The transformed exception's own category now drives the status code,
not the original FORBIDDEN.
+ Assert.assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(),
response.getStatus());
+
+ final Object queryId =
response.getMetadata().getFirst(QueryResource.QUERY_ID_RESPONSE_HEADER);
+ Assert.assertNotNull(queryId);
+ MatcherAssert.assertThat(
+ ((ErrorResponse) response.getEntity()).getUnderlyingException(),
+ new DruidExceptionMatcher(
+ DruidException.Persona.USER,
+ DruidException.Category.RUNTIME_FAILURE,
+ "general"
+ ).expectMessageIs(StringUtils.format("sanitized[%s]", queryId))
+ );
+
+ // Sanitization applies to the client response only; the request log keeps
the original 403.
+ Assert.assertEquals(1, testRequestLogger.getNativeQuerylogs().size());
+ Assert.assertEquals(
+ Status.FORBIDDEN.getStatusCode(),
+
testRequestLogger.getNativeQuerylogs().get(0).getQueryStats().getStats().get(DruidMetrics.STATUS_CODE)
+ );
+ }
+
+ @Test
+ public void testDruidExceptionFromAuthorizeIsCountedByCategory() throws
IOException
+ {
+ expectPermissiveHappyPathAuth();
+
+ final QueryLifecycleFactory realFactory = createQueryLifecycleFactory();
+ final QueryLifecycleFactory failingFactory =
Mockito.mock(QueryLifecycleFactory.class);
+ Mockito.when(failingFactory.factorize()).thenAnswer(invocation -> {
+ final QueryLifecycle lifecycle = Mockito.spy(realFactory.factorize());
+ // DEFENSIVE maps to the "failed" counter, unlike the FORBIDDEN thrown
by the blocklist.
+ Mockito.doThrow(DruidException.defensive("oh no"))
+ .when(lifecycle)
+ .authorize(ArgumentMatchers.any(HttpServletRequest.class));
+ return lifecycle;
+ });
+
+ final QueryResource failingQueryResource =
createQueryResource(failingFactory);
+
+ final Response response = failingQueryResource.doPost(
+ new
ByteArrayInputStream(SIMPLE_TIMESERIES_QUERY.getBytes(StandardCharsets.UTF_8)),
+ null /*pretty*/,
+ testServletRequest
+ );
+
+ Assert.assertNotNull(response);
+ Assert.assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(),
response.getStatus());
+ Assert.assertEquals(1, failingQueryResource.getFailedQueryCount());
+ Assert.assertEquals(0, failingQueryResource.getInterruptedQueryCount());
+ }
+
+ @Test
+ public void testQueryExceptionFromAuthorizeIsRecorded() throws IOException
+ {
+ expectPermissiveHappyPathAuth();
+
+ final QueryLifecycleFactory realFactory = createQueryLifecycleFactory();
+ final QueryLifecycleFactory failingFactory =
Mockito.mock(QueryLifecycleFactory.class);
+ Mockito.when(failingFactory.factorize()).thenAnswer(invocation -> {
+ final QueryLifecycle lifecycle = Mockito.spy(realFactory.factorize());
+
Mockito.doThrow(QueryCapacityExceededException.withErrorMessageAndResolvedHost("too
busy"))
+ .when(lifecycle)
+ .authorize(ArgumentMatchers.any(HttpServletRequest.class));
+ return lifecycle;
+ });
+
+ final QueryResource failingQueryResource =
createQueryResource(failingFactory);
+
+ final Response response = failingQueryResource.doPost(
+ new
ByteArrayInputStream(SIMPLE_TIMESERIES_QUERY.getBytes(StandardCharsets.UTF_8)),
+ null /*pretty*/,
+ testServletRequest
+ );
+
+ // A QueryException keeps its own status and its legacy response body.
+ Assert.assertNotNull(response);
+ Assert.assertEquals(429, response.getStatus());
+ MatcherAssert.assertThat(
+ StringUtils.fromUtf8((byte[]) response.getEntity()),
+ CoreMatchers.containsString("too busy")
+ );
+
+ // It now shares the DruidException path, so the failure is recorded
server-side and not just returned.
+ Assert.assertEquals(1, failingQueryResource.getFailedQueryCount());
+ Assert.assertEquals(1, testRequestLogger.getNativeQuerylogs().size());
+ final Map<String, Object> stats =
testRequestLogger.getNativeQuerylogs().get(0).getQueryStats().getStats();
+ Assert.assertEquals(false, stats.get("success"));
+ Assert.assertEquals(429, stats.get(DruidMetrics.STATUS_CODE));
+ }
+
+ @Test
+ public void testNonBlocklistedQueryIsNotAffectedByBlocklist() throws
IOException
+ {
+ expectPermissiveHappyPathAuth();
+
+ final QueryResource blockingQueryResource =
createQueryResourceWithBlocklist(
+ new ServerConfig(),
+ new DefaultQueryBlocklistRule("block-other",
ImmutableSet.of("some_other_datasource"), null, null)
+ );
+
+ final MockHttpServletResponse response = expectAsyncRequestFlow(
+ testServletRequest,
+ SIMPLE_TIMESERIES_QUERY.getBytes(StandardCharsets.UTF_8),
+ blockingQueryResource
+ );
+
+ Assert.assertEquals(Status.OK.getStatusCode(), response.getStatus());
+ }
+
+ private QueryResource createQueryResourceWithBlocklist(ServerConfig
serverConfig, QueryBlocklistRule... rules)
+ {
+ final BrokerViewOfBrokerConfig brokerViewOfBrokerConfig =
Mockito.mock(BrokerViewOfBrokerConfig.class);
+ Mockito.when(brokerViewOfBrokerConfig.getDynamicConfig()).thenReturn(
+ new
BrokerDynamicConfig.Builder().withQueryBlocklist(Arrays.asList(rules)).build()
+ );
+
+ return createQueryResource(
+ new QueryLifecycleFactory(
+ CONGLOMERATE,
+ TEST_SEGMENT_WALKER,
+ new DefaultGenericQueryMetricsFactory(),
+ emitter,
+ testRequestLogger,
+ new AuthConfig(),
+ NoopPolicyEnforcer.instance(),
+ AuthTestUtils.TEST_AUTHORIZER_MAPPER,
+ new DefaultQueryConfig(Map.of()),
+ brokerViewOfBrokerConfig
+ ),
+ null,
+ queryScheduler,
+ ResponseContextConfig.newConfig(true),
+ smileMapper,
+ serverConfig
+ );
+ }
+
private void expectPermissiveHappyPathAuth()
{
testServletRequest.setAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT,
AUTHENTICATION_RESULT);
diff --git a/sql/src/main/java/org/apache/druid/sql/avatica/ErrorHandler.java
b/sql/src/main/java/org/apache/druid/sql/avatica/ErrorHandler.java
index 8e2927279ee..32a0369ce17 100644
--- a/sql/src/main/java/org/apache/druid/sql/avatica/ErrorHandler.java
+++ b/sql/src/main/java/org/apache/druid/sql/avatica/ErrorHandler.java
@@ -87,9 +87,10 @@ class ErrorHandler
return new
RuntimeException(errorResponseTransformStrategy.transformIfNeeded((SanitizableException)
error.getCause()));
}
if (error instanceof DruidException) {
+ final DruidException druidError = (DruidException) error;
String errorId = UUID.randomUUID().toString();
- Optional<Exception> transformedException =
errorResponseTransformStrategy.maybeTransform(
- (DruidException) error,
+ Optional<DruidException> transformedException =
errorResponseTransformStrategy.maybeTransform(
+ druidError,
Optional.of(errorId)
);
@@ -97,7 +98,7 @@ class ErrorHandler
// Log the exception here itself, since the error has been transformed.
log.error(error, StringUtils.format("External Error ID: [%s]",
errorId));
}
- QueryInterruptedException wrappedError =
QueryInterruptedException.wrapIfNeeded(transformedException.orElse((Exception)
error));
+ QueryInterruptedException wrappedError =
QueryInterruptedException.wrapIfNeeded(transformedException.orElse(druidError));
return (QueryException)
errorResponseTransformStrategy.transformIfNeeded(wrappedError);
}
QueryInterruptedException wrappedError =
QueryInterruptedException.wrapIfNeeded(error);
diff --git a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
index 4ca78c2774a..41b9f6cb84f 100644
--- a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
+++ b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java
@@ -371,32 +371,36 @@ public class SqlResource
final ErrorResponseTransformStrategy strategy
)
{
+ final String sqlQueryId =
queryContext.getString(QueryContexts.CTX_SQL_QUERY_ID);
+ final String errorId = sqlQueryId == null ? UUID.randomUUID().toString() :
sqlQueryId;
+
+ final DruidException druidException;
+ final Map<String, String> headers;
if (e instanceof DruidException) {
- final String sqlQueryId =
queryContext.getString(QueryContexts.CTX_SQL_QUERY_ID);
- String errorId = sqlQueryId == null ? UUID.randomUUID().toString() :
sqlQueryId;
- Optional<Exception> transformed =
strategy.maybeTransform((DruidException) e, Optional.of(errorId));
- if (transformed.isPresent()) {
- // Log the exception here itself, since the error has been transformed.
- log.error(e, StringUtils.format("External Error ID: [%s]", errorId));
- }
- return QueryResultPusher.handleDruidExceptionBeforeResponseStarted(
- (DruidException) transformed.orElse(e),
- MediaType.APPLICATION_JSON_TYPE,
- sqlQueryId != null
- ? ImmutableMap.<String, String>builder()
- .put(QueryResource.QUERY_ID_RESPONSE_HEADER,
sqlQueryId)
- .put(SQL_QUERY_ID_RESPONSE_HEADER, sqlQueryId)
- .build()
- : Collections.emptyMap()
- );
+ druidException = (DruidException) e;
+ headers = sqlQueryId != null
+ ? ImmutableMap.<String, String>builder()
+ .put(QueryResource.QUERY_ID_RESPONSE_HEADER,
sqlQueryId)
+ .put(SQL_QUERY_ID_RESPONSE_HEADER, sqlQueryId)
+ .build()
+ : Collections.emptyMap();
} else {
- return QueryResultPusher.handleDruidExceptionBeforeResponseStarted(
- DruidException.forPersona(DruidException.Persona.OPERATOR)
- .ofCategory(DruidException.Category.RUNTIME_FAILURE)
- .build(e, "Cannot handle query"),
- MediaType.APPLICATION_JSON_TYPE,
- Collections.emptyMap()
- );
+ druidException =
DruidException.forPersona(DruidException.Persona.OPERATOR)
+
.ofCategory(DruidException.Category.RUNTIME_FAILURE)
+ .build(e, "Cannot handle query");
+ headers = Collections.emptyMap();
}
+
+ final Optional<DruidException> transformed =
strategy.maybeTransform(druidException, Optional.of(errorId));
+ if (transformed.isPresent()) {
+ // Nothing else logs this failure, and the client is only given the
error id from here on.
+ log.error(e, StringUtils.format("External Error ID: [%s]", errorId));
+ }
+
+ return QueryResultPusher.handleDruidExceptionBeforeResponseStarted(
+ transformed.orElse(druidException),
+ MediaType.APPLICATION_JSON_TYPE,
+ headers
+ );
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
