FIR-33627: json formatted server error (#426)

src/integrationTest/java/integration/tests/StatementTest.java

@@ -3,6 +3,7 @@
 import com.firebolt.jdbc.connection.FireboltConnection;
 import com.firebolt.jdbc.exception.FireboltException;
 import integration.ConnectionInfo;
+import integration.EnvironmentCondition;
 import integration.IntegrationTest;
 import kotlin.collections.ArrayDeque;
 import org.hamcrest.Matchers;
@@ -99,6 +100,25 @@ void shouldThrowExceptionWhenTryingToReuseStatementClosedOnCompletion() throws S
 		}
 	}
 
+	@Test
+	void shouldThrowExceptionWhenExecutingWrongQuery() throws SQLException {
+		try (Connection connection = createConnection(); Statement statement = connection.createStatement()) {
+			String errorMessage = assertThrows(FireboltException.class, () -> statement.executeQuery("select wrong query")).getMessage();
+			assertTrue(errorMessage.contains("Column 'wrong' does not exist."));
+		}
+	}
+
+	@Test
+	@EnvironmentCondition(value = "4.2.0", comparison = EnvironmentCondition.Comparison.GE)
+	void shouldThrowExceptionWhenExecutingWrongQueryWithJsonError() throws SQLException {
+		try (Connection connection = createConnection(); Statement statement = connection.createStatement()) {
+			statement.execute("set advanced_mode=1");
+			statement.execute("set enable_json_error_output_format=true");
+			String errorMessage = assertThrows(FireboltException.class, () -> statement.executeQuery("select wrong query")).getMessage();
+			assertTrue(errorMessage.contains("Column 'wrong' does not exist."));
+		}
+	}
+
 	@Test
 	void shouldReturnTrueWhenExecutingAStatementThatReturnsAResultSet() throws SQLException {
 		try (Connection connection = createConnection(); Statement statement = connection.createStatement()) {

src/integrationTest/java/integration/tests/SystemEngineTest.java

@@ -38,6 +38,7 @@
 import java.util.stream.Stream;
 
 import static com.firebolt.jdbc.connection.FireboltConnectionUserPassword.SYSTEM_ENGINE_NAME;
+import static integration.EnvironmentCondition.Attribute.fireboltVersion;
 import static java.lang.String.format;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Map.entry;
@@ -91,6 +92,25 @@ void shouldSelect1() throws SQLException {
 		}
 	}
 
+	@Test
+	void shouldThrowExceptionWhenExecutingWrongQuery() throws SQLException {
+		try (Connection connection = createConnection(getSystemEngineName()); Statement statement = connection.createStatement()) {
+			String errorMessage = assertThrows(FireboltException.class, () -> statement.executeQuery("select wrong query")).getMessage();
+			assertTrue(errorMessage.contains("Column 'wrong' does not exist."));
+		}
+	}
+
+	@Test
+	@EnvironmentCondition(value = "4.2.0", attribute = fireboltVersion, comparison = EnvironmentCondition.Comparison.GE)
+	void shouldThrowExceptionWhenExecutingWrongQueryWithJsonError() throws SQLException {
+		try (Connection connection = createConnection(getSystemEngineName()); Statement statement = connection.createStatement()) {
+			statement.execute("set advanced_mode=1");
+			statement.execute("set enable_json_error_output_format=true");
+			String errorMessage = assertThrows(FireboltException.class, () -> statement.executeQuery("select wrong query")).getMessage();
+			assertTrue(errorMessage.contains("Column 'wrong' does not exist."));
+		}
+	}
+
 	@Test
 	void shouldFailToSelectFromCustomDbUsingSystemEngine() throws SQLException {
 		ConnectionInfo current = integration.ConnectionInfo.getInstance();
@@ -103,7 +123,7 @@ void shouldFailToSelectFromCustomDbUsingSystemEngine() throws SQLException {
 		Collection<String> expectedErrorMessages = Set.of(
 				"Queries against table dummy require a user engine",
 				"The system engine doesn't support queries against table dummy. Run this query on a user engine.",
-				"Line 1, Column 22: relation \"dummy\" does not exist");
+				"relation \"dummy\" does not exist");
 
 		try (Connection systemConnection = DriverManager.getConnection(systemEngineJdbcUrl, principal, secret);
 			 Connection customConnection = DriverManager.getConnection(customEngineJdbcUrl, principal, secret)) {

src/main/java/com/firebolt/jdbc/client/FireboltClient.java

@@ -3,6 +3,8 @@
 import com.firebolt.jdbc.connection.CacheListener;
 import com.firebolt.jdbc.connection.FireboltConnection;
 import com.firebolt.jdbc.exception.FireboltException;
+import com.firebolt.jdbc.exception.ServerError;
+import com.firebolt.jdbc.exception.ServerError.Error.Location;
 import com.firebolt.jdbc.resultset.compress.LZ4InputStream;
 import com.firebolt.jdbc.util.CloseableUtil;
 import lombok.Getter;
@@ -13,6 +15,7 @@
 import okhttp3.Request;
 import okhttp3.RequestBody;
 import okhttp3.Response;
+import org.json.JSONException;
 import org.json.JSONObject;
 
 import java.io.BufferedReader;
@@ -24,13 +27,17 @@
 import java.nio.charset.StandardCharsets;
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import static java.lang.String.format;
@@ -46,6 +53,7 @@ public abstract class FireboltClient implements CacheListener {
 	private static final String HEADER_USER_AGENT = "User-Agent";
 	private static final String HEADER_PROTOCOL_VERSION = "Firebolt-Protocol-Version";
 	private static final Logger log = Logger.getLogger(FireboltClient.class.getName());
+	private static final Pattern plainErrorPattern = Pattern.compile("Line (\\d+), Column (\\d+): (.*)$", Pattern.MULTILINE);
 	private final OkHttpClient httpClient;
 	private final String headerUserAgentValue;
 	protected final FireboltConnection connection;
@@ -142,17 +150,19 @@ protected void validateResponse(String host, Response response, Boolean isCompre
 				throw new FireboltException(format("Could not query Firebolt at %s. The engine is not running.", host), statusCode);
 			}
 			String errorMessageFromServer = extractErrorMessage(response, isCompress);
-			validateResponse(host, statusCode, errorMessageFromServer);
+			ServerError serverError = parseServerError(errorMessageFromServer);
+			String processedErrorMessage = serverError.getErrorMessage();
+			validateResponse(host, statusCode, processedErrorMessage);
 			String errorResponseMessage = format(
 					"Server failed to execute query with the following error:%n%s%ninternal error:%n%s",
-					errorMessageFromServer, getInternalErrorWithHeadersText(response));
+					processedErrorMessage, getInternalErrorWithHeadersText(response));
 			if (statusCode == HTTP_UNAUTHORIZED) {
 				getConnection().removeExpiredTokens();
 				throw new FireboltException(format(
 						"Could not query Firebolt at %s. The operation is not authorized or the token is expired and has been cleared from the cache.%n%s",
-						host, errorResponseMessage), statusCode, errorMessageFromServer);
+						host, errorResponseMessage), statusCode, processedErrorMessage);
 			}
-			throw new FireboltException(errorResponseMessage, statusCode, errorMessageFromServer);
+			throw new FireboltException(errorResponseMessage, statusCode, processedErrorMessage);
 		}
 	}
 
@@ -194,6 +204,50 @@ private String extractErrorMessage(Response response, boolean isCompress) throws
 		return new String(entityBytes, StandardCharsets.UTF_8);
 	}
 
+	private ServerError parseServerError(String responseText) {
+		try {
+			if (responseText == null) {
+				return new ServerError(null, null);
+			}
+			ServerError serverError = new ServerError(new JSONObject(responseText));
+			ServerError.Error[] errors = serverError.getErrors() == null ? null : Arrays.stream(serverError.getErrors()).map(this::updateError).toArray(ServerError.Error[]::new);
+			return new ServerError(serverError.getQuery(), errors);
+		} catch (JSONException e) {
+			String message = responseText;
+			Location location = null;
+			Entry<Location, String> locationAndText = getLocationFromMessage(responseText);
+			if (locationAndText != null) {
+				location = locationAndText.getKey();
+				message = locationAndText.getValue();
+			}
+			return new ServerError(null, new ServerError.Error[] {new ServerError.Error(null, message, null, null, null, null, null, location)});
+		}
+	}
+
+	private ServerError.Error updateError(ServerError.Error error) {
+		if (error == null || error.getDescription() == null) {
+			return error;
+		}
+		Entry<Location, String> locationAndText = getLocationFromMessage(error.getDescription());
+		if (locationAndText == null) {
+			return error;
+		}
+		Location location = Objects.requireNonNullElse(error.getLocation(), locationAndText.getKey());
+		return new ServerError.Error(error.getCode(), error.getName(), error.getSeverity(), error.getSource(),
+				locationAndText.getValue(), error.getResolution(), error.getHelpLink(), location);
+	}
+
+	private Entry<Location, String> getLocationFromMessage(String responseText) {
+		Matcher m = plainErrorPattern.matcher(responseText);
+		if (m.find()) {
+			int line = Integer.parseInt(m.group(1));
+			int column = Integer.parseInt(m.group(2));
+			String message = m.group(3);
+			return Map.entry(new Location(line, column, column), message);
+		}
+		return null;
+	}
+
 	protected boolean isCallSuccessful(int statusCode) {
 		return statusCode >= 200 && statusCode <= 299; // Call is considered successful when the status code is 2XX
 	}