Merge pull request #14 from padilo/feature/log_errors

Send internal errors to logger

src/main/java/scmspain/karyon/restrouter/RestRouterHandler.java

@@ -3,16 +3,20 @@
 import com.google.common.net.HttpHeaders;
 import com.google.common.net.MediaType;
 import io.netty.buffer.ByteBuf;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpResponseStatus;
 import io.reactivex.netty.protocol.http.server.HttpServerRequest;
 import io.reactivex.netty.protocol.http.server.HttpServerResponse;
 import io.reactivex.netty.protocol.http.server.RequestHandler;
 import org.apache.commons.lang.StringUtils;
 import org.commonjava.mimeparse.MIMEParse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import rx.Observable;
 import scmspain.karyon.restrouter.exception.CannotSerializeException;
 import scmspain.karyon.restrouter.exception.InvalidAcceptHeaderException;
-import scmspain.karyon.restrouter.handlers.DefaultKaryonErrorHandler;
 import scmspain.karyon.restrouter.handlers.ErrorHandler;
+import scmspain.karyon.restrouter.handlers.KaryonRestRouterErrorHandler;
 import scmspain.karyon.restrouter.handlers.RestRouterErrorDTO;
 import scmspain.karyon.restrouter.serializer.SerializeManager;
 import scmspain.karyon.restrouter.serializer.SerializeWriter;
@@ -22,6 +26,7 @@
 import scmspain.karyon.restrouter.transport.http.RouteNotFound;
 
 import javax.inject.Inject;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -30,9 +35,12 @@
 public class RestRouterHandler implements RequestHandler<ByteBuf, ByteBuf> {
   private SerializeManager serializerManager;
   private RestUriRouter<ByteBuf, ByteBuf> restUriRouter;
-  private DefaultKaryonErrorHandler defaultKaryonErrorHandler = new DefaultKaryonErrorHandler();
+  private KaryonRestRouterErrorHandler karyonRestRouterErrorHandler = new KaryonRestRouterErrorHandler();
   private ErrorHandler<ByteBuf> errorHandler;
 
+  private static final Logger L = LoggerFactory.getLogger(RestRouterHandler.class);
+
+
   /**
    * Creates an instance
    * @param restUriRouter the rest uri router
@@ -87,9 +95,38 @@ public Observable<Void> handle(HttpServerRequest<ByteBuf> request,
 
     }
 
-    return result;
+    return result.onErrorResumeNext(throwable -> {
+      response.setStatus(HttpResponseStatus.INTERNAL_SERVER_ERROR);
+
+      logError(throwable, request);
+
+      return Observable.empty();
+    });
   }
 
+  private void logError(Throwable throwable, HttpServerRequest<ByteBuf> request) {
+    try {
+      HttpMethod method = request.getHttpMethod();
+      String methodStr = method != null ? method.name() : "<Unknown>";
+      String path = request.getPath() + "?" + request.getQueryString();
+
+      String requestHeaders = getRequestHeaders(request);
+
+      String message = String.format("Internal server error requesting [%s %s] [HEADERS=> %s]", methodStr, path, requestHeaders);
+
+      L.error(message, throwable);
+    } catch (Throwable t) {
+      // Just to be sure we don't generate additional errors
+      L.error("Internal server error", throwable);
+    }
+  }
+
+  private String getRequestHeaders(HttpServerRequest<ByteBuf> request) {
+    return request.getHeaders().entries().stream()
+        .map(entry -> entry.getKey() + ": " + entry.getValue())
+        .reduce((entry1, entry2) -> entry1 + ", " + entry2)
+        .orElse("");
+  }
 
   private Observable<Void> handleSupported(Route<ByteBuf,ByteBuf> route,
                                           HttpServerRequest<ByteBuf> request,
@@ -122,7 +159,7 @@ private Observable<Void> handleSupported(Route<ByteBuf,ByteBuf> route,
 
     // If RouteNotFound is not handle it will be handled here
     resultObs = resultObs.onErrorResumeNext(throwable -> {
-      return defaultKaryonErrorHandler.handleError(request, throwable, response::setStatus);
+      return karyonRestRouterErrorHandler.handleError(request, throwable, response::setStatus);
     });
 
     Serializer serializer = serializerManager.getSerializer(contentType)
@@ -142,7 +179,7 @@ private Observable<Void> handleCustomSerialization(Route<ByteBuf, ByteBuf> route
         .process(request, response);
 
     resultObs = resultObs.onErrorResumeNext(throwable -> {
-      return defaultKaryonErrorHandler.handleError(request, throwable, response::setStatus)
+      return karyonRestRouterErrorHandler.handleError(request, throwable, response::setStatus)
           .map(this::serializeErrorDto)
           .flatMap(response::writeStringAndFlush);
     });

src/main/java/scmspain/karyon/restrouter/RestRouterScanner.java

@@ -84,13 +84,13 @@ private void validate(SerializeManager serializeManager) {
       }
 
       if(!serializeManager.getSupportedMediaTypes().containsAll(produces)) {
-        RuntimeException e = new RuntimeException("Existe una route con produces y no hay serializacion para alguno de los media types");
+        RuntimeException e = new RuntimeException("Exists a route with a produces that cannot be handle");
         logger.error("error", e);
         throw e;
       }
 
       if(!produces.isEmpty() && custom) {
-        RuntimeException e = new RuntimeException("Route con produces y custom");
+        RuntimeException e = new RuntimeException("There is a route which is not serializable annotated with produce");
         logger.error("error", e);
         throw e;
       }

src/main/java/scmspain/karyon/restrouter/handlers/DefaultKaryonErrorHandler.java → src/main/java/scmspain/karyon/restrouter/handlers/KaryonRestRouterErrorHandler.java

@@ -4,6 +4,8 @@
 import io.netty.buffer.ByteBuf;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.reactivex.netty.protocol.http.server.HttpServerRequest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import rx.Observable;
 import scmspain.karyon.restrouter.exception.CannotSerializeException;
 import scmspain.karyon.restrouter.exception.InvalidAcceptHeaderException;
@@ -15,7 +17,8 @@
  * Default error handler, this is the error generated when the defined {@link ErrorHandler} cannot
  * handle the generated exception
  */
-public class DefaultKaryonErrorHandler implements ErrorHandler<ByteBuf> {
+public class KaryonRestRouterErrorHandler implements ErrorHandler<ByteBuf> {
+  private static final Logger L = LoggerFactory.getLogger(KaryonRestRouterErrorHandler.class);
 
   @Override
   public Observable<RestRouterErrorDTO> handleError(
@@ -29,7 +32,7 @@ public Observable<RestRouterErrorDTO> handleError(
 
       return Observable.just(new RestRouterErrorDTO("Path: " + request.getPath() + " NOT FOUND"));
 
-    }  else if (throwable instanceof  ParamAnnotationException) {
+    }  else if (throwable instanceof ParamAnnotationException) {
       statusCode.set(HttpResponseStatus.BAD_REQUEST);
 
       return Observable.empty();
@@ -53,7 +56,7 @@ public Observable<RestRouterErrorDTO> handleError(
           new RestRouterErrorDTO("Cannot serialize the response in the given accept: " + request.getHeaders().get(HttpHeaders.ACCEPT)));
 
     } else {
-      statusCode.set(HttpResponseStatus.INTERNAL_SERVER_ERROR);
+
       return Observable.error(throwable);
     }
   }

src/test/java/scmspain/karyon/restrouter/RestRouterHandlerTest.java

@@ -5,6 +5,7 @@
 import com.google.common.net.HttpHeaders;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.PooledByteBufAllocator;
+import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.reactivex.netty.protocol.http.server.HttpRequestHeaders;
 import io.reactivex.netty.protocol.http.server.HttpResponseHeaders;
@@ -370,21 +371,21 @@ public void testWhenAcceptIsEmptyAndSupportedAreJsonAndXmlAndTestAndProducesCust
     given(routeHandler.process(request, response))
         .willReturn(Observable.error(new RuntimeException("test")));
 
+    given(request.getHttpMethod())
+        .willReturn(HttpMethod.GET);
 
     // When
     RestRouterHandler restRouterHandler = new RestRouterHandler(restUriRouter, serializerManager, errorHandler);
     Observable<Void> responseBody = restRouterHandler.handle(request, response);
 
     responseBody.subscribe(subscriber);
 
-    List<Throwable> throwableList = subscriber.getOnErrorEvents();
-
     // Then
+    subscriber.assertNoErrors();
     verify(routeHandler).process(request, response);
     verify(serializer, never()).serialize(any(), any());
     verify(response.getHeaders(), never()).setHeader(any(), any());
-
-    assertThat(throwableList, hasItem(Matchers.isA(RuntimeException.class)));
+    verify(response).setStatus(HttpResponseStatus.INTERNAL_SERVER_ERROR);
   }
 
   @Test