From 5efec3a517949c27eadf93555c9a89b57cbc602d Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 24 Jun 2020 11:02:20 -0600 Subject: [PATCH] Add error logging when http test fails (#58505) Netty4HttpServerTransportTests has started to fail intermittently. It seems like unexpected successful responses are being received when the test is simulating errors. This commit adds logging to the test to provide additional information when there is an unexpected success. It also adds the logging to the nio http test. --- .../http/netty4/Netty4HttpServerTransportTests.java | 13 ++++++++++++- .../http/nio/NioHttpServerTransportTests.java | 12 ++++++++++++ .../elasticsearch/test/rest/FakeRestRequest.java | 4 ++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java b/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java index 38582b37333..6fb2ad01f15 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java @@ -39,6 +39,7 @@ import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.HttpVersion; +import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.network.NetworkAddress; @@ -62,6 +63,7 @@ import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.SharedGroupFactory; @@ -161,6 +163,8 @@ public class Netty4HttpServerTransportTests extends ESTestCase { @Override public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) { + logger.error(new ParameterizedMessage("--> Unexpected bad request [{}]", + FakeRestRequest.requestToString(channel.request())), cause); throw new AssertionError(); } }; @@ -222,6 +226,7 @@ public class Netty4HttpServerTransportTests extends ESTestCase { @Override public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) { + logger.error("--> Unexpected successful request [{}]", FakeRestRequest.requestToString(request)); throw new AssertionError(); } @@ -275,12 +280,12 @@ public class Netty4HttpServerTransportTests extends ESTestCase { assertThat(causeReference.get(), instanceOf(TooLongFrameException.class)); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/58433") public void testCorsRequest() throws InterruptedException { final HttpServerTransport.Dispatcher dispatcher = new HttpServerTransport.Dispatcher() { @Override public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) { + logger.error("--> Unexpected successful request [{}]", FakeRestRequest.requestToString(request)); throw new AssertionError(); } @@ -288,6 +293,8 @@ public class Netty4HttpServerTransportTests extends ESTestCase { public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) { + logger.error(new ParameterizedMessage("--> Unexpected bad request [{}]", + FakeRestRequest.requestToString(channel.request())), cause); throw new AssertionError(); } @@ -340,6 +347,7 @@ public class Netty4HttpServerTransportTests extends ESTestCase { @Override public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) { + logger.error("--> Unexpected successful request [{}]", FakeRestRequest.requestToString(request)); throw new AssertionError("Should not have received a dispatched request"); } @@ -347,6 +355,8 @@ public class Netty4HttpServerTransportTests extends ESTestCase { public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) { + logger.error(new ParameterizedMessage("--> Unexpected bad request [{}]", + FakeRestRequest.requestToString(channel.request())), cause); throw new AssertionError("Should not have received a dispatched request"); } @@ -385,4 +395,5 @@ public class Netty4HttpServerTransportTests extends ESTestCase { group.shutdownGracefully().await(); } } + } diff --git a/plugins/transport-nio/src/test/java/org/elasticsearch/http/nio/NioHttpServerTransportTests.java b/plugins/transport-nio/src/test/java/org/elasticsearch/http/nio/NioHttpServerTransportTests.java index ea3d44f60d1..8f8d0179b35 100644 --- a/plugins/transport-nio/src/test/java/org/elasticsearch/http/nio/NioHttpServerTransportTests.java +++ b/plugins/transport-nio/src/test/java/org/elasticsearch/http/nio/NioHttpServerTransportTests.java @@ -31,6 +31,7 @@ import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.HttpVersion; +import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.network.NetworkAddress; @@ -55,6 +56,7 @@ import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.nio.NioGroupFactory; @@ -152,6 +154,8 @@ public class NioHttpServerTransportTests extends ESTestCase { @Override public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) { + logger.error(new ParameterizedMessage("--> Unexpected bad request [{}]", + FakeRestRequest.requestToString(channel.request())), cause); throw new AssertionError(); } }; @@ -215,6 +219,7 @@ public class NioHttpServerTransportTests extends ESTestCase { @Override public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) { + logger.error("--> Unexpected successful request [{}]", FakeRestRequest.requestToString(request)); throw new AssertionError(); } @@ -222,6 +227,8 @@ public class NioHttpServerTransportTests extends ESTestCase { public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) { + logger.error(new ParameterizedMessage("--> Unexpected bad request [{}]", + FakeRestRequest.requestToString(channel.request())), cause); throw new AssertionError(); } @@ -275,6 +282,7 @@ public class NioHttpServerTransportTests extends ESTestCase { @Override public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) { + logger.error("--> Unexpected successful request [{}]", FakeRestRequest.requestToString(request)); throw new AssertionError(); } @@ -333,6 +341,7 @@ public class NioHttpServerTransportTests extends ESTestCase { @Override public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) { + logger.error("--> Unexpected successful request [{}]", FakeRestRequest.requestToString(request)); throw new AssertionError("Should not have received a dispatched request"); } @@ -340,6 +349,8 @@ public class NioHttpServerTransportTests extends ESTestCase { public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) { + logger.error(new ParameterizedMessage("--> Unexpected bad request [{}]", + FakeRestRequest.requestToString(channel.request())), cause); throw new AssertionError("Should not have received a dispatched request"); } @@ -370,4 +381,5 @@ public class NioHttpServerTransportTests extends ESTestCase { } } } + } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java index bab26854015..e36d4ae13b6 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java @@ -239,4 +239,8 @@ public class FakeRestRequest extends RestRequest { return new FakeRestRequest(xContentRegistry, fakeHttpRequest, params, new FakeHttpChannel(address)); } } + + public static String requestToString(RestRequest restRequest) { + return "method=" + restRequest.method() + ",path=" + restRequest.rawPath(); + } }