From 8b83cf067c1314b6408892c81c7e00f5a86432e2 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Wed, 12 Oct 2016 08:14:06 +0200 Subject: [PATCH] Watcher: Ensure awesome painless exceptions are propagated to the user (elastic/elasticsearch#3707) When adding a watch which has a painless component, the scriptexception was wrapped into a deprecated exception which means, that the awesome painless descriptions were lost. This wrapping has been removed. Closes elastic/elasticsearch#3161 Original commit: elastic/x-pack-elasticsearch@1703fe4eb6151d8cdfa6588d7f06f1ade6f67ad7 --- .../script/ExecutableScriptCondition.java | 10 ++---- .../script/ScriptConditionTests.java | 6 ++-- .../HttpSecretsIntegrationTests.java | 20 +++-------- .../10_basic.yaml | 0 .../20_minimal_body.yaml | 0 .../30_inline_watch.yaml | 0 .../test/watcher_painless/40_exception.yaml | 36 +++++++++++++++++++ 7 files changed, 47 insertions(+), 25 deletions(-) rename qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/{watcher_groovy => watcher_painless}/10_basic.yaml (100%) rename qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/{watcher_groovy => watcher_painless}/20_minimal_body.yaml (100%) rename qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/{watcher_groovy => watcher_painless}/30_inline_watch.yaml (100%) create mode 100644 qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/40_exception.yaml diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/condition/script/ExecutableScriptCondition.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/condition/script/ExecutableScriptCondition.java index bec88e6a726..e948e80ad66 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/condition/script/ExecutableScriptCondition.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/watcher/condition/script/ExecutableScriptCondition.java @@ -31,13 +31,9 @@ public class ExecutableScriptCondition extends ExecutableCondition conditionParser.createExecutable(scriptCondition)); } @@ -182,9 +182,9 @@ public class ScriptConditionTests extends ESTestCase { XContentParser parser = XContentFactory.xContent(builder.bytes()).createParser(builder.bytes()); parser.nextToken(); ScriptCondition scriptCondition = conditionParser.parseCondition("_watch", parser, false); - GeneralScriptException exception = expectThrows(GeneralScriptException.class, + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> conditionParser.createExecutable(scriptCondition)); - assertThat(exception.getMessage(), containsString("script_lang not supported [not_a_valid_lang]]")); + assertThat(exception.getMessage(), containsString("script_lang not supported [not_a_valid_lang]")); } public void testScriptConditionThrowException() throws Exception { diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/test/integration/HttpSecretsIntegrationTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/test/integration/HttpSecretsIntegrationTests.java index 46dd8456719..107463ed85a 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/test/integration/HttpSecretsIntegrationTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/test/integration/HttpSecretsIntegrationTests.java @@ -8,16 +8,15 @@ package org.elasticsearch.xpack.watcher.test.integration; import com.squareup.okhttp.mockwebserver.MockResponse; import com.squareup.okhttp.mockwebserver.MockWebServer; import com.squareup.okhttp.mockwebserver.RecordedRequest; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.support.XContentMapValues; -import org.elasticsearch.xpack.security.crypto.CryptoService; -import org.elasticsearch.xpack.watcher.client.WatcherClient; -import org.elasticsearch.xpack.watcher.execution.ActionExecutionMode; import org.elasticsearch.xpack.common.http.HttpRequestTemplate; import org.elasticsearch.xpack.common.http.auth.basic.ApplicableBasicAuth; import org.elasticsearch.xpack.common.http.auth.basic.BasicAuth; +import org.elasticsearch.xpack.security.crypto.CryptoService; +import org.elasticsearch.xpack.watcher.client.WatcherClient; +import org.elasticsearch.xpack.watcher.execution.ActionExecutionMode; import org.elasticsearch.xpack.watcher.support.xcontent.XContentSource; import org.elasticsearch.xpack.watcher.test.AbstractWatcherIntegrationTestCase; import org.elasticsearch.xpack.watcher.transport.actions.execute.ExecuteWatchResponse; @@ -29,7 +28,6 @@ import org.joda.time.DateTime; import org.junit.After; import org.junit.Before; -import java.net.BindException; import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -62,16 +60,8 @@ public class HttpSecretsIntegrationTests extends AbstractWatcherIntegrationTestC @Before public void init() throws Exception { - for (int webPort = 9200; webPort < 9300; webPort++) { - try { - webServer = new MockWebServer(); - webServer.start(webPort); - return; - } catch (BindException be) { - logger.warn("port [{}] was already in use trying next port", webPort); - } - } - throw new ElasticsearchException("unable to find open port between 9200 and 9300"); + webServer = new MockWebServer(); + webServer.start(); } @After diff --git a/qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_groovy/10_basic.yaml b/qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/10_basic.yaml similarity index 100% rename from qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_groovy/10_basic.yaml rename to qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/10_basic.yaml diff --git a/qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_groovy/20_minimal_body.yaml b/qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/20_minimal_body.yaml similarity index 100% rename from qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_groovy/20_minimal_body.yaml rename to qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/20_minimal_body.yaml diff --git a/qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_groovy/30_inline_watch.yaml b/qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/30_inline_watch.yaml similarity index 100% rename from qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_groovy/30_inline_watch.yaml rename to qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/30_inline_watch.yaml diff --git a/qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/40_exception.yaml b/qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/40_exception.yaml new file mode 100644 index 00000000000..1928ee5ba1a --- /dev/null +++ b/qa/smoke-test-watcher-with-painless/src/test/resources/rest-api-spec/test/watcher_painless/40_exception.yaml @@ -0,0 +1,36 @@ +--- +"Test awesome painless exceptions are returned including the script_stack field": + - do: + cluster.health: + wait_for_status: green + + - do: + catch: request + xpack.watcher.put_watch: + id: "my_exe_watch" + body: > + { + "trigger" : { + "schedule" : { "cron" : "0 0 0 1 * ? 2099" } + }, + "input" : { + "simple" : {} + }, + "condition" : { + "script" : { + "inline" : "FOO == 1", + "lang" : "painless" + } + }, + "actions" : { + "email_admin" : { + "email" : { + "to" : "someone@domain.host.com", + "subject" : "404 recently encountered" + } + } + } + } + + - is_true: error.script_stack + - match: { status: 500 }