refactor: move error handler configuration to ServerConfig class

Before this change the JsonObject passed to the ServerVerticle was used to read the configuration for the custom error template to use. With this change the JsonObject was replaced by the ServerConfig object. In addition to a custom errorHandlerClassName a new errorHandlerTemplate property was added, which can be used to just specify a different (HTML) template to use in the DefaultErrorHandler.

Switched to promisified interfaces in FileSystemHelper. Increased the code coverage error threshold to 80% instead of 81%, as the violation plugin did fail at 81.0% eventhough the failIfLessThanThresholdError was set.

gradle/codeCoverage.gradle

@@ -52,8 +52,9 @@ consoleReporter {
         // Set this property to a certain C0 coverage percentage.
         // When the coverage is less than this value and
         // failIfLessThanThresholdError property is set to true,
-        // the build will fail.
-        thresholdError 81
+        // the build will fail. Even if the property is called
+        // failIfLessThan... the build will actually fail at 80.0%
+        thresholdError 80
 
         // Set this property if you want to customize build error message
         // when you use 'failIfLessThanThresholdError' feature.

src/generated/java/io/neonbee/config/ServerConfigConverter.java

@@ -38,6 +38,16 @@ static void fromJson(Iterable<java.util.Map.Entry<String, Object>> json, ServerC
                     obj.setEndpointConfigs(list);
                 }
                 break;
+            case "errorHandlerClassName":
+                if (member.getValue() instanceof String) {
+                    obj.setErrorHandlerClassName((String) member.getValue());
+                }
+                break;
+            case "errorHandlerTemplate":
+                if (member.getValue() instanceof String) {
+                    obj.setErrorHandlerTemplate((String) member.getValue());
+                }
+                break;
             case "sessionCookieName":
                 if (member.getValue() instanceof String) {
                     obj.setSessionCookieName((String) member.getValue());
@@ -81,6 +91,12 @@ static void toJson(ServerConfig obj, java.util.Map<String, Object> json) {
             obj.getEndpointConfigs().forEach(item -> array.add(item.toJson()));
             json.put("endpointConfigs", array);
         }
+        if (obj.getErrorHandlerClassName() != null) {
+            json.put("errorHandlerClassName", obj.getErrorHandlerClassName());
+        }
+        if (obj.getErrorHandlerTemplate() != null) {
+            json.put("errorHandlerTemplate", obj.getErrorHandlerTemplate());
+        }
         if (obj.getSessionCookieName() != null) {
             json.put("sessionCookieName", obj.getSessionCookieName());
         }

src/main/java/io/neonbee/config/ServerConfig.java

@@ -19,6 +19,7 @@
 import io.neonbee.endpoint.metrics.MetricsEndpoint;
 import io.neonbee.endpoint.odatav4.ODataV4Endpoint;
 import io.neonbee.endpoint.raw.RawEndpoint;
+import io.neonbee.internal.handler.DefaultErrorHandler;
 import io.neonbee.internal.json.ImmutableJsonObject;
 import io.neonbee.internal.verticle.ServerVerticle;
 import io.vertx.codegen.annotations.DataObject;
@@ -41,6 +42,7 @@
 import io.vertx.core.net.TrustOptions;
 import io.vertx.core.tracing.TracingPolicy;
 import io.vertx.ext.web.RoutingContext;
+import io.vertx.ext.web.handler.ErrorHandler;
 
 /**
  * Handles the configuration for the {@linkplain ServerVerticle}, extending the {@linkplain HttpServerOptions} providing
@@ -210,6 +212,10 @@ public String getCorrelationId(RoutingContext routingContext) {
 
     private List<AuthHandlerConfig> authChainConfig;
 
+    private String errorHandlerClassName;
+
+    private String errorHandlerTemplate;
+
     /**
      * Create a default server configuration.
      */
@@ -398,6 +404,61 @@ public ServerConfig setAuthChainConfig(List<AuthHandlerConfig> authChainConfig)
         return this;
     }
 
+    /**
+     * Returns a custom error handler class name, which is instantiated as failure handler of the
+     * {@link ServerVerticle}. The {@link DefaultErrorHandler} is used in case no value is supplied. The class must
+     * implement the {@link ErrorHandler} interface and must provide either a default constructor, or, in case an error
+     * handler template is defined using {@link #setErrorHandlerTemplate(String)} a constructor accepting one string.
+     * The parameter-less constructor will be used as a fallback, in case no other constructor is found, the set error
+     * template will be ignored in that case.
+     *
+     * @return the class name of the error handler to handle failures in the server verticle or null, in case no custom
+     *         error handler should be used. The server verticle will fall back to {@link DefaultErrorHandler} in the
+     *         latter case
+     */
+    public String getErrorHandlerClassName() {
+        return errorHandlerClassName;
+    }
+
+    /**
+     * Sets a custom error handler class name.
+     *
+     * @see #getErrorHandlerClassName()
+     * @param errorHandlerClassName the class name of a class implementing {@link ErrorHandler} or null
+     * @return the {@link ServerConfig} for chaining
+     */
+    @Fluent
+    public ServerConfig setErrorHandlerClassName(String errorHandlerClassName) {
+        this.errorHandlerClassName = errorHandlerClassName;
+        return this;
+    }
+
+    /**
+     * Returns the path to an error handler template to use either in the {@link DefaultErrorHandler} or in any custom
+     * error handler set using {@link #setErrorHandlerClassName(String)}. Note that any custom error handler may ignore
+     * the template specified.
+     *
+     * @return the file name of an error handler template, or null. In the latter case the error handler has the choice
+     *         of the template to use
+     */
+    public String getErrorHandlerTemplate() {
+        return errorHandlerTemplate;
+    }
+
+    /**
+     * Sets the path to an error handler template to use either in the {@link DefaultErrorHandler} or in any custom
+     * error handler set using {@link #setErrorHandlerClassName(String)}.
+     *
+     * @see #getErrorHandlerTemplate()
+     * @param errorHandlerTemplate the file path to an error handler template to use
+     * @return the {@link ServerConfig} for chaining
+     */
+    @Fluent
+    public ServerConfig setErrorHandlerTemplate(String errorHandlerTemplate) {
+        this.errorHandlerTemplate = errorHandlerTemplate;
+        return this;
+    }
+
     /*
      * Override all HttpServerOptions setters, to return the right type for chaining
      */

src/main/java/io/neonbee/internal/handler/DefaultErrorHandler.java

@@ -5,6 +5,7 @@
 import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR;
 import static io.vertx.core.http.HttpHeaders.CONTENT_TYPE;
 
+import java.io.IOException;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
@@ -40,17 +41,25 @@ public class DefaultErrorHandler implements io.vertx.ext.web.handler.ErrorHandle
      * Returns a new ErrorHandler with the default {@link #DEFAULT_ERROR_HANDLER_TEMPLATE template}.
      */
     public DefaultErrorHandler() {
-        this(DEFAULT_ERROR_HANDLER_TEMPLATE);
+        try {
+            this.errorTemplate = readErrorTemplate(DEFAULT_ERROR_HANDLER_TEMPLATE);
+        } catch (IOException e) {
+            throw new RuntimeException("Could not read default error template", e);
+        }
     }
 
     /**
      * Returns a new ErrorHandler with the passed template.
      *
      * @param errorTemplateName resource path to the template.
+     * @throws IOException in case the template is not found or cannot be read from the current class loader.
      */
-    public DefaultErrorHandler(String errorTemplateName) {
-        Objects.requireNonNull(errorTemplateName);
-        this.errorTemplate = Objects.requireNonNull(readResourceToBuffer(errorTemplateName)).toString();
+    public DefaultErrorHandler(String errorTemplateName) throws IOException {
+        this.errorTemplate = readErrorTemplate(errorTemplateName);
+    }
+
+    private String readErrorTemplate(String errorTemplateName) throws IOException {
+        return readResourceToBuffer(Objects.requireNonNull(errorTemplateName)).toString();
     }
 
     @Override

src/main/java/io/neonbee/internal/helper/BufferHelper.java

@@ -4,6 +4,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.NoSuchFileException;
 
 import io.vertx.core.buffer.Buffer;
 
@@ -51,15 +52,18 @@ public static Buffer inputStreamToBuffer(InputStream input, int bufferSize) thro
      *
      * @param resource The resource to read
      * @return a Vert.x {@link Buffer}
+     * @throws IOException in case of an issue whilst reading the resource
      */
     @edu.umd.cs.findbugs.annotations.SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE",
             justification = "False positive in Spotbugs, see https://github.com/spotbugs/spotbugs/issues/1338")
-    public static Buffer readResourceToBuffer(String resource) {
+    public static Buffer readResourceToBuffer(String resource) throws IOException {
         ClassLoader classLoader = getClassLoader();
         try (InputStream input = classLoader.getResourceAsStream(resource)) {
-            return input != null ? inputStreamToBuffer(input) : null;
-        } catch (IOException ioe) {
-            throw new RuntimeException(ioe);
+            if (input == null) {
+                throw new NoSuchFileException(resource);
+            }
+
+            return inputStreamToBuffer(input);
         }
     }
 

src/main/java/io/neonbee/internal/helper/FileSystemHelper.java

@@ -45,7 +45,7 @@ public static Future<Boolean> isDirectory(Vertx vertx, Path path) {
      * @return Future of {@link List} of {@link String}s
      */
     public static Future<List<Path>> readDir(Vertx vertx, Path path) {
-        return Future.<List<String>>future(handler -> vertx.fileSystem().readDir(path.toString(), handler))
+        return vertx.fileSystem().readDir(path.toString())
                 .map(files -> files.stream().map(Path::of).collect(Collectors.toList()));
     }
 
@@ -59,7 +59,7 @@ public static Future<List<Path>> readDir(Vertx vertx, Path path) {
      * @return Future of {@link List} of {@link String}s
      */
     public static Future<List<Path>> readDir(Vertx vertx, Path path, String filter) {
-        return Future.<List<String>>future(handler -> vertx.fileSystem().readDir(path.toString(), filter, handler))
+        return vertx.fileSystem().readDir(path.toString(), filter)
                 .map(files -> files.stream().map(Path::of).collect(Collectors.toList()));
     }
 
@@ -74,7 +74,7 @@ public static Future<List<Path>> readDir(Vertx vertx, Path path, String filter)
      * @return Future of {@link AsyncFile}
      */
     public static Future<AsyncFile> openFile(Vertx vertx, OpenOptions options, Path path) {
-        return Future.<AsyncFile>future(promise -> vertx.fileSystem().open(path.toString(), options, promise));
+        return vertx.fileSystem().open(path.toString(), options);
     }
 
     /**
@@ -85,7 +85,7 @@ public static Future<AsyncFile> openFile(Vertx vertx, OpenOptions options, Path
      * @return Future of {@link Buffer}
      */
     public static Future<Buffer> readFile(Vertx vertx, Path path) {
-        return Future.<Buffer>future(readFilePromise -> vertx.fileSystem().readFile(path.toString(), readFilePromise));
+        return vertx.fileSystem().readFile(path.toString());
     }
 
     /**
@@ -130,7 +130,7 @@ private static Future<JsonObject> parseYAML(Vertx vertx, Buffer buffer) {
      * @return Future of {@link Void}
      */
     public static Future<Void> writeFile(Vertx vertx, Path path, Buffer buffer) {
-        return Future.<Void>future(promise -> vertx.fileSystem().writeFile(path.toString(), buffer, promise));
+        return vertx.fileSystem().writeFile(path.toString(), buffer);
     }
 
     /**
@@ -141,7 +141,7 @@ public static Future<Void> writeFile(Vertx vertx, Path path, Buffer buffer) {
      * @return Future of {@link Void}
      */
     public static Future<Void> deleteRecursive(Vertx vertx, Path path) {
-        return Future.<Void>future(promise -> vertx.fileSystem().deleteRecursive(path.toString(), true, promise));
+        return vertx.fileSystem().deleteRecursive(path.toString(), true);
     }
 
     /**
@@ -152,7 +152,7 @@ public static Future<Void> deleteRecursive(Vertx vertx, Path path) {
      * @return Future of {@link Boolean}
      */
     public static Future<Boolean> exists(Vertx vertx, Path path) {
-        return Future.<Boolean>future(promise -> vertx.fileSystem().exists(path.toString(), promise));
+        return vertx.fileSystem().exists(path.toString());
     }
 
     /**
@@ -163,7 +163,7 @@ public static Future<Boolean> exists(Vertx vertx, Path path) {
      * @return Future of {@link Void}
      */
     public static Future<Void> createDirs(Vertx vertx, Path path) {
-        return Future.<Void>future(promise -> vertx.fileSystem().mkdirs(path.toString(), promise));
+        return vertx.fileSystem().mkdirs(path.toString());
     }
 
     /**
@@ -174,6 +174,6 @@ public static Future<Void> createDirs(Vertx vertx, Path path) {
      * @return Future of {@link FileProps}
      */
     public static Future<FileProps> getProperties(Vertx vertx, Path path) {
-        return Future.<FileProps>future(promise -> vertx.fileSystem().props(path.toString(), promise));
+        return vertx.fileSystem().props(path.toString());
     }
 }

src/main/java/io/neonbee/internal/verticle/ServerVerticle.java

@@ -4,10 +4,12 @@
 import static io.vertx.core.Future.succeededFuture;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
+import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.InvocationTargetException;
 import java.util.List;
 import java.util.Optional;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -79,7 +81,8 @@ public void start(Promise<Void> startPromise) {
         Route rootRoute = router.route();
 
         try {
-            rootRoute.failureHandler(getErrorHandler(config()));
+            rootRoute.failureHandler(
+                    createErrorHandler(config.getErrorHandlerClassName(), config.getErrorHandlerTemplate()));
             rootRoute.handler(new LoggerHandler());
             rootRoute.handler(BodyHandler.create(false /* do not handle file uploads */));
             rootRoute.handler(new CorrelationIdHandler(config.getCorrelationStrategy()));
@@ -121,11 +124,26 @@ public void start(Promise<Void> startPromise) {
     }
 
     @VisibleForTesting
-    static ErrorHandler getErrorHandler(JsonObject config) throws ClassNotFoundException, NoSuchMethodException,
-            IllegalAccessException, InvocationTargetException, InstantiationException {
-        String className = config.getString("errorHandler", DEFAULT_ERROR_HANDLER_CLASS_NAME);
-        String errorHandlerClassName = className.isEmpty() ? DEFAULT_ERROR_HANDLER_CLASS_NAME : className;
-        return (ErrorHandler) Class.forName(errorHandlerClassName).getConstructor().newInstance();
+    // reason for PMD.EmptyCatchBlock empty cache block is "path-through" to fallback implementation and for
+    // PMD.SignatureDeclareThrowsException it does not matter, if this private method does not expose any concrete
+    // exceptions as the ServerVerticle start method anyways catches all exceptions in a generic try-catch block
+    @SuppressWarnings({ "PMD.EmptyCatchBlock", "PMD.SignatureDeclareThrowsException" })
+    static ErrorHandler createErrorHandler(String className, String template) throws Exception {
+        Class<?> classObject = Class.forName(Optional.ofNullable(className).filter(Predicate.not(String::isBlank))
+                .orElse(DEFAULT_ERROR_HANDLER_CLASS_NAME));
+        if (template != null) {
+            try {
+                return (ErrorHandler) classObject.getConstructor(String.class).newInstance(template);
+            } catch (InvocationTargetException e) {
+                // check for an IOException when reading the template and rather propagate that for better traceability
+                throw e.getCause() instanceof IOException ? (IOException) e.getCause() : e;
+            } catch (NoSuchMethodException e) {
+                // do nothing here, if there is no such constructor, assume
+                // the custom error handler class doesn't accept templates
+            }
+        }
+
+        return (ErrorHandler) classObject.getConstructor().newInstance();
     }
 
     /**

src/test/java/io/neonbee/internal/verticle/ServerVerticleTest.java

@@ -8,6 +8,7 @@
 
 import java.lang.reflect.Method;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.util.concurrent.TimeUnit;
 
@@ -112,13 +113,16 @@ void testLargerMaximumInitialLineAndCookieSizesConfig(VertxTestContext testCtx)
     }
 
     @Test
-    void testgGetErrorHandlerDefault() throws Exception {
-        JsonObject config = new JsonObject();
-        assertThat(ServerVerticle.getErrorHandler(config)).isInstanceOf(DefaultErrorHandler.class);
+    void testGetErrorHandlerDefault() throws Exception {
+        assertThat(ServerVerticle.createErrorHandler(null, null)).isInstanceOf(DefaultErrorHandler.class);
 
-        ClassNotFoundException exception = assertThrows(ClassNotFoundException.class,
-                () -> ServerVerticle.getErrorHandler(config.put("errorHandler", "Hugo")));
-        assertThat(exception).hasMessageThat().contains("Hugo");
+        NoSuchFileException nsfException =
+                assertThrows(NoSuchFileException.class, () -> ServerVerticle.createErrorHandler(null, "Max"));
+        assertThat(nsfException).hasMessageThat().contains("Max");
+
+        ClassNotFoundException cnfException =
+                assertThrows(ClassNotFoundException.class, () -> ServerVerticle.createErrorHandler("Hugo", null));
+        assertThat(cnfException).hasMessageThat().contains("Hugo");
     }
 
     @Override