You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by yc...@apache.org on 2022/05/06 23:33:38 UTC
[cassandra-sidecar] 01/01: CASSANDRASC-36: Support for ErrorHandler in Sidecar
This is an automated email from the ASF dual-hosted git repository.
ycai pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-sidecar.git
commit 2e233ec579e4d2a23021116027d75e776e7ad9ec
Author: Francisco Guerrero <fr...@apple.com>
AuthorDate: Wed Apr 27 10:24:00 2022 -0700
CASSANDRASC-36: Support for ErrorHandler in Sidecar
This commit adds a default `ErrorHandler` to the `Route#failureHandler()`. The default implementation
for Sidecar is `ErrorHandlerImpl`.
Additionally, we allow for custom implementations of the `ErrorHandler` to be injected (via module
overrides). This allows downstream projects to provide custom implementations of the `ErrorHandler` to
fit the specific needs of the project.
Patch by Francisco Guerrero; Reviewed by Dinesh Joshi, Saranya Krishnakumar and Yifan Cai for CASSANDRASC-36
---
.../sidecar/common/testing/CassandraPod.java | 8 +--
.../org/apache/cassandra/sidecar/MainModule.java | 14 ++++-
.../sidecar/LoggerHandlerInjectionTest.java | 65 +++++++++++++++-------
.../sidecar/StreamSSTableComponentTest.java | 64 +++++++++++----------
4 files changed, 96 insertions(+), 55 deletions(-)
diff --git a/cassandra-integration-tests/src/test/java/org/apache/cassandra/sidecar/common/testing/CassandraPod.java b/cassandra-integration-tests/src/test/java/org/apache/cassandra/sidecar/common/testing/CassandraPod.java
index afa658e..65c6b76 100644
--- a/cassandra-integration-tests/src/test/java/org/apache/cassandra/sidecar/common/testing/CassandraPod.java
+++ b/cassandra-integration-tests/src/test/java/org/apache/cassandra/sidecar/common/testing/CassandraPod.java
@@ -339,9 +339,9 @@ class CassandraPod
coreV1Api.deleteNamespacedService(podName, namespace, null, null, null, null, null, null);
}
- catch (Exception ignored)
+ catch (Exception ex)
{
- logger.info("Could not delete service {}", podName);
+ logger.info(String.format("Could not delete service %s", podName), ex);
}
}
@@ -356,9 +356,9 @@ class CassandraPod
logger.info("Deleting pod {}", podName);
coreV1Api.deleteNamespacedPod(podName, namespace, null, null, null, null, null, null);
}
- catch (Exception ignored)
+ catch (Exception ex)
{
- logger.error("Exception when stopping pod: {}", ignored.getMessage());
+ logger.error(String.format("Unable to delete pod %s", podName), ex);
}
}
}
diff --git a/src/main/java/org/apache/cassandra/sidecar/MainModule.java b/src/main/java/org/apache/cassandra/sidecar/MainModule.java
index a7eb33d..6925eb8 100644
--- a/src/main/java/org/apache/cassandra/sidecar/MainModule.java
+++ b/src/main/java/org/apache/cassandra/sidecar/MainModule.java
@@ -46,6 +46,7 @@ import io.vertx.core.http.HttpServerOptions;
import io.vertx.core.net.JksOptions;
import io.vertx.ext.dropwizard.DropwizardMetricsOptions;
import io.vertx.ext.web.Router;
+import io.vertx.ext.web.handler.ErrorHandler;
import io.vertx.ext.web.handler.LoggerHandler;
import io.vertx.ext.web.handler.StaticHandler;
import org.apache.cassandra.sidecar.cassandra40.Cassandra40Factory;
@@ -129,10 +130,12 @@ public class MainModule extends AbstractModule
@Provides
@Singleton
- public Router vertxRouter(Vertx vertx, LoggerHandler loggerHandler)
+ public Router vertxRouter(Vertx vertx, LoggerHandler loggerHandler, ErrorHandler errorHandler)
{
Router router = Router.router(vertx);
- router.route().handler(loggerHandler);
+ router.route()
+ .failureHandler(errorHandler)
+ .handler(loggerHandler);
// Static web assets for Swagger
StaticHandler swaggerStatic = StaticHandler.create("META-INF/resources/webjars/swagger-ui");
@@ -261,4 +264,11 @@ public class MainModule extends AbstractModule
{
return LoggerHandler.create();
}
+
+ @Provides
+ @Singleton
+ public ErrorHandler errorHandler(Vertx vertx)
+ {
+ return ErrorHandler.create(vertx);
+ }
}
diff --git a/src/test/java/org/apache/cassandra/sidecar/LoggerHandlerInjectionTest.java b/src/test/java/org/apache/cassandra/sidecar/LoggerHandlerInjectionTest.java
index d658ddb..774f880 100644
--- a/src/test/java/org/apache/cassandra/sidecar/LoggerHandlerInjectionTest.java
+++ b/src/test/java/org/apache/cassandra/sidecar/LoggerHandlerInjectionTest.java
@@ -1,8 +1,10 @@
package org.apache.cassandra.sidecar;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
@@ -27,6 +29,7 @@ import io.vertx.ext.web.handler.LoggerHandler;
import io.vertx.junit5.VertxExtension;
import io.vertx.junit5.VertxTestContext;
+import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR;
import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND;
import static io.netty.handler.codec.http.HttpResponseStatus.NO_CONTENT;
import static org.assertj.core.api.Assertions.assertThat;
@@ -34,6 +37,9 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+/**
+ * Tests for the {@link LoggerHandler} injection tests
+ */
@DisplayName("LoggerHandler Injection Test")
@ExtendWith(VertxExtension.class)
public class LoggerHandlerInjectionTest
@@ -41,62 +47,82 @@ public class LoggerHandlerInjectionTest
private Vertx vertx;
private Configuration config;
private final Logger logger = mock(Logger.class);
+ private HttpServer server;
@BeforeEach
void setUp() throws InterruptedException
{
FakeLoggerHandler loggerHandler = new FakeLoggerHandler(logger);
- Injector injector = Guice.createInjector(Modules.override(Modules.override(new MainModule()).with(new TestModule()))
- .with(binder -> binder.bind(LoggerHandler.class).toInstance(loggerHandler)));
+ Injector injector = Guice.createInjector(Modules.override(Modules.override(new MainModule())
+ .with(new TestModule()))
+ .with(binder -> binder.bind(LoggerHandler.class)
+ .toInstance(loggerHandler)));
vertx = injector.getInstance(Vertx.class);
config = injector.getInstance(Configuration.class);
Router router = injector.getInstance(Router.class);
- router.get("/500-route").handler(p -> {
- throw new RuntimeException("Fails with 500");
- });
+ router.get("/500-route").handler(p ->
+ {
+ throw new RuntimeException("Fails with 500");
+ });
- router.get("/404-route").handler(p -> {
- throw new HttpException(NOT_FOUND.code(), "Sorry, it's not here");
- });
+ router.get("/404-route").handler(p ->
+ {
+ throw new HttpException(NOT_FOUND.code(), "Sorry, it's not here");
+ });
- router.get("/204-route").handler(p -> {
- throw new HttpException(NO_CONTENT.code(), "Sorry, no content");
- });
+ router.get("/204-route").handler(p ->
+ {
+ throw new HttpException(NO_CONTENT.code(), "Sorry, no content");
+ });
VertxTestContext context = new VertxTestContext();
- HttpServer server = injector.getInstance(HttpServer.class);
+ server = injector.getInstance(HttpServer.class);
server.listen(config.getPort(), context.succeedingThenComplete());
context.awaitCompletion(5, TimeUnit.SECONDS);
}
+ @AfterEach
+ void tearDown() throws InterruptedException
+ {
+ final CountDownLatch closeLatch = new CountDownLatch(1);
+ server.close(res -> closeLatch.countDown());
+ vertx.close();
+ if (closeLatch.await(60, TimeUnit.SECONDS)) {
+ logger.info("Close event received before timeout.");
+ } else {
+ logger.error("Close event timed out.");
+ }
+ }
+
@DisplayName("Should log at error level when the request fails with a 500 code")
@Test
public void testInjectedLoggerHandlerLogsAtErrorLevel(VertxTestContext testContext)
{
- helper("/500-route", testContext, 500, "Internal Server Error");
+ helper("/500-route", testContext, INTERNAL_SERVER_ERROR.code(),
+ "Error 500: Internal Server Error");
}
@DisplayName("Should log at warn level when the request fails with a 404 error")
@Test
public void testInjectedLoggerHandlerLogsAtWarnLevel(VertxTestContext testContext)
{
- helper("/404-route", testContext, 404, "Not Found");
+ helper("/404-route", testContext, NOT_FOUND.code(), "Error 404: Not Found");
}
- @DisplayName("Should log at info level when the request returns with a 500 error")
+ @DisplayName("Should log at info level when the request returns with a 204 status code")
@Test
public void testInjectedLoggerHandlerLogsAtInfoLevel(VertxTestContext testContext)
{
- helper("/204-route", testContext, 204, null);
+ helper("/204-route", testContext, NO_CONTENT.code(), null);
}
private void helper(String requestURI, VertxTestContext testContext, int expectedStatusCode, String expectedBody)
{
WebClient client = WebClient.create(vertx);
- Handler<HttpResponse<String>> responseVerifier = response -> testContext.verify(
- () -> {
+ Handler<HttpResponse<String>> responseVerifier = response -> testContext.verify(() ->
+ {
assertThat(response.statusCode()).isEqualTo(expectedStatusCode);
if (expectedBody == null)
{
@@ -110,8 +136,7 @@ public class LoggerHandlerInjectionTest
verify(logger, times(1)).info("{}", expectedStatusCode);
});
client.get(config.getPort(), "localhost", requestURI)
- .as(BodyCodec.string())
- .ssl(false)
+ .as(BodyCodec.string()).ssl(false)
.send(testContext.succeeding(responseVerifier));
}
diff --git a/src/test/java/org/apache/cassandra/sidecar/StreamSSTableComponentTest.java b/src/test/java/org/apache/cassandra/sidecar/StreamSSTableComponentTest.java
index ffbf3bb..fc03af9 100644
--- a/src/test/java/org/apache/cassandra/sidecar/StreamSSTableComponentTest.java
+++ b/src/test/java/org/apache/cassandra/sidecar/StreamSSTableComponentTest.java
@@ -20,7 +20,13 @@ import io.vertx.ext.web.codec.BodyCodec;
import io.vertx.junit5.VertxExtension;
import io.vertx.junit5.VertxTestContext;
-import static org.junit.Assert.assertEquals;
+import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST;
+import static io.netty.handler.codec.http.HttpResponseStatus.FORBIDDEN;
+import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND;
+import static io.netty.handler.codec.http.HttpResponseStatus.OK;
+import static io.netty.handler.codec.http.HttpResponseStatus.PARTIAL_CONTENT;
+import static io.netty.handler.codec.http.HttpResponseStatus.REQUESTED_RANGE_NOT_SATISFIABLE;
+import static org.assertj.core.api.Assertions.assertThat;
/**
* Test for StreamSSTableComponent
@@ -42,7 +48,7 @@ public class StreamSSTableComponentTest
config = injector.getInstance(Configuration.class);
VertxTestContext context = new VertxTestContext();
- server.listen(config.getPort(), context.completing());
+ server.listen(config.getPort(), context.succeedingThenComplete());
context.awaitCompletion(5, TimeUnit.SECONDS);
}
@@ -69,8 +75,8 @@ public class StreamSSTableComponentTest
.as(BodyCodec.buffer())
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(200, response.statusCode());
- assertEquals("data", response.bodyAsString());
+ assertThat(response.statusCode()).isEqualTo(OK.code());
+ assertThat(response.bodyAsString()).isEqualTo("data");
context.completeNow();
})));
}
@@ -84,7 +90,7 @@ public class StreamSSTableComponentTest
client.get(config.getPort(), "localhost", "/api/v1/stream" + testRoute)
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(404, response.statusCode());
+ assertThat(response.statusCode()).isEqualTo(NOT_FOUND.code());
context.completeNow();
})));
}
@@ -98,7 +104,7 @@ public class StreamSSTableComponentTest
client.get(config.getPort(), "localhost", "/api/v1/stream" + testRoute)
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(404, response.statusCode());
+ assertThat(response.statusCode()).isEqualTo(NOT_FOUND.code());
context.completeNow();
})));
}
@@ -112,8 +118,8 @@ public class StreamSSTableComponentTest
client.get(config.getPort(), "localhost", "/api/v1/stream" + testRoute)
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(403, response.statusCode());
- assertEquals("system keyspace is forbidden", response.statusMessage());
+ assertThat(response.statusCode()).isEqualTo(FORBIDDEN.code());
+ assertThat(response.statusMessage()).isEqualTo("system keyspace is forbidden");
context.completeNow();
})));
}
@@ -127,8 +133,8 @@ public class StreamSSTableComponentTest
client.get(config.getPort(), "localhost", "/api/v1/stream" + testRoute)
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(400, response.statusCode());
- assertEquals("Invalid path params found", response.statusMessage());
+ assertThat(response.statusCode()).isEqualTo(BAD_REQUEST.code());
+ assertThat(response.statusMessage()).isEqualTo("Invalid path params found");
context.completeNow();
})));
}
@@ -142,8 +148,8 @@ public class StreamSSTableComponentTest
client.get(config.getPort(), "localhost", "/api/v1/stream" + testRoute)
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(400, response.statusCode());
- assertEquals("Invalid path params found", response.statusMessage());
+ assertThat(response.statusCode()).isEqualTo(BAD_REQUEST.code());
+ assertThat(response.statusMessage()).isEqualTo("Invalid path params found");
context.completeNow();
})));
}
@@ -157,8 +163,8 @@ public class StreamSSTableComponentTest
client.get(config.getPort(), "localhost", "/api/v1/stream" + testRoute)
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(400, response.statusCode());
- assertEquals("Invalid path params found", response.statusMessage());
+ assertThat(response.statusCode()).isEqualTo(BAD_REQUEST.code());
+ assertThat(response.statusMessage()).isEqualTo("Invalid path params found");
context.completeNow();
})));
}
@@ -174,8 +180,8 @@ public class StreamSSTableComponentTest
.as(BodyCodec.buffer())
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(200, response.statusCode());
- assertEquals("data", response.bodyAsString());
+ assertThat(response.statusCode()).isEqualTo(OK.code());
+ assertThat(response.bodyAsString()).isEqualTo("data");
context.completeNow();
})));
}
@@ -190,7 +196,7 @@ public class StreamSSTableComponentTest
.putHeader("Range", "bytes=4-3")
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(416, response.statusCode());
+ assertThat(response.statusCode()).isEqualTo(REQUESTED_RANGE_NOT_SATISFIABLE.code());
context.completeNow();
})));
}
@@ -205,7 +211,7 @@ public class StreamSSTableComponentTest
.putHeader("Range", "bytes=5-9")
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(416, response.statusCode());
+ assertThat(response.statusCode()).isEqualTo(REQUESTED_RANGE_NOT_SATISFIABLE.code());
context.completeNow();
})));
}
@@ -220,7 +226,7 @@ public class StreamSSTableComponentTest
.putHeader("Range", "bytes=5-")
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(416, response.statusCode());
+ assertThat(response.statusCode()).isEqualTo(REQUESTED_RANGE_NOT_SATISFIABLE.code());
context.completeNow();
})));
}
@@ -236,8 +242,8 @@ public class StreamSSTableComponentTest
.as(BodyCodec.buffer())
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(200, response.statusCode());
- assertEquals("data", response.bodyAsString());
+ assertThat(response.statusCode()).isEqualTo(OK.code());
+ assertThat(response.bodyAsString()).isEqualTo("data");
context.completeNow();
})));
}
@@ -253,8 +259,8 @@ public class StreamSSTableComponentTest
.as(BodyCodec.buffer())
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(206, response.statusCode());
- assertEquals("dat", response.bodyAsString());
+ assertThat(response.statusCode()).isEqualTo(PARTIAL_CONTENT.code());
+ assertThat(response.bodyAsString()).isEqualTo("dat");
context.completeNow();
})));
}
@@ -270,8 +276,8 @@ public class StreamSSTableComponentTest
.as(BodyCodec.buffer())
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(206, response.statusCode());
- assertEquals("ta", response.bodyAsString());
+ assertThat(response.statusCode()).isEqualTo(PARTIAL_CONTENT.code());
+ assertThat(response.bodyAsString()).isEqualTo("ta");
context.completeNow();
})));
}
@@ -286,7 +292,7 @@ public class StreamSSTableComponentTest
.putHeader("Range", "bytes=-5")
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(416, response.statusCode());
+ assertThat(response.statusCode()).isEqualTo(REQUESTED_RANGE_NOT_SATISFIABLE.code());
context.completeNow();
})));
}
@@ -301,7 +307,7 @@ public class StreamSSTableComponentTest
.putHeader("Range", "bits=0-2")
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(416, response.statusCode());
+ assertThat(response.statusCode()).isEqualTo(REQUESTED_RANGE_NOT_SATISFIABLE.code());
context.completeNow();
})));
}
@@ -317,8 +323,8 @@ public class StreamSSTableComponentTest
.as(BodyCodec.buffer())
.send(context.succeeding(response -> context.verify(() ->
{
- assertEquals(200, response.statusCode());
- assertEquals("data", response.bodyAsString());
+ assertThat(response.statusCode()).isEqualTo(OK.code());
+ assertThat(response.bodyAsString()).isEqualTo("data");
context.completeNow();
})));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org