fix: harden HTTPServer failure handling #2283 #2284

integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/ExporterIT.java

@@ -157,7 +157,11 @@ void testErrorHandling() throws IOException {
     start("error");
     Response response = scrape("GET", "");
     assertThat(response.status).isEqualTo(500);
-    assertThat(response.stringBody()).contains("Simulating an error.");
+    assertErrorResponseBody(response.stringBody());
+  }
+
+  protected void assertErrorResponseBody(String body) {
+    assertThat(body).contains("Simulating an error.");
   }
 
   @Test

integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/HttpServerIT.java

@@ -1,10 +1,19 @@
 package io.prometheus.metrics.it.exporter.test;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 import java.io.IOException;
 import java.net.URISyntaxException;
 
 class HttpServerIT extends ExporterIT {
   public HttpServerIT() throws IOException, URISyntaxException {
     super("exporter-httpserver-sample");
   }
+
+  @Override
+  protected void assertErrorResponseBody(String body) {
+    assertThat(body)
+        .isEqualTo("An internal error occurred while scraping metrics.\n")
+        .doesNotContain("Simulating an error.");
+  }
 }

prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HTTPServer.java

@@ -12,14 +12,13 @@
 import io.prometheus.metrics.model.registry.PrometheusRegistry;
 import java.io.Closeable;
 import java.io.IOException;
-import java.io.InputStream;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
+import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nullable;
@@ -39,6 +38,10 @@
 @StableApi
 public class HTTPServer implements Closeable {
 
+  private static final int DEFAULT_MIN_THREADS = 1;
+  private static final int DEFAULT_MAX_THREADS = 10;
+  private static final int DEFAULT_QUEUE_SIZE = 100;
+
   static {
     if (!System.getProperties().containsKey("sun.net.httpserver.maxReqTime")) {
       System.setProperty("sun.net.httpserver.maxReqTime", "60");
@@ -153,22 +156,13 @@ public void handle(HttpExchange exchange) throws IOException {
             }
           }
         } else {
-          drainInputAndClose(exchange);
+          exchange.getRequestBody().close();
           exchange.sendResponseHeaders(403, -1);
         }
       }
     };
   }
 
-  private void drainInputAndClose(HttpExchange httpExchange) throws IOException {
-    InputStream inputStream = httpExchange.getRequestBody();
-    byte[] b = new byte[4096];
-    while (inputStream.read(b) != -1) {
-      // nop
-    }
-    inputStream.close();
-  }
-
   /** Stop the HTTP server. Same as {@link #close()}. */
   public void stop() {
     close();
@@ -337,13 +331,12 @@ private ExecutorService makeExecutorService() {
         return executorService;
       } else {
         return new ThreadPoolExecutor(
-            1,
-            10,
+            DEFAULT_MIN_THREADS,
+            DEFAULT_MAX_THREADS,
             120,
             TimeUnit.SECONDS,
-            new SynchronousQueue<>(true),
-            NamedDaemonThreadFactory.defaultThreadFactory(true),
-            new BlockingRejectedExecutionHandler());
+            new ArrayBlockingQueue<>(DEFAULT_QUEUE_SIZE),
+            NamedDaemonThreadFactory.defaultThreadFactory(true));
       }
     }
 

prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HttpExchangeAdapter.java

@@ -6,8 +6,6 @@
 import io.prometheus.metrics.exporter.common.PrometheusHttpResponse;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.io.PrintWriter;
-import java.io.StringWriter;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
@@ -18,6 +16,10 @@
 
 public class HttpExchangeAdapter implements PrometheusHttpExchange {
 
+  private static final Logger logger = Logger.getLogger(HttpExchangeAdapter.class.getName());
+  private static final byte[] ERROR_RESPONSE =
+      "An internal error occurred while scraping metrics.\n".getBytes(StandardCharsets.UTF_8);
+
   private final HttpExchange httpExchange;
   private final HttpRequest request = new HttpRequest();
   private final HttpResponse response = new HttpResponse();
@@ -92,52 +94,42 @@ public HttpResponse getResponse() {
 
   @Override
   public void handleException(IOException e) throws IOException {
-    sendErrorResponseWithStackTrace(e);
+    sendErrorResponse(e);
   }
 
   @Override
   public void handleException(RuntimeException e) {
-    sendErrorResponseWithStackTrace(e);
+    sendErrorResponse(e);
   }
 
-  private void sendErrorResponseWithStackTrace(Exception requestHandlerException) {
+  private void sendErrorResponse(Exception requestHandlerException) {
     if (!responseSent) {
       responseSent = true;
+      logger.log(
+          Level.SEVERE,
+          "The Prometheus metrics HTTPServer caught an Exception during scrape.",
+          requestHandlerException);
       try {
-        StringWriter stringWriter = new StringWriter();
-        PrintWriter printWriter = new PrintWriter(stringWriter);
-        printWriter.write("An Exception occurred while scraping metrics: ");
-        requestHandlerException.printStackTrace(new PrintWriter(printWriter));
-        byte[] stackTrace = stringWriter.toString().getBytes(StandardCharsets.UTF_8);
         httpExchange.getResponseHeaders().set("Content-Type", "text/plain; charset=utf-8");
-        httpExchange.sendResponseHeaders(500, stackTrace.length);
-        httpExchange.getResponseBody().write(stackTrace);
+        httpExchange.sendResponseHeaders(500, ERROR_RESPONSE.length);
+        httpExchange.getResponseBody().write(ERROR_RESPONSE);
       } catch (IOException errorWriterException) {
-        // We want to avoid logging so that we don't mess with application logs when the HTTPServer
-        // is used in a Java agent.
-        // However, if we can't even send an error response to the client there's nothing we can do
-        // but logging a message.
-        Logger.getLogger(this.getClass().getName())
-            .log(
-                Level.SEVERE,
-                "The Prometheus metrics HTTPServer caught an Exception during scrape and "
-                    + "failed to send an error response to the client.",
-                errorWriterException);
-        Logger.getLogger(this.getClass().getName())
-            .log(
-                Level.SEVERE,
-                "Original Exception that caused the Prometheus scrape error:",
-                requestHandlerException);
+        // If we can't even send an error response to the client, logging is the only remaining
+        // signal.
+        logger.log(
+            Level.SEVERE,
+            "The Prometheus metrics HTTPServer caught an Exception during scrape and "
+                + "failed to send an error response to the client.",
+            errorWriterException);
       }
     } else {
       // If the exception occurs after response headers have been sent, it's too late to respond
       // with HTTP 500.
-      Logger.getLogger(this.getClass().getName())
-          .log(
-              Level.SEVERE,
-              "The Prometheus metrics HTTPServer caught an Exception while trying to send "
-                  + "the metrics response.",
-              requestHandlerException);
+      logger.log(
+          Level.SEVERE,
+          "The Prometheus metrics HTTPServer caught an Exception while trying to send "
+              + "the metrics response.",
+          requestHandlerException);
     }
   }
 

prometheus-metrics-exporter-httpserver/src/test/java/io/prometheus/metrics/exporter/httpserver/HTTPServerTest.java

@@ -25,6 +25,7 @@
 import java.security.Principal;
 import java.util.List;
 import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadPoolExecutor;
 import javax.net.ssl.SSLContext;
 import javax.security.auth.Subject;
 import org.junit.jupiter.api.BeforeEach;
@@ -158,6 +159,21 @@ void metricsCustomRootPath() throws Exception {
         "my_counter_total 1.0");
   }
 
+  @Test
+  void defaultExecutorHasBoundedQueueAndNonBlockingRejection() throws Exception {
+    HTTPServer server = HTTPServer.builder().port(0).buildAndStart();
+    try {
+      assertThat(server.executorService).isInstanceOf(ThreadPoolExecutor.class);
+      ThreadPoolExecutor executor = (ThreadPoolExecutor) server.executorService;
+      assertThat(executor.getMaximumPoolSize()).isEqualTo(10);
+      assertThat(executor.getQueue().remainingCapacity()).isEqualTo(100);
+      assertThat(executor.getRejectedExecutionHandler())
+          .isInstanceOf(ThreadPoolExecutor.AbortPolicy.class);
+    } finally {
+      server.stop();
+    }
+  }
+
   @Test
   void registryThrows() throws Exception {
     HTTPServer server =
@@ -171,7 +187,13 @@ public MetricSnapshots scrape(PrometheusScrapeRequest scrapeRequest) {
                   }
                 })
             .buildAndStart();
-    run(server, "/metrics", 500, "An Exception occurred while scraping metrics");
+    run(
+        server,
+        "/metrics",
+        500,
+        "An internal error occurred while scraping metrics.",
+        "IllegalStateException",
+        "test");
   }
 
   @Test
@@ -237,6 +259,16 @@ void healthDisabled() throws Exception {
   private static void run(
       HTTPServer server, String path, int expectedStatusCode, String expectedBody)
       throws Exception {
+    run(server, path, expectedStatusCode, expectedBody, new String[0]);
+  }
+
+  private static void run(
+      HTTPServer server,
+      String path,
+      int expectedStatusCode,
+      String expectedBody,
+      String... unexpectedBody)
+      throws Exception {
     // we cannot use try-with-resources or even client.close(), or the test will fail with Java 17
     @SuppressWarnings("resource")
     final HttpClient client = HttpClient.newBuilder().build();
@@ -248,6 +280,9 @@ private static void run(
           client.send(request, HttpResponse.BodyHandlers.ofString());
       assertThat(response.statusCode()).isEqualTo(expectedStatusCode);
       assertThat(response.body()).contains(expectedBody);
+      if (unexpectedBody.length > 0) {
+        assertThat(response.body()).doesNotContain(unexpectedBody);
+      }
     } finally {
       server.stop();
     }

prometheus-metrics-exporter-httpserver/src/test/java/io/prometheus/metrics/exporter/httpserver/HttpExchangeAdapterTest.java

@@ -2,11 +2,14 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import com.sun.net.httpserver.Headers;
 import com.sun.net.httpserver.HttpExchange;
+import java.io.ByteArrayOutputStream;
 import java.net.URI;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 import org.junit.jupiter.api.Test;
 
@@ -46,4 +49,24 @@ void getHeadersWhenNotPresent() {
     HttpExchangeAdapter adapter = new HttpExchangeAdapter(httpExchange);
     assertThat(adapter.getRequest().getHeaders("Accept").hasMoreElements()).isFalse();
   }
+
+  @Test
+  void handleExceptionReturnsGenericMessageWithoutStackTrace() throws Exception {
+    HttpExchange httpExchange = mock(HttpExchange.class);
+    Headers headers = new Headers();
+    ByteArrayOutputStream responseBody = new ByteArrayOutputStream();
+    when(httpExchange.getResponseHeaders()).thenReturn(headers);
+    when(httpExchange.getResponseBody()).thenReturn(responseBody);
+    HttpExchangeAdapter adapter = new HttpExchangeAdapter(httpExchange);
+
+    adapter.handleException(new IllegalStateException("secret failure"));
+
+    String body = new String(responseBody.toByteArray(), StandardCharsets.UTF_8);
+    assertThat(body).isEqualTo("An internal error occurred while scraping metrics.\n");
+    assertThat(body).doesNotContain("IllegalStateException");
+    assertThat(body).doesNotContain("secret failure");
+    assertThat(body).doesNotContain("at ");
+    assertThat(headers.getFirst("Content-Type")).isEqualTo("text/plain; charset=utf-8");
+    verify(httpExchange).sendResponseHeaders(500, body.getBytes(StandardCharsets.UTF_8).length);
+  }
 }