-
Notifications
You must be signed in to change notification settings - Fork 3.8k
More useful query errors. #3335
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,88 +21,125 @@ | |
|
|
||
| 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"; | ||
| public static final String QUERY_TIMEOUT = "Query timeout"; | ||
| 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; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, this should probably be kept for rolling-update purposes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. causeMessage was dropped by the broker previously, so I didn't think it was important to keep it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It also wasn't written (the old QueryResource only serialized the message, not the whole exception)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure ok |
||
| @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) { | ||
| return super.getMessage(); | ||
| } | ||
|
|
||
| @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; | ||
| } | ||
|
|
||
| @JsonProperty("host") | ||
| public String getHost() | ||
| { | ||
| return host; | ||
| if (e instanceof QueryInterruptedException) { | ||
| return ((QueryInterruptedException) e).getErrorClass(); | ||
| } else if (e != null) { | ||
| return e.getClass().getName(); | ||
| } else { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| public boolean isNotKnown() | ||
| private static String getHostFromThrowable(Throwable e) | ||
| { | ||
| return !listKnownException.contains(getMessage()); | ||
| if (e instanceof QueryInterruptedException) { | ||
| return ((QueryInterruptedException) e).getHost(); | ||
| } else { | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we rename this to QueryFailedException or something more meaningful :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe its not renamed to maintain backwards compatibility ?