Skip to content
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

Improves error messages around talking to Druid #61

Merged
merged 1 commit into from
Oct 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ Current

### Changed:

- [Improves error messages when querying Druid goes wrong](https:://github.com/yahoo/fili/pull/61)
- The `ResponseException` now includes a message that prints the `ResponseException`'s internal state
(i.e. the druid query and response code) using the error messages
`ErrorMessageFormat::FAILED_TO_SEND_QUERY_TO_DRUID` and `ErrorMessageFormat::ERROR_FROM_DRUID`
- The druid query and status code, reason and response body are now logged at the error level in the
failure and error callbacks in `AsyncDruidWebServiceImpl`

- [Fili now supports custom Druid query types](https://github.com/yahoo/fili/pull/57)
* `QueryType` has been turned into an interface, backed by an enum `DefaultQueryType`.
- The default implementations of `DruidResponseParser` `DruidQueryBuilder`, `WeightEvaluationQuery` and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,42 @@ public ResponseException(
DruidQuery<?> druidQuery,
Throwable cause
) {
super(buildMessage(reason, description, statusCode, druidQuery, cause));
this.statusCode = statusCode;
this.reason = reason;
this.description = description;
this.druidQuery = druidQuery;
this.cause = cause;
}

/**
* Builds a message for this exception that is basically a straightforward serialization of its interesting state.
*
* @param reason The reason for the error
* @param description A description of the error
* @param statusCode The status code received from Druid
* @param druidQuery The druid query that triggered the invalid response
* @param cause The cause of this exception, if any
*
* @return A Stringification of the parameters to serve as this exception's message
*/
private static String buildMessage(
String reason,
String description,
int statusCode,
DruidQuery<?> druidQuery,
Throwable cause
) {
return String.format(
"Reason: %s, Description: %s, statusCode: %d, druid query: %s, cause: %s",
reason,
description,
statusCode,
druidQuery,
cause
);
}

public String getReason() {
return reason;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ public enum ErrorMessageFormat implements MessageFormatter {
INVALID_ASYNC_AFTER( "Invalid 'asyncAfter' parameter: '%s'. 'asyncAfter' must be either 'never' or an integer number of milliseconds."),

FILTER_JOBFIELD_UNDEFINED("Filter field '%s' does not exist. The possible fields to filter on are '%s'"),

FAILED_TO_SEND_QUERY_TO_DRUID("Failed to retrieve data.", "Failed to send the query %s to Druid."),

ERROR_FROM_DRUID("Failed to retrieve data.", "Received %s with status code %s for reason %s when sending %s to Druid"),

;
// CHECKSTYLE:ON

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
import com.yahoo.bard.webservice.druid.model.query.DruidAggregationQuery;
import com.yahoo.bard.webservice.async.ResponseException;
import com.yahoo.bard.webservice.web.DataApiRequest;
import com.yahoo.bard.webservice.web.ErrorMessageFormat;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import rx.subjects.Subject;

Expand All @@ -27,6 +31,7 @@
* Response Processor which will perform result set mapping.
*/
public abstract class MappingResponseProcessor implements ResponseProcessor {
private static final Logger LOG = LoggerFactory.getLogger(MappingResponseProcessor.class);

protected final DataApiRequest apiRequest;
protected final ResponseContext responseContext;
Expand Down Expand Up @@ -102,6 +107,7 @@ public HttpErrorCallback getStandardError(
return new HttpErrorCallback() {
@Override
public void invoke(int statusCode, String reason, String responseBody) {
LOG.error(ErrorMessageFormat.ERROR_FROM_DRUID.logFormat(responseBody, statusCode, reason, druidQuery));
responseEmitter.onError(new ResponseException(statusCode, reason, responseBody, druidQuery));
}
};
Expand All @@ -122,6 +128,7 @@ public FailureCallback getStandardFailure(
return new FailureCallback() {
@Override
public void invoke(Throwable error) {
LOG.error(ErrorMessageFormat.FAILED_TO_SEND_QUERY_TO_DRUID.logFormat(druidQuery), error);
responseEmitter.onError(new ResponseException(Status.INTERNAL_SERVER_ERROR, druidQuery, error));
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ class ResultSetResponseProcessorSpec extends Specification {
entity.contains("message1234")
}


def "Test error callback"() {
setup:
def resultSetResponseProcessor = new ResultSetResponseProcessor(
Expand Down