More useful query errors. (#3335)

Follow-up to #1773, which meant to add more useful query errors but
did not actually do so. Since that patch, any error other than
interrupt/cancel/timeout was reported as `{"error":"Unknown exception"}`.

With this patch, the error fields are:

- error, one of the specific strings "Query interrupted", "Query timeout",
  "Query cancelled", or "Unknown exception" (same behavior as before).
- errorMessage, the message of the topmost non-QueryInterruptedException
  in the causality chain.
- errorClass, the class of the topmost non-QueryInterruptedException
  in the causality chain.
- host, the host that failed the query.

docs/content/querying/querying.md

@@ -66,3 +66,35 @@ For example, if the query ID is `abc123`, the query can be cancelled as follows:
 ```sh
 curl -X DELETE "http://host:port/druid/v2/abc123"
 ```
+
+Query Errors
+------------
+
+If a query fails, you will get an HTTP 500 response containing a JSON object with the following structure:
+
+```json
+{
+  "error" : "Query timeout",
+  "errorMessage" : "Timeout waiting for task.",
+  "errorClass" : "java.util.concurrent.TimeoutException",
+  "host" : "druid1.example.com:8083"
+}
+```
+
+The fields in the response are:
+
+|field|description|
+|-----|-----------|
+|error|A well-defined error code (see below).|
+|errorMessage|A free-form message with more information about the error. May be null.|
+|errorClass|The class of the exception that caused this error. May be null.|
+|host|The host on which this error occurred. May be null.|
+
+Possible codes for the *error* field include:
+
+|code|description|
+|----|-----------|
+|`Query timeout`|The query timed out.|
+|`Query interrupted`|The query was interrupted, possibly due to JVM shutdown.|
+|`Query cancelled`|The query was cancelled through the query cancellation API.|
+|`Unknown exception`|Some other exception occurred. Check errorMessage and errorClass for details, although keep in mind that the contents of those fields are free-form and may change from release to release.|

processing/src/main/java/io/druid/query/QueryInterruptedException.java

@@ -21,12 +21,24 @@ package io.druid.query;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.collect.ImmutableSet;
 
-import java.util.Set;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.TimeoutException;
 
+/**
+ * Exception representing a failed query. The name "QueryInterruptedException" is a misnomer; this is actually
+ * used on the client side for *all* kinds of failed queries.
+ *
+ * Fields:
+ * - "errorCode" is a well-defined errorCode code taken from a specific list (see the static constants). "Unknown exception"
+ * represents all wrapped exceptions other than interrupt/timeout/cancellation.
+ * - "errorMessage" is the toString of the wrapped exception
+ * - "errorClass" is the class of the wrapped exception
+ * - "host" is the host that the errorCode occurred on
+ *
+ * The QueryResource is expected to emit the JSON form of this object when errors happen, and the DirectDruidClient
+ * deserializes and wraps them.
+ */
 public class QueryInterruptedException extends RuntimeException
 {
   public static final String QUERY_INTERRUPTED = "Query interrupted";
@@ -34,75 +46,100 @@ public class QueryInterruptedException extends RuntimeException
   public static final String QUERY_CANCELLED = "Query cancelled";
   public static final String UNKNOWN_EXCEPTION = "Unknown exception";
 
-  private static final Set<String> listKnownException = ImmutableSet.of(
-      QUERY_CANCELLED,
-      QUERY_INTERRUPTED,
-      QUERY_TIMEOUT,
-      UNKNOWN_EXCEPTION
-  );
-
-  @JsonProperty
-  private final String causeMessage;
-  @JsonProperty
+  private final String errorCode;
+  private final String errorClass;
   private final String host;
 
   @JsonCreator
   public QueryInterruptedException(
-      @JsonProperty("error") String message,
-      @JsonProperty("causeMessage") String causeMessage,
+      @JsonProperty("error") String errorCode,
+      @JsonProperty("errorMessage") String errorMessage,
+      @JsonProperty("errorClass") String errorClass,
       @JsonProperty("host") String host
   )
   {
-    super(message);
-    this.causeMessage = causeMessage;
+    super(errorMessage);
+    this.errorCode = errorCode;
+    this.errorClass = errorClass;
     this.host = host;
   }
 
+  /**
+   * Creates a new QueryInterruptedException wrapping an underlying exception. The errorMessage and errorClass
+   * of this exception will be based on the highest non-QueryInterruptedException in the causality chain.
+   *
+   * @param cause wrapped exception
+   */
   public QueryInterruptedException(Throwable cause)
   {
-    this(cause, null);
+    this(cause, getHostFromThrowable(cause));
   }
 
-  public QueryInterruptedException(Throwable e, String host)
+  public QueryInterruptedException(Throwable cause, String host)
   {
-    super(e);
+    super(cause == null ? null : cause.getMessage(), cause);
+    this.errorCode = getErrorCodeFromThrowable(cause);
+    this.errorClass = getErrorClassFromThrowable(cause);
     this.host = host;
-    causeMessage = e.getMessage();
   }
 
   @JsonProperty("error")
+  public String getErrorCode()
+  {
+    return errorCode;
+  }
+
+  @JsonProperty("errorMessage")
   @Override
   public String getMessage()
   {
-    if (this.getCause() == null) {
     return super.getMessage();
-    } else if (this.getCause() instanceof QueryInterruptedException) {
-      return getCause().getMessage();
-    } else if (this.getCause() instanceof InterruptedException) {
+  }
+
+  @JsonProperty
+  public String getErrorClass()
+  {
+    return errorClass;
+  }
+
+  @JsonProperty
+  public String getHost()
+  {
+    return host;
+  }
+
+  private static String getErrorCodeFromThrowable(Throwable e)
+  {
+    if (e instanceof QueryInterruptedException) {
+      return ((QueryInterruptedException) e).getErrorCode();
+    } else if (e instanceof InterruptedException) {
       return QUERY_INTERRUPTED;
-    } else if (this.getCause() instanceof CancellationException) {
+    } else if (e instanceof CancellationException) {
       return QUERY_CANCELLED;
-    } else if (this.getCause() instanceof TimeoutException) {
+    } else if (e instanceof TimeoutException) {
       return QUERY_TIMEOUT;
     } else {
       return UNKNOWN_EXCEPTION;
     }
   }
 
-  @JsonProperty("causeMessage")
-  public String getCauseMessage()
+  private static String getErrorClassFromThrowable(Throwable e)
   {
-    return causeMessage;
+    if (e instanceof QueryInterruptedException) {
+      return ((QueryInterruptedException) e).getErrorClass();
+    } else if (e != null) {
+      return e.getClass().getName();
+    } else {
+      return null;
+    }
   }
 
-  @JsonProperty("host")
-  public String getHost()
+  private static String getHostFromThrowable(Throwable e)
   {
-    return host;
-  }
-
-  public boolean isNotKnown()
-  {
-    return !listKnownException.contains(getMessage());
+    if (e instanceof QueryInterruptedException) {
+      return ((QueryInterruptedException) e).getHost();
+    } else {
+      return null;
+    }
   }
 }

processing/src/test/java/io/druid/query/ChainedExecutionQueryRunnerTest.java

@@ -281,7 +281,7 @@ public class ChainedExecutionQueryRunnerTest
     }
     catch (ExecutionException e) {
       Assert.assertTrue(e.getCause() instanceof QueryInterruptedException);
-      Assert.assertEquals("Query timeout", e.getCause().getMessage());
+      Assert.assertEquals("Query timeout", ((QueryInterruptedException) e.getCause()).getErrorCode());
       cause = (QueryInterruptedException) e.getCause();
     }
     queriesInterrupted.await();

processing/src/test/java/io/druid/query/QueryInterruptedExceptionTest.java

@@ -0,0 +1,195 @@
+/*
+ * Licensed to Metamarkets Group Inc. (Metamarkets) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. Metamarkets licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package io.druid.query;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Throwables;
+import com.metamx.common.ISE;
+import io.druid.jackson.DefaultObjectMapper;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.TimeoutException;
+
+public class QueryInterruptedExceptionTest
+{
+  private static final ObjectMapper MAPPER = new DefaultObjectMapper();
+
+  @Test
+  public void testErrorCode()
+  {
+    Assert.assertEquals(
+        "Query cancelled",
+        new QueryInterruptedException(new QueryInterruptedException(new CancellationException())).getErrorCode()
+    );
+    Assert.assertEquals("Query cancelled", new QueryInterruptedException(new CancellationException()).getErrorCode());
+    Assert.assertEquals("Query interrupted", new QueryInterruptedException(new InterruptedException()).getErrorCode());
+    Assert.assertEquals("Query timeout", new QueryInterruptedException(new TimeoutException()).getErrorCode());
+    Assert.assertEquals("Unknown exception", new QueryInterruptedException(null).getErrorCode());
+    Assert.assertEquals("Unknown exception", new QueryInterruptedException(new ISE("Something bad!")).getErrorCode());
+    Assert.assertEquals(
+        "Unknown exception",
+        new QueryInterruptedException(new QueryInterruptedException(new ISE("Something bad!"))).getErrorCode()
+    );
+  }
+
+  @Test
+  public void testErrorMessage()
+  {
+    Assert.assertEquals(
+        null,
+        new QueryInterruptedException(new QueryInterruptedException(new CancellationException())).getMessage()
+    );
+    Assert.assertEquals(
+        null,
+        new QueryInterruptedException(new CancellationException()).getMessage()
+    );
+    Assert.assertEquals(
+        null,
+        new QueryInterruptedException(new InterruptedException()).getMessage()
+    );
+    Assert.assertEquals(
+        null,
+        new QueryInterruptedException(new TimeoutException()).getMessage()
+    );
+    Assert.assertEquals(
+        null,
+        new QueryInterruptedException(null).getMessage()
+    );
+    Assert.assertEquals(
+        "Something bad!",
+        new QueryInterruptedException(new ISE("Something bad!")).getMessage()
+    );
+    Assert.assertEquals(
+        "Something bad!",
+        new QueryInterruptedException(new QueryInterruptedException(new ISE("Something bad!"))).getMessage()
+    );
+  }
+
+  @Test
+  public void testErrorClass()
+  {
+    Assert.assertEquals(
+        "java.util.concurrent.CancellationException",
+        new QueryInterruptedException(new QueryInterruptedException(new CancellationException())).getErrorClass()
+    );
+    Assert.assertEquals(
+        "java.util.concurrent.CancellationException",
+        new QueryInterruptedException(new CancellationException()).getErrorClass()
+    );
+    Assert.assertEquals(
+        "java.lang.InterruptedException",
+        new QueryInterruptedException(new InterruptedException()).getErrorClass()
+    );
+    Assert.assertEquals(
+        "java.util.concurrent.TimeoutException",
+        new QueryInterruptedException(new TimeoutException()).getErrorClass()
+    );
+    Assert.assertEquals(
+        null,
+        new QueryInterruptedException(null).getErrorClass()
+    );
+    Assert.assertEquals(
+        "com.metamx.common.ISE",
+        new QueryInterruptedException(new ISE("Something bad!")).getErrorClass()
+    );
+    Assert.assertEquals(
+        "com.metamx.common.ISE",
+        new QueryInterruptedException(new QueryInterruptedException(new ISE("Something bad!"))).getErrorClass()
+    );
+  }
+
+  @Test
+  public void testHost()
+  {
+    Assert.assertEquals(
+        "myhost",
+        new QueryInterruptedException(new QueryInterruptedException(new CancellationException(), "myhost")).getHost()
+    );
+  }
+
+  @Test
+  public void testSerde()
+  {
+    Assert.assertEquals(
+        "Query cancelled",
+        roundTrip(new QueryInterruptedException(new QueryInterruptedException(new CancellationException()))).getErrorCode()
+    );
+    Assert.assertEquals(
+        "java.util.concurrent.CancellationException",
+        roundTrip(new QueryInterruptedException(new QueryInterruptedException(new CancellationException()))).getErrorClass()
+    );
+    Assert.assertEquals(
+        null,
+        roundTrip(new QueryInterruptedException(new QueryInterruptedException(new CancellationException()))).getMessage()
+    );
+    Assert.assertEquals(
+        "java.util.concurrent.CancellationException",
+        roundTrip(new QueryInterruptedException(new CancellationException())).getErrorClass()
+    );
+    Assert.assertEquals(
+        "java.lang.InterruptedException",
+        roundTrip(new QueryInterruptedException(new InterruptedException())).getErrorClass()
+    );
+    Assert.assertEquals(
+        "java.util.concurrent.TimeoutException",
+        roundTrip(new QueryInterruptedException(new TimeoutException())).getErrorClass()
+    );
+    Assert.assertEquals(
+        null,
+        roundTrip(new QueryInterruptedException(null)).getErrorClass()
+    );
+    Assert.assertEquals(
+        "com.metamx.common.ISE",
+        roundTrip(new QueryInterruptedException(new ISE("Something bad!"))).getErrorClass()
+    );
+    Assert.assertEquals(
+        "com.metamx.common.ISE",
+        roundTrip(new QueryInterruptedException(new QueryInterruptedException(new ISE("Something bad!")))).getErrorClass()
+    );
+    Assert.assertEquals(
+        "Something bad!",
+        roundTrip(new QueryInterruptedException(new ISE("Something bad!"))).getMessage()
+    );
+    Assert.assertEquals(
+        "Something bad!",
+        roundTrip(new QueryInterruptedException(new QueryInterruptedException(new ISE("Something bad!")))).getMessage()
+    );
+    Assert.assertEquals(
+        "Unknown exception",
+        roundTrip(new QueryInterruptedException(new ISE("Something bad!"))).getErrorCode()
+    );
+    Assert.assertEquals(
+        "Unknown exception",
+        roundTrip(new QueryInterruptedException(new QueryInterruptedException(new ISE("Something bad!")))).getErrorCode()
+    );
+  }
+
+  private static QueryInterruptedException roundTrip(final QueryInterruptedException e)
+  {
+    try {
+      return MAPPER.readValue(MAPPER.writeValueAsBytes(e), QueryInterruptedException.class);
+    }
+    catch (Exception e2) {
+      throw Throwables.propagate(e2);
+    }
+  }
+}

server/src/main/java/io/druid/client/DirectDruidClient.java

@@ -478,13 +478,7 @@ public class DirectDruidClient<T> implements QueryRunner<T>
           final JsonToken nextToken = jp.nextToken();
           if (nextToken == JsonToken.START_OBJECT) {
             QueryInterruptedException cause = jp.getCodec().readValue(jp, QueryInterruptedException.class);
-            //case we get an exception with an unknown message.
-            if (cause.isNotKnown()) {
-              throw new QueryInterruptedException(QueryInterruptedException.UNKNOWN_EXCEPTION, cause.getMessage(), host);
-            } else {
             throw new QueryInterruptedException(cause, host);
-            }
-
           } else if (nextToken != JsonToken.START_ARRAY) {
             throw new IAE("Next token wasn't a START_ARRAY, was[%s] from url [%s]", jp.getCurrentToken(), url);
           } else {

server/src/main/java/io/druid/server/QueryResource.java

@@ -345,11 +345,7 @@ public class QueryResource
         log.error(e2, "Unable to log query [%s]!", query);
       }
       return Response.serverError().type(contentType).entity(
-          jsonWriter.writeValueAsBytes(
-              ImmutableMap.of(
-                  "error", e.getMessage() == null ? "null exception" : e.getMessage()
-              )
-          )
+          jsonWriter.writeValueAsBytes(new QueryInterruptedException(e))
       ).build();
     }
     catch (Exception e) {
@@ -395,11 +391,7 @@ public class QueryResource
          .emit();
 
       return Response.serverError().type(contentType).entity(
-          jsonWriter.writeValueAsBytes(
-              ImmutableMap.of(
-                  "error", e.getMessage() == null ? "null exception" : e.getMessage()
-              )
-          )
+          jsonWriter.writeValueAsBytes(new QueryInterruptedException(e))
       ).build();
     }
     finally {

server/src/test/java/io/druid/client/DirectDruidClientTest.java

@@ -20,7 +20,6 @@
 package io.druid.client;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.util.concurrent.Futures;
@@ -314,7 +313,7 @@ public class DirectDruidClientTest
 
     TimeBoundaryQuery query = Druids.newTimeBoundaryQueryBuilder().dataSource("test").build();
     HashMap<String, List> context = Maps.newHashMap();
-    interruptionFuture.set(new ByteArrayInputStream("{\"error\":\"testing\"}".getBytes()));
+    interruptionFuture.set(new ByteArrayInputStream("{\"error\":\"testing1\",\"errorMessage\":\"testing2\"}".getBytes()));
     Sequence results = client1.run(query, context);
 
     QueryInterruptedException actualException = null;
@@ -325,9 +324,9 @@ public class DirectDruidClientTest
       actualException = e;
     }
     Assert.assertNotNull(actualException);
-    Assert.assertEquals(actualException.getMessage(), QueryInterruptedException.UNKNOWN_EXCEPTION);
-    Assert.assertEquals(actualException.getCauseMessage(), "testing");
-    Assert.assertEquals(actualException.getHost(), hostName);
+    Assert.assertEquals("testing1", actualException.getErrorCode());
+    Assert.assertEquals("testing2", actualException.getMessage());
+    Assert.assertEquals(hostName, actualException.getHost());
     EasyMock.verify(httpClient);
   }
 }