From 9567f337f5735ee21aa54c76433d764ea6b525e2 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Wed, 10 Jul 2019 14:10:15 +0300 Subject: [PATCH] SQL: handle SQL not being available in a more graceful way (#43665) * Add test for SQL not being available error message in JDBC. * Add a new qa sub-project that explicitly disables SQL XPack module in Gradle. (cherry picked from commit 8a1ac8a3a88a325ec9b99963e0fa288c18ee0ee5) --- x-pack/plugin/sql/qa/no-sql/build.gradle | 5 +++++ .../xpack/sql/qa/no_sql/JdbcNoSqlIT.java | 13 ++++++++++++ .../xpack/sql/qa/jdbc/JdbcNoSqlTestCase.java | 21 +++++++++++++++++++ .../sql/client/JreHttpUrlConnection.java | 9 ++++---- .../xpack/sql/client/RemoteFailure.java | 11 +++++++--- .../xpack/sql/client/RemoteFailureTests.java | 4 ++-- .../resources/remote_failure/bogus_error.json | 2 +- 7 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 x-pack/plugin/sql/qa/no-sql/build.gradle create mode 100644 x-pack/plugin/sql/qa/no-sql/src/test/java/org/elasticsearch/xpack/sql/qa/no_sql/JdbcNoSqlIT.java create mode 100644 x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcNoSqlTestCase.java diff --git a/x-pack/plugin/sql/qa/no-sql/build.gradle b/x-pack/plugin/sql/qa/no-sql/build.gradle new file mode 100644 index 00000000000..d0f8a3007c4 --- /dev/null +++ b/x-pack/plugin/sql/qa/no-sql/build.gradle @@ -0,0 +1,5 @@ +testClusters.integTest { + setting 'xpack.security.enabled', 'false' + setting 'xpack.sql.enabled', 'false' + setting 'xpack.license.self_generated.type', 'trial' +} diff --git a/x-pack/plugin/sql/qa/no-sql/src/test/java/org/elasticsearch/xpack/sql/qa/no_sql/JdbcNoSqlIT.java b/x-pack/plugin/sql/qa/no-sql/src/test/java/org/elasticsearch/xpack/sql/qa/no_sql/JdbcNoSqlIT.java new file mode 100644 index 00000000000..c1c59877fdc --- /dev/null +++ b/x-pack/plugin/sql/qa/no-sql/src/test/java/org/elasticsearch/xpack/sql/qa/no_sql/JdbcNoSqlIT.java @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.qa.no_sql; + +import org.elasticsearch.xpack.sql.qa.jdbc.JdbcNoSqlTestCase; + +public class JdbcNoSqlIT extends JdbcNoSqlTestCase { + +} diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcNoSqlTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcNoSqlTestCase.java new file mode 100644 index 00000000000..1b4e37dbc2b --- /dev/null +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcNoSqlTestCase.java @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.sql.qa.jdbc; + +import java.sql.Connection; +import java.sql.SQLException; + +public class JdbcNoSqlTestCase extends JdbcIntegrationTestCase { + + public void testJdbcExceptionMessage() throws SQLException { + try (Connection c = esJdbc()) { + SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * FROM bla").executeQuery()); + assertTrue(e.getMessage().startsWith("X-Pack/SQL does not seem to be available on the Elasticsearch" + + " node using the access path")); + } + } +} diff --git a/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/JreHttpUrlConnection.java b/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/JreHttpUrlConnection.java index 59a6e82e987..716b1bb058a 100644 --- a/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/JreHttpUrlConnection.java +++ b/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/JreHttpUrlConnection.java @@ -48,8 +48,8 @@ public class JreHttpUrlConnection implements Closeable { * error. */ public static final String SQL_STATE_BAD_SERVER = "bad_server"; - private static final String SQL_NOT_AVAILABLE_ERROR_MESSAGE = "request [" + SQL_QUERY_REST_ENDPOINT - + "] contains unrecognized parameter: [mode]"; + private static final String SQL_NOT_AVAILABLE_ERROR_MESSAGE = "Incorrect HTTP method for uri [" + SQL_QUERY_REST_ENDPOINT + + "?error_trace] and method [POST], allowed:"; public static R http(String path, String query, ConnectionConfiguration cfg, Function handler) { final URI uriPath = cfg.baseUri().resolve(path); // update path if needed @@ -181,9 +181,8 @@ public class JreHttpUrlConnection implements Closeable { if (type == null) { // check if x-pack or sql are not available (x-pack not installed or sql not enabled) // by checking the error message the server is sending back - if (con.getResponseCode() >= HttpURLConnection.HTTP_BAD_REQUEST - && failure.reason().contains(SQL_NOT_AVAILABLE_ERROR_MESSAGE)) { - return new ResponseOrException<>(new SQLException("X-Pack/SQL do not seem to be available" + if (con.getResponseCode() >= HttpURLConnection.HTTP_BAD_REQUEST && failure.reason().contains(SQL_NOT_AVAILABLE_ERROR_MESSAGE)) { + return new ResponseOrException<>(new SQLException("X-Pack/SQL does not seem to be available" + " on the Elasticsearch node using the access path '" + con.getURL().getHost() + (con.getURL().getPort() > 0 ? ":" + con.getURL().getPort() : "") diff --git a/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/RemoteFailure.java b/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/RemoteFailure.java index 61e62c390ec..aca3003d668 100644 --- a/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/RemoteFailure.java +++ b/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/RemoteFailure.java @@ -155,10 +155,15 @@ public class RemoteFailure { } else { switch (fieldName) { case "error": - if (token != JsonToken.START_OBJECT) { - throw new IOException("Expected [error] to be an object but was [" + token + "][" + parser.getText() + "]"); + if (token != JsonToken.START_OBJECT && token != JsonToken.VALUE_STRING) { + throw new IOException("Expected [error] to be an object or string but was [" + token + "][" + + parser.getText() + "]"); + } + if (token == JsonToken.VALUE_STRING) { + exception = new RemoteFailure(StringUtils.EMPTY, parser.getText(), null, null, null, null); + } else { + exception = parseFailure(parser); } - exception = parseFailure(parser); continue; case "status": if (token != JsonToken.VALUE_NUMBER_INT) { diff --git a/x-pack/plugin/sql/sql-client/src/test/java/org/elasticsearch/xpack/sql/client/RemoteFailureTests.java b/x-pack/plugin/sql/sql-client/src/test/java/org/elasticsearch/xpack/sql/client/RemoteFailureTests.java index ee3a859b548..2029493bcbe 100644 --- a/x-pack/plugin/sql/sql-client/src/test/java/org/elasticsearch/xpack/sql/client/RemoteFailureTests.java +++ b/x-pack/plugin/sql/sql-client/src/test/java/org/elasticsearch/xpack/sql/client/RemoteFailureTests.java @@ -70,9 +70,9 @@ public class RemoteFailureTests extends ESTestCase { public void testBogusError() { IOException e = expectThrows(IOException.class, () -> parse("bogus_error.json")); assertEquals( - "Can't parse error from Elasticsearch [Expected [error] to be an object but was [VALUE_STRING][bogus]] " + "Can't parse error from Elasticsearch [Expected [error] to be an object or string but was [START_ARRAY][[]] " + "at [line 1 col 12]. Response:\n" - + "{ \"error\": \"bogus\" }", + + "{ \"error\": [\"bogus\"] }", e.getMessage()); } diff --git a/x-pack/plugin/sql/sql-client/src/test/resources/remote_failure/bogus_error.json b/x-pack/plugin/sql/sql-client/src/test/resources/remote_failure/bogus_error.json index f79361cec1c..49c31ca6f54 100644 --- a/x-pack/plugin/sql/sql-client/src/test/resources/remote_failure/bogus_error.json +++ b/x-pack/plugin/sql/sql-client/src/test/resources/remote_failure/bogus_error.json @@ -1 +1 @@ -{ "error": "bogus" } \ No newline at end of file +{ "error": ["bogus"] } \ No newline at end of file