Fix NPE in InstrumentedHandler#doStop (Jetty 9, 10, 11) (#3379)

Fixes #3325

Author: joschi

metrics-jetty10/src/main/java/io/dropwizard/metrics/jetty10/InstrumentedHandler.java

@@ -273,9 +273,12 @@ protected void doStop() throws Exception {
         metricRegistry.remove(name(prefix, NAME_PERCENT_5XX_1M));
         metricRegistry.remove(name(prefix, NAME_PERCENT_5XX_5M));
         metricRegistry.remove(name(prefix, NAME_PERCENT_5XX_15M));
-        responseCodeMeters.keySet().stream()
-                .map(sc -> name(getMetricPrefix(), String.format("%d-responses", sc)))
-                .forEach(m -> metricRegistry.remove(m));
+
+        if (responseCodeMeters != null) {
+            responseCodeMeters.keySet().stream()
+                    .map(sc -> name(getMetricPrefix(), String.format("%d-responses", sc)))
+                    .forEach(metricRegistry::remove);
+        }
         super.doStop();
     }
 

metrics-jetty10/src/test/java/io/dropwizard/metrics/jetty10/InstrumentedHandlerTest.java

@@ -24,6 +24,7 @@
 
 import static com.codahale.metrics.annotation.ResponseMeteredLevel.ALL;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 public class InstrumentedHandlerTest {
     private final HttpClient client = new HttpClient();
@@ -111,6 +112,14 @@ public void responseTimesAreRecordedForBlockingResponses() throws Exception {
         assertResponseTimesValid();
     }
 
+    @Test
+    public void doStopDoesNotThrowNPE() throws Exception {
+        InstrumentedHandler handler = new InstrumentedHandler(registry, null, ALL);
+        handler.setHandler(new TestHandler());
+
+        assertThatCode(handler::doStop).doesNotThrowAnyException();
+    }
+
     @Test
     @Ignore("flaky on virtual machines")
     public void responseTimesAreRecordedForAsyncResponses() throws Exception {

metrics-jetty11/src/main/java/io/dropwizard/metrics/jetty11/InstrumentedHandler.java

@@ -273,9 +273,12 @@ protected void doStop() throws Exception {
         metricRegistry.remove(name(prefix, NAME_PERCENT_5XX_1M));
         metricRegistry.remove(name(prefix, NAME_PERCENT_5XX_5M));
         metricRegistry.remove(name(prefix, NAME_PERCENT_5XX_15M));
-        responseCodeMeters.keySet().stream()
-                .map(sc -> name(getMetricPrefix(), String.format("%d-responses", sc)))
-                .forEach(m -> metricRegistry.remove(m));
+
+        if (responseCodeMeters != null) {
+            responseCodeMeters.keySet().stream()
+                    .map(sc -> name(getMetricPrefix(), String.format("%d-responses", sc)))
+                    .forEach(metricRegistry::remove);
+        }
         super.doStop();
     }
 

metrics-jetty11/src/test/java/io/dropwizard/metrics/jetty11/InstrumentedHandlerTest.java

@@ -24,6 +24,7 @@
 
 import static com.codahale.metrics.annotation.ResponseMeteredLevel.ALL;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 public class InstrumentedHandlerTest {
     private final HttpClient client = new HttpClient();
@@ -111,6 +112,14 @@ public void responseTimesAreRecordedForBlockingResponses() throws Exception {
         assertResponseTimesValid();
     }
 
+    @Test
+    public void doStopDoesNotThrowNPE() throws Exception {
+        InstrumentedHandler handler = new InstrumentedHandler(registry, null, ALL);
+        handler.setHandler(new TestHandler());
+
+        assertThatCode(handler::doStop).doesNotThrowAnyException();
+    }
+
     @Test
     @Ignore("flaky on virtual machines")
     public void responseTimesAreRecordedForAsyncResponses() throws Exception {

metrics-jetty9/src/main/java/com/codahale/metrics/jetty9/InstrumentedHandler.java

@@ -281,9 +281,12 @@ protected void doStop() throws Exception {
         metricRegistry.remove(name(prefix, NAME_PERCENT_5XX_1M));
         metricRegistry.remove(name(prefix, NAME_PERCENT_5XX_5M));
         metricRegistry.remove(name(prefix, NAME_PERCENT_5XX_15M));
-        responseCodeMeters.keySet().stream()
-                .map(sc -> name(getMetricPrefix(), String.format("%d-responses", sc)))
-                .forEach(m -> metricRegistry.remove(m));
+
+        if (responseCodeMeters != null) {
+            responseCodeMeters.keySet().stream()
+                    .map(sc -> name(getMetricPrefix(), String.format("%d-responses", sc)))
+                    .forEach(metricRegistry::remove);
+        }
         super.doStop();
     }
 

metrics-jetty9/src/test/java/com/codahale/metrics/jetty9/InstrumentedHandlerTest.java

@@ -24,6 +24,7 @@
 
 import static com.codahale.metrics.annotation.ResponseMeteredLevel.ALL;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 public class InstrumentedHandlerTest {
     private final HttpClient client = new HttpClient();
@@ -111,6 +112,14 @@ public void responseTimesAreRecordedForBlockingResponses() throws Exception {
         assertResponseTimesValid();
     }
 
+    @Test
+    public void doStopDoesNotThrowNPE() throws Exception {
+        InstrumentedHandler handler = new InstrumentedHandler(registry, null, ALL);
+        handler.setHandler(new TestHandler());
+
+        assertThatCode(handler::doStop).doesNotThrowAnyException();
+    }
+
     @Test
     @Ignore("flaky on virtual machines")
     public void responseTimesAreRecordedForAsyncResponses() throws Exception {