Merge branch 'mikaelq-fix-exception-return-200' into core for PR #272

Author: sapessi

aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/AwsProxyExceptionHandler.java

@@ -22,6 +22,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.ws.rs.InternalServerErrorException;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
 
@@ -31,7 +32,8 @@
 /**
  * Default implementation of the <code>ExceptionHandler</code> object that returns AwsProxyResponse objects.
  *
- * Returns application/json messages with a status code of 500 when the RequestReader failed to read the incoming event.
+ * Returns application/json messages with a status code of 500 when the RequestReader failed to read the incoming event
+ *  or if InternalServerErrorException is thrown.
  * For all other exceptions returns a 502. Responses are populated with a JSON object containing a message property.
  *
  * @see ExceptionHandler
@@ -76,7 +78,7 @@ public AwsProxyResponse handle(Throwable ex) {
         // adding a print stack trace in case we have no appender or we are running inside SAM local, where need the
         // output to go to the stderr.
         ex.printStackTrace();
-        if (ex instanceof InvalidRequestEventException) {
+        if (ex instanceof InvalidRequestEventException || ex instanceof InternalServerErrorException) {
             return new AwsProxyResponse(500, headers, getErrorJson(INTERNAL_SERVER_ERROR));
         } else {
             return new AwsProxyResponse(502, headers, getErrorJson(GATEWAY_TIMEOUT_ERROR));

aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/AwsProxyExceptionHandlerTest.java

@@ -3,7 +3,6 @@
 
 import com.amazonaws.serverless.exceptions.InvalidRequestEventException;
 import com.amazonaws.serverless.exceptions.InvalidResponseObjectException;
-import com.amazonaws.serverless.proxy.AwsProxyExceptionHandler;
 import com.amazonaws.serverless.proxy.model.AwsProxyResponse;
 import com.amazonaws.serverless.proxy.model.ErrorModel;
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -16,18 +15,22 @@
 
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
 
+import javax.ws.rs.InternalServerErrorException;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
 import java.io.*;
 
 
 public class AwsProxyExceptionHandlerTest {
+    private static final String INTERNAL_SERVER_ERROR_MESSAGE = "Internal server error";
     private static final String INVALID_REQUEST_MESSAGE = "Invalid request error";
     private static final String INVALID_RESPONSE_MESSAGE = "Invalid response error";
     private AwsProxyExceptionHandler exceptionHandler;
     private ObjectMapper objectMapper;
 
+
     @Before
     public void setUp() {
         exceptionHandler = new AwsProxyExceptionHandler();
@@ -88,6 +91,44 @@ public void typedHandle_InvalidResponseObjectException_jsonContentTypeHeader() {
         assertEquals(MediaType.APPLICATION_JSON, resp.getMultiValueHeaders().getFirst(HttpHeaders.CONTENT_TYPE));
     }
 
+    @Test
+    public void typedHandle_InternalServerErrorException_500State() {
+        // Needed to mock InternalServerErrorException because it leverages RuntimeDelegate to set an internal
+        // response object.
+        InternalServerErrorException mockInternalServerErrorException = Mockito.mock(InternalServerErrorException.class);
+        Mockito.when(mockInternalServerErrorException.getMessage()).thenReturn(INTERNAL_SERVER_ERROR_MESSAGE);
+
+        AwsProxyResponse resp = exceptionHandler.handle(mockInternalServerErrorException);
+
+        assertNotNull(resp);
+        assertEquals(500, resp.getStatusCode());
+    }
+
+    @Test
+    public void typedHandle_InternalServerErrorException_responseString()
+            throws JsonProcessingException {
+        InternalServerErrorException mockInternalServerErrorException = Mockito.mock(InternalServerErrorException.class);
+        Mockito.when(mockInternalServerErrorException.getMessage()).thenReturn(INTERNAL_SERVER_ERROR_MESSAGE);
+
+        AwsProxyResponse resp = exceptionHandler.handle(mockInternalServerErrorException);
+
+        assertNotNull(resp);
+        String body = objectMapper.writeValueAsString(new ErrorModel(AwsProxyExceptionHandler.INTERNAL_SERVER_ERROR));
+        assertEquals(body, resp.getBody());
+    }
+
+    @Test
+    public void typedHandle_InternalServerErrorException_jsonContentTypeHeader() {
+        InternalServerErrorException mockInternalServerErrorException = Mockito.mock(InternalServerErrorException.class);
+        Mockito.when(mockInternalServerErrorException.getMessage()).thenReturn(INTERNAL_SERVER_ERROR_MESSAGE);
+
+        AwsProxyResponse resp = exceptionHandler.handle(mockInternalServerErrorException);
+
+        assertNotNull(resp);
+        assertTrue(resp.getMultiValueHeaders().containsKey(HttpHeaders.CONTENT_TYPE));
+        assertEquals(MediaType.APPLICATION_JSON, resp.getMultiValueHeaders().getFirst(HttpHeaders.CONTENT_TYPE));
+    }
+
     @Test
     public void typedHandle_NullPointerException_responseObject()
             throws JsonProcessingException {

aws-serverless-java-container-jersey/src/main/java/com/amazonaws/serverless/proxy/jersey/JerseyServletResponseWriter.java

@@ -122,14 +122,8 @@ public void commit() {
 
 
     public void failure(Throwable throwable) {
-        try {
-            log.error("failure", throwable);
-            jerseyLatch.countDown();
-            servletResponse.flushBuffer();
-        } catch (IOException e) {
-            log.error("Could not fail response", e);
-            throw new InternalServerErrorException(e);
-        }
+        log.error("failure", throwable);
+        throw new InternalServerErrorException("Jersey failed to process request", throwable);
     }
 
 

aws-serverless-java-container-jersey/src/test/java/com/amazonaws/serverless/proxy/jersey/EchoJerseyResource.java

@@ -24,6 +24,7 @@
 import org.glassfish.jersey.media.multipart.FormDataMultiPart;
 import org.glassfish.jersey.media.multipart.FormDataParam;
 
+import javax.inject.Inject;
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -59,6 +60,9 @@ public class EchoJerseyResource {
     @Context
     SecurityContext securityCtx;
 
+    @Inject
+    JerseyDependency jerseyDependency;
+
     @Path("/decoded-param") @GET
     @Produces(MediaType.APPLICATION_JSON)
     public SingleValueModel echoDecodedParam(@QueryParam("param") String param) {

aws-serverless-java-container-jersey/src/test/java/com/amazonaws/serverless/proxy/jersey/JerseyAwsProxyTest.java

@@ -14,16 +14,15 @@
 
 
 import com.amazonaws.serverless.proxy.internal.LambdaContainerHandler;
-import com.amazonaws.serverless.proxy.jersey.providers.ServletRequestFilter;
-import com.amazonaws.serverless.proxy.model.AwsProxyRequest;
-import com.amazonaws.serverless.proxy.model.AwsProxyResponse;
 import com.amazonaws.serverless.proxy.internal.servlet.AwsServletContext;
-import com.amazonaws.serverless.proxy.jersey.model.MapResponseModel;
-import com.amazonaws.serverless.proxy.jersey.model.SingleValueModel;
 import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder;
 import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext;
+import com.amazonaws.serverless.proxy.jersey.model.MapResponseModel;
+import com.amazonaws.serverless.proxy.jersey.model.SingleValueModel;
+import com.amazonaws.serverless.proxy.jersey.providers.ServletRequestFilter;
+import com.amazonaws.serverless.proxy.model.AwsProxyRequest;
+import com.amazonaws.serverless.proxy.model.AwsProxyResponse;
 import com.amazonaws.services.lambda.runtime.Context;
-
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.commons.codec.binary.Base64;
@@ -37,13 +36,15 @@
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
-
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.UUID;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * Unit test class for the Jersey AWS_PROXY default implementation
@@ -57,13 +58,26 @@ public class JerseyAwsProxyTest {
 
 
     private static ObjectMapper objectMapper = new ObjectMapper();
+
     private static ResourceConfig app = new ResourceConfig().packages("com.amazonaws.serverless.proxy.jersey")
+            .register(LoggingFeature.class)
+            .register(ServletRequestFilter.class)
+            .register(MultiPartFeature.class)
+            .register(new ResourceBinder())
+            .property(LoggingFeature.LOGGING_FEATURE_VERBOSITY_SERVER, LoggingFeature.Verbosity.PAYLOAD_ANY);
+
+    private static ResourceConfig appWithoutRegisteredDependencies = new ResourceConfig()
+            .packages("com.amazonaws.serverless.proxy.jersey")
             .register(LoggingFeature.class)
             .register(ServletRequestFilter.class)
             .register(MultiPartFeature.class)
             .property(LoggingFeature.LOGGING_FEATURE_VERBOSITY_SERVER, LoggingFeature.Verbosity.PAYLOAD_ANY);
+
     private static JerseyLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse> handler = JerseyLambdaContainerHandler.getAwsProxyHandler(app);
 
+    private static JerseyLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse> handlerWithoutRegisteredDependencies
+            = JerseyLambdaContainerHandler.getAwsProxyHandler(appWithoutRegisteredDependencies);
+
     private static Context lambdaContext = new MockLambdaContext();
 
     private boolean isAlb;
@@ -76,15 +90,14 @@ public JerseyAwsProxyTest(boolean alb) {
     public static Collection<Object> data() {
         return Arrays.asList(new Object[] { false, true });
     }
-    
+
     private AwsProxyRequestBuilder getRequestBuilder(String path, String method) {
         AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder(path, method);
         if (isAlb) builder.alb();
-        
+
         return builder;
     }
 
-
     @Test
     public void alb_basicRequest_expectSuccess() {
         AwsProxyRequest request = getRequestBuilder("/echo/headers", "GET")
@@ -130,6 +143,18 @@ public void headers_servletRequest_echo() {
         validateMapResponseModel(output);
     }
 
+    @Test
+    public void headers_servletRequest_failedDependencyInjection_expectInternalServerError() {
+        AwsProxyRequest request = getRequestBuilder("/echo/servlet-headers", "GET")
+                .json()
+                .header(CUSTOM_HEADER_KEY, CUSTOM_HEADER_VALUE)
+                .build();
+
+        AwsProxyResponse output = handlerWithoutRegisteredDependencies.proxy(request, lambdaContext);
+        assertEquals("application/json", output.getMultiValueHeaders().getFirst("Content-Type"));
+        assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), output.getStatusCode());
+    }
+
     @Test
     public void context_servletResponse_setCustomHeader() {
         AwsProxyRequest request = getRequestBuilder("/echo/servlet-response", "GET")

aws-serverless-java-container-jersey/src/test/java/com/amazonaws/serverless/proxy/jersey/JerseyDependency.java

@@ -0,0 +1,6 @@
+package com.amazonaws.serverless.proxy.jersey;
+
+// This class is used to test HK2 dependency injection.
+public class JerseyDependency {
+
+}

aws-serverless-java-container-jersey/src/test/java/com/amazonaws/serverless/proxy/jersey/JerseyInjectionTest.java

@@ -30,7 +30,7 @@
  */
 public class JerseyInjectionTest {
 
-    // Test ressource binder
+    // Test resource binder
     private static class ResourceBinder extends AbstractBinder {
 
         @Override

aws-serverless-java-container-jersey/src/test/java/com/amazonaws/serverless/proxy/jersey/JerseyParamEncodingTest.java

@@ -11,7 +11,6 @@
 import com.amazonaws.services.lambda.runtime.Context;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
-import org.glassfish.jersey.logging.LoggingFeature;
 import org.glassfish.jersey.media.multipart.MultiPartFeature;
 import org.glassfish.jersey.server.ResourceConfig;
 import org.junit.Ignore;
@@ -26,10 +25,6 @@
 import java.net.URLEncoder;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.logging.ConsoleHandler;
-import java.util.logging.Level;
-import java.util.logging.Logger;
-import java.util.logging.SimpleFormatter;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -69,6 +64,7 @@ public class JerseyParamEncodingTest {
     private static ObjectMapper objectMapper = new ObjectMapper();
     private static ResourceConfig app = new ResourceConfig().packages("com.amazonaws.serverless.proxy.jersey")
                                                             .register(MultiPartFeature.class)
+                                                            .register(new ResourceBinder())
                                                             .property("jersey.config.server.tracing.type", "ALL")
                                                             .property("jersey.config.server.tracing.threshold", "VERBOSE");
     private static JerseyLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse> handler = JerseyLambdaContainerHandler.getAwsProxyHandler(app);

aws-serverless-java-container-jersey/src/test/java/com/amazonaws/serverless/proxy/jersey/ResourceBinder.java

@@ -0,0 +1,12 @@
+package com.amazonaws.serverless.proxy.jersey;
+
+import org.glassfish.jersey.internal.inject.AbstractBinder;
+
+import javax.inject.Singleton;
+
+public class ResourceBinder extends AbstractBinder {
+    @Override
+    protected void configure() {
+        bind(new JerseyDependency()).to(JerseyDependency.class).in(Singleton.class);
+    }
+}