Error handling support in HTTP for Nima WebServer. (helidon-io#5436)

nima/tests/integration/webserver/webserver/src/test/java/io/helidon/nima/tests/integration/server/ErrorHandlingTest.java

@@ -0,0 +1,169 @@
+/*
+ * Copyright (c) 2022 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.nima.tests.integration.server;
+
+import io.helidon.common.http.Http;
+import io.helidon.nima.testing.junit5.webserver.DirectClient;
+import io.helidon.nima.testing.junit5.webserver.RoutingTest;
+import io.helidon.nima.testing.junit5.webserver.SetUpRoute;
+import io.helidon.nima.webclient.http1.Http1Client;
+import io.helidon.nima.webclient.http1.Http1ClientResponse;
+import io.helidon.nima.webserver.http.ErrorHandler;
+import io.helidon.nima.webserver.http.Filter;
+import io.helidon.nima.webserver.http.FilterChain;
+import io.helidon.nima.webserver.http.HttpRouting;
+import io.helidon.nima.webserver.http.RoutingRequest;
+import io.helidon.nima.webserver.http.RoutingResponse;
+import io.helidon.nima.webserver.http.ServerRequest;
+import io.helidon.nima.webserver.http.ServerResponse;
+
+import org.junit.jupiter.api.Test;
+
+import static io.helidon.common.testing.http.junit5.HttpHeaderMatcher.hasHeader;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@RoutingTest
+class ErrorHandlingTest {
+    private static final Http.HeaderName CONTROL_HEADER = Http.Header.create("X-HELIDON-JUNIT");
+    private static final Http.HeaderValue FIRST = Http.Header.create(CONTROL_HEADER, "first");
+    private static final Http.HeaderValue SECOND = Http.Header.create(CONTROL_HEADER, "second");
+    private static final Http.HeaderValue ROUTING = Http.Header.create(CONTROL_HEADER, "routing");
+    private static final Http.HeaderValue CUSTOM = Http.Header.create(CONTROL_HEADER, "custom");
+
+    private final Http1Client client;
+
+    ErrorHandlingTest(DirectClient client) {
+        this.client = client;
+    }
+
+    @SetUpRoute
+    static void routing(HttpRouting.Builder builder) {
+        builder.error(FirstException.class, new FirstHandler())
+                .error(SecondException.class, new SecondHandler())
+                .error(CustomRoutingException.class, new CustomRoutingHandler())
+                .addFilter(new FirstFilter())
+                .addFilter(new SecondFilter())
+                .get("/", ErrorHandlingTest::handler);
+    }
+
+    @Test
+    void testOk() {
+        String response = client.get()
+                .request(String.class);
+        assertThat(response, is("Done"));
+    }
+
+    @Test
+    void testFirst() {
+        String response = client.get()
+                .header(FIRST)
+                .request(String.class);
+        assertThat(response, is("First"));
+    }
+
+    @Test
+    void testSecond() {
+        String response = client.get()
+                .header(SECOND)
+                .request(String.class);
+        assertThat(response, is("Second"));
+    }
+
+    @Test
+    void testCustom() {
+        String response = client.get()
+                .header(CUSTOM)
+                .request(String.class);
+        assertThat(response, is("Custom"));
+    }
+
+    @Test
+    void testUnhandled() {
+        try (Http1ClientResponse response = client.get()
+                .header(ROUTING)
+                .request()) {
+            assertThat(response.status(), is(Http.Status.INTERNAL_SERVER_ERROR_500));
+            assertThat(response.headers(), hasHeader(Http.HeaderValues.CONTENT_LENGTH_ZERO));
+        }
+    }
+
+    private static void handler(ServerRequest req, ServerResponse res) throws Exception {
+        if (req.headers().contains(ROUTING)) {
+            throw new RoutingException();
+        }
+        if (req.headers().contains(CUSTOM)) {
+            throw new CustomRoutingException();
+        }
+        res.send("Done");
+    }
+
+    private static class FirstFilter implements Filter {
+        @Override
+        public void filter(FilterChain chain, RoutingRequest req, RoutingResponse res) {
+            if (req.headers().contains(FIRST)) {
+                throw new FirstException();
+            }
+            chain.proceed();
+        }
+    }
+
+    private static class SecondFilter implements Filter {
+        @Override
+        public void filter(FilterChain chain, RoutingRequest req, RoutingResponse res) {
+            if (req.headers().contains(SECOND)) {
+                throw new SecondException();
+            }
+            chain.proceed();
+        }
+    }
+
+    private static class FirstHandler implements ErrorHandler<FirstException> {
+        @Override
+        public void handle(ServerRequest req, ServerResponse res, FirstException throwable) {
+            res.send("First");
+        }
+    }
+
+    private static class SecondHandler implements ErrorHandler<SecondException> {
+        @Override
+        public void handle(ServerRequest req, ServerResponse res, SecondException throwable) {
+            res.send("Second");
+        }
+    }
+
+    private static class CustomRoutingHandler implements ErrorHandler<CustomRoutingException> {
+        @Override
+        public void handle(ServerRequest req, ServerResponse res, CustomRoutingException throwable) {
+            res.send("Custom");
+        }
+    }
+
+    private static class FirstException extends RuntimeException {
+    }
+
+    private static class SecondException extends RuntimeException {
+    }
+
+    private static class RoutingException extends Exception {
+
+    }
+
+    private static class CustomRoutingException extends RoutingException {
+
+    }
+}

nima/webserver/webserver/pom.xml

@@ -65,10 +65,25 @@
             <artifactId>junit-jupiter-api</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <artifactId>junit-jupiter-params</artifactId>
+            <groupId>org.junit.jupiter</groupId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <artifactId>mockito-core</artifactId>
+            <groupId>org.mockito</groupId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.hamcrest</groupId>
             <artifactId>hamcrest-all</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <artifactId>helidon-common-testing-junit5</artifactId>
+            <groupId>io.helidon.common.testing</groupId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/Executable.java → nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ErrorHandler.java

@@ -17,15 +17,20 @@
 package io.helidon.nima.webserver.http;
 
 /**
- * A runnable that can throw a checked exception.
- * This is to allow users to throw exception from their routes (and these will either end in an
- * {@link io.helidon.common.http.InternalServerException}, or will be handled by exception handler.
+ * The routing error handler.
+ * Can be mapped to the error cause in {@link io.helidon.nima.webserver.http.HttpRouting}.
+ *
+ * @param <T> type of throwable handled by this handler
+ * @see io.helidon.nima.webserver.http.HttpRouting.Builder#error(Class, ErrorHandler)
  */
-interface Executable {
+@FunctionalInterface
+public interface ErrorHandler<T extends Throwable> {
     /**
-     * Execute with a possible checked exception.
+     * Error handling consumer.
      *
-     * @throws Exception any exception
+     * @param req the server request
+     * @param res the server response
+     * @param throwable the cause of the error
      */
-    void execute() throws Exception;
+    void handle(ServerRequest req, ServerResponse res, T throwable);
 }

nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/ErrorHandlers.java

@@ -18,6 +18,10 @@
 
 import java.io.UncheckedIOException;
 import java.net.SocketException;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.Callable;
 
 import io.helidon.common.http.BadRequestException;
 import io.helidon.common.http.DirectHandler;
@@ -32,8 +36,25 @@
  */
 public final class ErrorHandlers {
     private static final System.Logger LOGGER = System.getLogger(ErrorHandlers.class.getName());
+    private final IdentityHashMap<Class<? extends Throwable>, ErrorHandler<?>> errorHandlers;
 
-    ErrorHandlers() {
+    private ErrorHandlers(IdentityHashMap<Class<? extends Throwable>, ErrorHandler<?>> errorHandlers) {
+        this.errorHandlers = errorHandlers;
+    }
+
+    /**
+     * Create error handlers.
+     *
+     * @param errorHandlers map of type to error handler
+     * @return new error handlers
+     */
+    public static ErrorHandlers create(Map<Class<? extends Throwable>, ErrorHandler<?>> errorHandlers) {
+        return new ErrorHandlers(new IdentityHashMap<>(errorHandlers));
+    }
+
+    @Override
+    public String toString() {
+        return "ErrorHandlers for " + errorHandlers.keySet();
     }
 
     /**
@@ -45,9 +66,9 @@ public final class ErrorHandlers {
      * @param response HTTP server response
      * @param task     task to execute
      */
-    public void runWithErrorHandling(ConnectionContext ctx, ServerRequest request, ServerResponse response, Executable task) {
+    public void runWithErrorHandling(ConnectionContext ctx, ServerRequest request, ServerResponse response, Callable<Void> task) {
         try {
-            task.execute();
+            task.call();
         } catch (CloseConnectionException | UncheckedIOException e) {
             // these errors must "bubble up"
             throw e;
@@ -65,10 +86,26 @@ public void runWithErrorHandling(ConnectionContext ctx, ServerRequest request, S
         } catch (InternalServerException e) {
             // this is the place error handling must be done
             // check if error handler exists for cause - if so, use it
-            if (hasErrorHandler(e.getCause())) {
-                handleError(ctx, request, response, e.getCause());
+            ErrorHandler errorHandler = null;
+            Throwable exception = null;
+
+            if (e.getCause() != null) {
+                var maybeEh = errorHandler(e.getCause().getClass());
+                if (maybeEh.isPresent()) {
+                    errorHandler = maybeEh.get();
+                    exception = e.getCause();
+                }
+            }
+
+            if (errorHandler == null) {
+                errorHandler = errorHandler(e.getClass()).orElse(null);
+                exception = e;
+            }
+
+            if (errorHandler == null) {
+                unhandledError(ctx, request, response, exception);
             } else {
-                handleError(ctx, request, response, e);
+                handleError(ctx, request, response, exception, errorHandler);
             }
         } catch (HttpException e) {
             handleError(ctx, request, response, e);
@@ -82,6 +119,26 @@ public void runWithErrorHandling(ConnectionContext ctx, ServerRequest request, S
         }
     }
 
+    @SuppressWarnings("unchecked")
+    <T extends Throwable> Optional<ErrorHandler<T>> errorHandler(Class<T> exceptionClass) {
+        // then look for error handlers that handle supertypes of this exception from lower to higher
+        Class<? extends Throwable> throwableClass = exceptionClass;
+        while (true) {
+            // first look for exact match
+            ErrorHandler<?> errorHandler = errorHandlers.get(throwableClass);
+            if (errorHandler != null) {
+                return Optional.of((ErrorHandler<T>) errorHandler);
+            }
+            if (!Throwable.class.isAssignableFrom(throwableClass)) {
+                return Optional.empty();
+            }
+            if (throwableClass == Throwable.class) {
+                return Optional.empty();
+            }
+            throwableClass = (Class<? extends Throwable>) throwableClass.getSuperclass();
+        }
+    }
+
     private void handleRequestException(ConnectionContext ctx,
                                         ServerRequest request,
                                         ServerResponse response,
@@ -102,12 +159,13 @@ private void handleRequestException(ConnectionContext ctx,
         ctx.directHandlers().handle(e, response, keepAlive);
     }
 
-    private boolean hasErrorHandler(Throwable cause) {
-        // TODO needs implementation (separate issue)
-        return true;
+    private void handleError(ConnectionContext ctx, ServerRequest request, ServerResponse response, Throwable e) {
+        errorHandler(e.getClass())
+                .ifPresentOrElse(it -> handleError(ctx, request, response, e, (ErrorHandler<Throwable>) it),
+                                 () -> unhandledError(ctx, request, response, e));
     }
 
-    private void handleError(ConnectionContext ctx, ServerRequest request, ServerResponse response, Throwable e) {
+    private void unhandledError(ConnectionContext ctx, ServerRequest request, ServerResponse response, Throwable e) {
         // to be handled by error handler
         handleRequestException(ctx, request, response, RequestException.builder()
                 .cause(e)
@@ -128,4 +186,17 @@ private void handleError(ConnectionContext ctx, ServerRequest request, ServerRes
                 .request(DirectTransportRequest.create(request.prologue(), request.headers()))
                 .build());
     }
+
+    private void handleError(ConnectionContext ctx,
+                             ServerRequest request,
+                             ServerResponse response,
+                             Throwable e,
+                             ErrorHandler<Throwable> it) {
+        try {
+            it.handle(request, response, e);
+        } catch (Exception ex) {
+            ctx.log(LOGGER, System.Logger.Level.TRACE, "Failed to handle exception.", ex);
+            unhandledError(ctx, request, response, e);
+        }
+    }
 }