From 696f59cb6a45e7db6e0c0d843af44c9ccbcd7f09 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 3 Jun 2019 16:01:22 +0200 Subject: Ensure always valid json from JsonMetricsRenderingException --- .../metricsproxy/http/GenericMetricsHandler.java | 18 ++++------ .../metric/model/json/GenericJsonModel.java | 7 +--- .../metric/model/json/JsonRenderingException.java | 41 ++++++++++++++++++++++ .../model/json/JsonRenderingExceptionTest.java | 27 ++++++++++++++ 4 files changed, 75 insertions(+), 18 deletions(-) create mode 100644 metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/JsonRenderingException.java create mode 100644 metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/JsonRenderingExceptionTest.java (limited to 'metrics-proxy') diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java index 488f26b7af7..cadfc053b94 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java @@ -6,14 +6,12 @@ package ai.vespa.metricsproxy.http; import ai.vespa.metricsproxy.core.MetricsManager; import ai.vespa.metricsproxy.metric.model.MetricsPacket; -import ai.vespa.metricsproxy.metric.model.json.GenericJsonModel; -import ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil; +import ai.vespa.metricsproxy.metric.model.json.JsonRenderingException; import ai.vespa.metricsproxy.service.VespaServices; import com.google.inject.Inject; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; -import com.yahoo.yolean.Exceptions; import java.io.IOException; import java.io.OutputStream; @@ -22,6 +20,8 @@ import java.time.Instant; import java.util.List; import java.util.concurrent.Executor; +import static ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil.toGenericJsonModel; + /** * Handler exposing the generic metrics format via http. * @@ -43,9 +43,9 @@ public class GenericMetricsHandler extends ThreadedHttpRequestHandler { public HttpResponse handle(HttpRequest request) { try { List metrics = metricsManager.getMetrics(vespaServices.getVespaServices(), Instant.now()); - return new Response(200, GenericJsonUtil.toGenericJsonModel(metrics).serialize()); - } catch (GenericJsonModel.JsonMetricsRenderingException e) { - return new ErrorResponse(500, Exceptions.toMessageString(e)); + return new Response(200, toGenericJsonModel(metrics).serialize()); + } catch (JsonRenderingException e) { + return new Response(500, e.getMessageAsJson()); } } @@ -68,10 +68,4 @@ public class GenericMetricsHandler extends ThreadedHttpRequestHandler { } } - private static class ErrorResponse extends Response { - ErrorResponse(int code, String data) { - super(code, "{\"error\":\"" + data + "\"}"); - } - } - } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java index 14f085ff388..03e353cab22 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java @@ -38,13 +38,8 @@ public class GenericJsonModel { return mapper.writeValueAsString(this); } catch (IOException e) { log.log(Level.WARNING, "Got exception when rendering metrics:", e); - throw new JsonMetricsRenderingException("Could not render metrics. Check the log for details."); + throw new JsonRenderingException("Could not render metrics. Check the log for details."); } } - public static class JsonMetricsRenderingException extends RuntimeException { - JsonMetricsRenderingException(String message) { - super(message); - } - } } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/JsonRenderingException.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/JsonRenderingException.java new file mode 100644 index 00000000000..05df3bb12c9 --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/JsonRenderingException.java @@ -0,0 +1,41 @@ +/* + * Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + */ + +package ai.vespa.metricsproxy.metric.model.json; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.io.IOException; +import java.util.Map; +import java.util.logging.Logger; + +import static java.util.logging.Level.WARNING; + +/** + * An exception whose message is always valid json or, if all fails, a plain text string. + * + * @author gjoranv + */ +public class JsonRenderingException extends RuntimeException { + + private static Logger log = Logger.getLogger(JsonRenderingException.class.getName()); + + JsonRenderingException(String message) { + super(message); + } + + public String getMessageAsJson() { + return wrap(getMessage()); + } + + private static String wrap(String message) { + try { + return new ObjectMapper().writeValueAsString(Map.of("error", message)); + } catch (IOException e) { + log.log(WARNING, "Could not encode error message to json:", e); + return "Could not encode error message to json, check the log for details."; + } + } + +} diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/JsonRenderingExceptionTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/JsonRenderingExceptionTest.java new file mode 100644 index 00000000000..9b502d3f67a --- /dev/null +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/JsonRenderingExceptionTest.java @@ -0,0 +1,27 @@ +/* + * Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + */ + +package ai.vespa.metricsproxy.metric.model.json; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * @author gjoranv + */ +public class JsonRenderingExceptionTest { + + @Test + public void error_message_is_wrapped_in_json_object() { + var exception = new JsonRenderingException("bad"); + assertEquals("{\"error\":\"bad\"}", exception.getMessageAsJson()); + } + + @Test + public void quotes_are_escaped() { + var exception = new JsonRenderingException("Message \" with \" embedded quotes."); + assertEquals("{\"error\":\"Message \\\" with \\\" embedded quotes.\"}", exception.getMessageAsJson()); + } +} -- cgit v1.2.3