Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Fetch error message in root cause for new default formatter #1001

Merged
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package com.amazon.opendistroforelasticsearch.sql.elasticsearch.response.error;

import lombok.Getter;
import org.elasticsearch.rest.RestStatus;
import org.json.JSONObject;

Expand All @@ -25,17 +26,23 @@
*/
public class ErrorMessage {

protected Exception exception;
protected Throwable exception;

private int status;
private String type;
private String reason;
private String details;
private final int status;

@Getter
private final String type;

@Getter
private final String reason;

@Getter
private final String details;

/**
* Error Message Constructor.
*/
public ErrorMessage(Exception exception, int status) {
public ErrorMessage(Throwable exception, int status) {
this.exception = exception;
this.status = status;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class ErrorMessageFactory {
* @param status exception status code
* @return error message
*/
public static ErrorMessage createErrorMessage(Exception e, int status) {
public static ErrorMessage createErrorMessage(Throwable e, int status) {
Throwable cause = unwrapCause(e);
if (cause instanceof ElasticsearchException) {
ElasticsearchException exception = (ElasticsearchException) cause;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import com.amazon.opendistroforelasticsearch.sql.common.antlr.SyntaxCheckException;
import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType;
import com.amazon.opendistroforelasticsearch.sql.elasticsearch.response.error.ErrorMessage;
import com.amazon.opendistroforelasticsearch.sql.elasticsearch.response.error.ErrorMessageFactory;
import com.amazon.opendistroforelasticsearch.sql.exception.QueryEngineException;
import com.amazon.opendistroforelasticsearch.sql.executor.ExecutionEngine.Schema;
import com.amazon.opendistroforelasticsearch.sql.protocol.response.QueryResult;
Expand Down Expand Up @@ -56,11 +58,13 @@ protected Object buildJsonObject(QueryResult response) {

@Override
public String format(Throwable t) {
int status = getStatus(t);
ErrorMessage message = ErrorMessageFactory.createErrorMessage(t, status);
Error error = new Error(
t.getClass().getSimpleName(),
t.getMessage(),
t.getMessage());
return jsonify(new JdbcErrorResponse(error, getStatus(t)));
message.getType(),
message.getReason(),
message.getDetails());
return jsonify(new JdbcErrorResponse(error, status));
}

private Column fetchColumn(Schema.Column col) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.gson.JsonParser;
import java.util.Arrays;
import org.elasticsearch.ElasticsearchException;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -118,7 +119,7 @@ void format_client_error_response_due_to_syntax_exception() {
"{\"error\":"
+ "{\""
+ "type\":\"SyntaxCheckException\","
+ "\"reason\":\"Invalid query syntax\","
+ "\"reason\":\"Invalid Query\","
+ "\"details\":\"Invalid query syntax\""
+ "},"
+ "\"status\":400}",
Expand All @@ -132,7 +133,7 @@ void format_client_error_response_due_to_semantic_exception() {
"{\"error\":"
+ "{\""
+ "type\":\"SemanticCheckException\","
+ "\"reason\":\"Invalid query semantics\","
+ "\"reason\":\"Invalid Query\","
+ "\"details\":\"Invalid query semantics\""
+ "},"
+ "\"status\":400}",
Expand All @@ -146,14 +147,33 @@ void format_server_error_response() {
"{\"error\":"
+ "{\""
+ "type\":\"IllegalStateException\","
+ "\"reason\":\"Execution error\","
+ "\"reason\":\"There was internal problem at backend\","
+ "\"details\":\"Execution error\""
+ "},"
+ "\"status\":503}",
formatter.format(new IllegalStateException("Execution error"))
);
}

@Test
void format_server_error_response_due_to_elasticsearch() {
assertJsonEquals(
"{\"error\":"
+ "{\""
+ "type\":\"ElasticsearchException\","
+ "\"reason\":\"Error occurred in Elasticsearch engine: all shards failed\","
+ "\"details\":\"ElasticsearchException[all shards failed]; "
+ "nested: IllegalStateException[Execution error];; "
+ "java.lang.IllegalStateException: Execution error\\n"
+ "For more details, please send request for Json format to see the raw response "
+ "from elasticsearch engine.\""
+ "},"
+ "\"status\":503}",
formatter.format(new ElasticsearchException("all shards failed",
new IllegalStateException("Execution error")))
);
}

private static void assertJsonEquals(String expected, String actual) {
assertEquals(
JsonParser.parseString(expected),
Expand Down