Fix Status Code in RequestTelemetry when Servlet Throws Exception

CHANGELOG.md

@@ -1,6 +1,7 @@
 # CHANGELOG
 
-## Version 2.0.1
+## Version 2.0.2
+- Fix #600 where RequestTelemetry has 200 whereas browser shows 500 (when servlet throws)
 - Fix issue when dependency start time wasn't being recorded correctly
 - Fixed #533 HTTP Dependency Telemetry now matches with .NET SDK
 - Introduced public method `httpMethodFinishedWithPath(String identifier, String method, String path, String correlationId, String uri, String target, int result, long delta)`
@@ -13,6 +14,9 @@
 - Fixed Issue #526 (NPE in MapUtil.copy())
 - Fixed Issue #513 (Memory leak in SDKShutdownActivity). This fix upgrades our Servlet version from 2.5 to 3.0. The SDK must now be run on an application server supporting Servlet 3.0.
 
+## Version 2.0.1
+-Fix Inconsistency in artifact names in POM files
+
 ## Version 2.0.0
 - Upgraded logback dependency version to 1.2.3
 - Improved FixedRateSampling so that it also supports Sampling Percentage set by users programmatically. Fixes [issue #535](https://github.com/Microsoft/ApplicationInsights-Java/issues/535)

web/build.gradle

@@ -46,6 +46,7 @@ dependencies {
     testCompile group: 'org.apache.logging.log4j', name: 'log4j-api', version: '2.1'
     testCompile group: 'org.json', name:'json', version:'20090211'
     testCompile group: 'com.microsoft.azure', name: 'azure-storage', version: '2.1.0'
+    testCompile group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1'
 }
 
 shadowJar {

web/src/main/java/com/microsoft/applicationinsights/web/extensibility/modules/WebRequestTrackingTelemetryModule.java

@@ -21,16 +21,9 @@
 
 package com.microsoft.applicationinsights.web.extensibility.modules;
 
-import java.util.Date;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import com.microsoft.applicationinsights.common.CommonUtils;
-//import org.apache.http.HttpStatus;
-import com.microsoft.applicationinsights.web.internal.ApplicationInsightsHttpResponseWrapper;
 import com.microsoft.applicationinsights.TelemetryClient;
 import com.microsoft.applicationinsights.TelemetryConfiguration;
+import com.microsoft.applicationinsights.common.CommonUtils;
 import com.microsoft.applicationinsights.extensibility.TelemetryModule;
 import com.microsoft.applicationinsights.internal.logger.InternalLogger;
 import com.microsoft.applicationinsights.telemetry.Duration;
@@ -40,6 +33,13 @@
 import com.microsoft.applicationinsights.web.internal.correlation.InstrumentationKeyResolver;
 import com.microsoft.applicationinsights.web.internal.correlation.TelemetryCorrelationUtils;
 
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.util.Date;
+
+
 /**
  * Created by yonisha on 2/2/2015.
  */
@@ -123,7 +123,7 @@ public void onEndRequest(ServletRequest req, ServletResponse res) {
 
             long endTime = new Date().getTime();
 
-            ApplicationInsightsHttpResponseWrapper response = ((ApplicationInsightsHttpResponseWrapper)res);
+            HttpServletResponse response = (HttpServletResponse)res;
             if (response != null) {
                 telemetry.setSuccess(response.getStatus() < 400 || response.getStatus() == 401 );
                 telemetry.setResponseCode(Integer.toString(response.getStatus()));

web/src/main/java/com/microsoft/applicationinsights/web/internal/ApplicationInsightsHttpResponseWrapper.java

@@ -26,10 +26,12 @@
 import java.io.IOException;
 
 /**
+ * @deprecated No longer needed as library has upgraded to Servlet API 3.0.1
  * This class wraps the HTTP servlet response in order to store the response status code.
  * This wrapper is used to support servlet API 2.5, which lacks the api to get response status.
  * Created by yonisha on 5/27/2015.
  */
+@Deprecated
 public class ApplicationInsightsHttpResponseWrapper extends HttpServletResponseWrapper {
 
     private int httpStatusCode = SC_OK;

web/src/main/java/com/microsoft/applicationinsights/web/internal/WebRequestTrackingFilter.java

@@ -21,30 +21,29 @@
 
 package com.microsoft.applicationinsights.web.internal;
 
-import java.io.IOException;
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.util.Date;
-import java.util.LinkedList;
+import com.microsoft.applicationinsights.TelemetryClient;
+import com.microsoft.applicationinsights.TelemetryConfiguration;
+import com.microsoft.applicationinsights.agent.internal.coresync.impl.AgentTLS;
+import com.microsoft.applicationinsights.common.CommonUtils;
+import com.microsoft.applicationinsights.internal.agent.AgentConnector;
+import com.microsoft.applicationinsights.internal.logger.InternalLogger;
+import com.microsoft.applicationinsights.internal.util.ThreadLocalCleaner;
 
 import javax.servlet.Filter;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.annotation.WebFilter;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import javax.servlet.annotation.WebFilter;
-
-import com.microsoft.applicationinsights.common.CommonUtils;
-import com.microsoft.applicationinsights.TelemetryClient;
-import com.microsoft.applicationinsights.TelemetryConfiguration;
-import com.microsoft.applicationinsights.agent.internal.coresync.impl.AgentTLS;
-import com.microsoft.applicationinsights.internal.agent.AgentConnector;
-import com.microsoft.applicationinsights.internal.logger.InternalLogger;
-import com.microsoft.applicationinsights.internal.util.ThreadLocalCleaner;
+import java.io.IOException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.Date;
+import java.util.LinkedList;
 
 /**
  * Created by yonisha on 2/2/2015.
@@ -75,28 +74,35 @@ public final class WebRequestTrackingFilter implements Filter {
      * @throws ServletException Exception that can be thrown from invoking the filters chain.
      */
     public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException {
-        ApplicationInsightsHttpResponseWrapper response = new ApplicationInsightsHttpResponseWrapper((HttpServletResponse)res);
+
+        //From Servlet API 3.0 onward there is getStatus() method in HttpServletResponse class.
+        HttpServletResponse response = (HttpServletResponse)res;
         setKeyOnTLS(key);
 
         boolean isRequestProcessedSuccessfully = invokeSafeOnBeginRequest(req, response);
 
         try {
             chain.doFilter(req, response);
             invokeSafeOnEndRequest(req, response, isRequestProcessedSuccessfully);
-        } catch (ServletException se) {
-            onException(se, req, response,isRequestProcessedSuccessfully);
-            throw se;
-        } catch (IOException ioe) {
-            onException(ioe, req, response, isRequestProcessedSuccessfully);
-            throw ioe;
-        } catch (RuntimeException re) {
-            onException(re, req, response, isRequestProcessedSuccessfully);
-            throw re;
-        } finally {
+        } catch (ServletException | IOException | RuntimeException ex) {
+
+            //Explicitly set the response to 500 as the above three exceptions converts to 500
+            //later in the the chain inside the catch block of org.apache.catalina.core.StandardWrapperValue class
+            //The call to this method happens after the Filter Chain has been completed and thus response object
+            //can never have this value at this time.
+            response.setStatus(500);
+            onException(ex, req, response,isRequestProcessedSuccessfully);
+            throw ex;
+        }  finally {
             cleanup();
         }
     }
 
+    /**
+     * This constructor is used to set the Application Name in case of SpringBoot Application. In J2EE or Servlet
+     * Applications we get this value from ServletContext.
+     * @param appName The Name of the Application in case of SpringBoot Application
+     */
     public WebRequestTrackingFilter(String appName) {
         this.appName = appName;
     }

web/src/test/java/com/microsoft/applicationinsights/web/internal/ApplicationInsightsHttpResponseWrapperTests.java

@@ -23,6 +23,7 @@
 
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import javax.servlet.http.HttpServletResponse;
@@ -33,6 +34,7 @@
 /**
  * Created by yonisha on 5/27/2015.
  */
+@Ignore
 public class ApplicationInsightsHttpResponseWrapperTests {
 
     private HttpServletResponse responseMock = mock(HttpServletResponse.class);

web/src/test/java/com/microsoft/applicationinsights/web/internal/WebRequestTrackingFilterTests.java

@@ -22,18 +22,18 @@
 package com.microsoft.applicationinsights.web.internal;
 
 import com.microsoft.applicationinsights.TelemetryClient;
-import com.microsoft.applicationinsights.internal.reflect.ClassDataUtils;
-import com.microsoft.applicationinsights.internal.reflect.ClassDataVerifier;
+import com.microsoft.applicationinsights.web.utils.ServletUtils;
 import org.junit.Assert;
 import org.junit.Test;
-import javax.servlet.*;
-import javax.servlet.http.HttpServletResponse;
-
 import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
-import com.microsoft.applicationinsights.web.utils.ServletUtils;
 
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
 import java.io.IOException;
 import java.lang.reflect.Field;
 
@@ -163,7 +163,7 @@ private void testException(FilterAndTelemetryClientMock createdData, Exception e
         try {
             FilterChain chain = mock(FilterChain.class);
 
-            ServletRequest request = ServletUtils.generateDummyServletRequest();
+            ServletRequest request =  ServletUtils.generateDummyServletRequest();
             ServletResponse response = ServletUtils.generateDummyServletResponse();
             Mockito.doThrow(expectedException).when(chain).doFilter(eq(request), any(ServletResponse.class));