diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParser.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParser.java index b2b4167d911..7e49a8ce13b 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParser.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParser.java @@ -6,8 +6,6 @@ package org.elasticsearch.xpack.notification.email.attachment; import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.message.ParameterizedMessage; -import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.ParseField; @@ -102,25 +100,22 @@ public class HttpEmailAttachementParser implements EmailAttachmentParser) () -> new ParameterizedMessage( - "Error executing HTTP request: [host[{}], port[{}], method[{}], path[{}]", - httpRequest.host(), - httpRequest.port(), - httpRequest.method(), - httpRequest.path()), - e); + throw new ElasticsearchException("Watch[{}] attachment[{}] Error executing HTTP request host[{}], port[{}], " + + "method[{}], path[{}], exception[{}]", + context.watch().id(), attachment.id(), httpRequest.host(), httpRequest.port(), httpRequest.method(), + httpRequest.path(), e.getMessage()); } - - throw new ElasticsearchException("Unable to get attachment of type [{}] with id [{}] in watch [{}] aborting watch execution", - type(), attachment.id(), context.watch().id()); } } diff --git a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParserTests.java b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParserTests.java index 7cb1f4d5393..da19c01e8a6 100644 --- a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParserTests.java +++ b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParserTests.java @@ -5,6 +5,8 @@ */ package org.elasticsearch.xpack.notification.email.attachment; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -17,9 +19,15 @@ import org.elasticsearch.xpack.common.http.HttpResponse; import org.elasticsearch.xpack.common.http.auth.HttpAuthRegistry; import org.elasticsearch.xpack.common.http.auth.basic.BasicAuth; import org.elasticsearch.xpack.common.http.auth.basic.BasicAuthFactory; +import org.elasticsearch.xpack.watcher.execution.WatchExecutionContext; +import org.elasticsearch.xpack.watcher.execution.Wid; import org.elasticsearch.xpack.watcher.test.MockTextTemplateEngine; +import org.elasticsearch.xpack.watcher.watch.Payload; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import org.junit.Before; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -28,6 +36,7 @@ import java.util.Map; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonMap; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.xpack.watcher.test.WatcherTestUtils.mockExecutionContextBuilder; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.Is.is; import static org.mockito.Matchers.any; @@ -38,6 +47,8 @@ public class HttpEmailAttachementParserTests extends ESTestCase { private HttpRequestTemplate.Parser httpRequestTemplateParser; private HttpClient httpClient; + private EmailAttachmentsParser emailAttachmentsParser; + private Map attachmentParsers; @Before public void init() throws Exception { @@ -45,16 +56,16 @@ public class HttpEmailAttachementParserTests extends ESTestCase { httpRequestTemplateParser = new HttpRequestTemplate.Parser(authRegistry); httpClient = mock(HttpClient.class); - HttpResponse response = new HttpResponse(200, "This is my response".getBytes(UTF_8)); - when(httpClient.execute(any(HttpRequest.class))).thenReturn(response); + attachmentParsers = new HashMap<>(); + attachmentParsers.put(HttpEmailAttachementParser.TYPE, + new HttpEmailAttachementParser(httpClient, httpRequestTemplateParser, new MockTextTemplateEngine())); + emailAttachmentsParser = new EmailAttachmentsParser(attachmentParsers); } public void testSerializationWorks() throws Exception { - Map attachmentParsers = new HashMap<>(); - attachmentParsers.put(HttpEmailAttachementParser.TYPE, - new HttpEmailAttachementParser(httpClient, httpRequestTemplateParser, new MockTextTemplateEngine())); - EmailAttachmentsParser emailAttachmentsParser = new EmailAttachmentsParser(attachmentParsers); + HttpResponse response = new HttpResponse(200, "This is my response".getBytes(UTF_8)); + when(httpClient.execute(any(HttpRequest.class))).thenReturn(response); String id = "some-id"; XContentBuilder builder = jsonBuilder().startObject().startObject(id) @@ -91,4 +102,59 @@ public class HttpEmailAttachementParserTests extends ESTestCase { assertThat(attachments.get(0).inline(), is(isInline)); } + + public void testNonOkHttpCodeThrowsException() throws Exception { + HttpResponse response = new HttpResponse(403, "This is my response".getBytes(UTF_8)); + when(httpClient.execute(any(HttpRequest.class))).thenReturn(response); + + HttpRequestTemplate requestTemplate = HttpRequestTemplate.builder("localhost", 80).path("foo").build(); + HttpRequestAttachment attachment = new HttpRequestAttachment("someid", requestTemplate, false, null); + WatchExecutionContext ctx = createWatchExecutionContext(); + + ElasticsearchException exception = expectThrows(ElasticsearchException.class, + () -> attachmentParsers.get(HttpEmailAttachementParser.TYPE).toAttachment(ctx, new Payload.Simple(), attachment)); + assertThat(exception.getMessage(), is("Watch[watch1] attachment[someid] HTTP error status host[localhost], port[80], " + + "method[GET], path[foo], status[403]")); + } + + public void testEmptyResponseThrowsException() throws Exception { + HttpResponse response = new HttpResponse(200); + when(httpClient.execute(any(HttpRequest.class))).thenReturn(response); + + HttpRequestTemplate requestTemplate = HttpRequestTemplate.builder("localhost", 80).path("foo").build(); + HttpRequestAttachment attachment = new HttpRequestAttachment("someid", requestTemplate, false, null); + WatchExecutionContext ctx = createWatchExecutionContext(); + + ElasticsearchException exception = expectThrows(ElasticsearchException.class, + () -> attachmentParsers.get(HttpEmailAttachementParser.TYPE).toAttachment(ctx, new Payload.Simple(), attachment)); + assertThat(exception.getMessage(), is("Watch[watch1] attachment[someid] HTTP empty response body host[localhost], port[80], " + + "method[GET], path[foo], status[200]")); + } + + public void testHttpClientThrowsException() throws Exception { + when(httpClient.execute(any(HttpRequest.class))).thenThrow(new IOException("whatever")); + + HttpRequestTemplate requestTemplate = HttpRequestTemplate.builder("localhost", 80).path("foo").build(); + HttpRequestAttachment attachment = new HttpRequestAttachment("someid", requestTemplate, false, null); + WatchExecutionContext ctx = createWatchExecutionContext(); + + ElasticsearchException exception = expectThrows(ElasticsearchException.class, + () -> attachmentParsers.get(HttpEmailAttachementParser.TYPE).toAttachment(ctx, new Payload.Simple(), attachment)); + assertThat(exception.getMessage(), is("Watch[watch1] attachment[someid] Error executing HTTP request host[localhost], port[80], " + + "method[GET], path[foo], exception[whatever]")); + } + + private WatchExecutionContext createWatchExecutionContext() { + DateTime now = DateTime.now(DateTimeZone.UTC); + Wid wid = new Wid(randomAsciiOfLength(5), randomLong(), now); + Map metadata = MapBuilder.newMapBuilder().put("_key", "_val").map(); + return mockExecutionContextBuilder("watch1") + .wid(wid) + .payload(new Payload.Simple()) + .time("watch1", now) + .metadata(metadata) + .buildMock(); + } + + } diff --git a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java index 3fcfc2666b3..cae107d5678 100644 --- a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java +++ b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java @@ -510,7 +510,7 @@ public class EmailActionTests extends ESTestCase { public void testThatOneFailedEmailAttachmentResultsInActionFailure() throws Exception { EmailService emailService = new AbstractWatcherIntegrationTestCase.NoopEmailService(); - TextTemplateEngine engine = mock(TextTemplateEngine.class); + TextTemplateEngine engine = new MockTextTemplateEngine(); HttpClient httpClient = mock(HttpClient.class); // setup mock response, second one is an error @@ -542,7 +542,6 @@ public class EmailActionTests extends ESTestCase { .endObject() .endObject(); XContentParser parser = JsonXContent.jsonXContent.createParser(builder.bytes()); - logger.info("JSON: {}", builder.string()); parser.nextToken(); @@ -563,7 +562,8 @@ public class EmailActionTests extends ESTestCase { assertThat(result, instanceOf(EmailAction.Result.Failure.class)); EmailAction.Result.Failure failure = (EmailAction.Result.Failure) result; assertThat(failure.reason(), - is("Unable to get attachment of type [http] with id [second] in watch [watch1] aborting watch execution")); + is("Watch[watch1] attachment[second] HTTP error status host[localhost], port[80], method[GET], path[/second], " + + "status[403]")); } private EmailActionFactory createEmailActionFactory() {