diff --git a/domains/games/apis/one_d4/BUILD.bazel b/domains/games/apis/one_d4/BUILD.bazel index cede3e8c..98bce2f9 100644 --- a/domains/games/apis/one_d4/BUILD.bazel +++ b/domains/games/apis/one_d4/BUILD.bazel @@ -260,7 +260,9 @@ java_library( artifact("io.micronaut:micronaut-http"), artifact("io.micronaut:micronaut-http-server"), artifact("io.micronaut:micronaut-inject"), + artifact("io.micronaut:micronaut-json-core"), artifact("io.micronaut.jaxrs:micronaut-jaxrs-server"), + artifact("tools.jackson.core:jackson-databind"), artifact("jakarta.inject:jakarta-inject-api"), artifact("jakarta.ws.rs:jakarta-ws.rs-api"), artifact("org.slf4j:slf4j-api"), diff --git a/domains/games/apis/one_d4/src/main/java/com/muchq/games/one_d4/api/ErrorHandler.java b/domains/games/apis/one_d4/src/main/java/com/muchq/games/one_d4/api/ErrorHandler.java index 2616af3b..2ca974b4 100644 --- a/domains/games/apis/one_d4/src/main/java/com/muchq/games/one_d4/api/ErrorHandler.java +++ b/domains/games/apis/one_d4/src/main/java/com/muchq/games/one_d4/api/ErrorHandler.java @@ -8,11 +8,13 @@ import io.micronaut.http.annotation.Error; import io.micronaut.http.server.exceptions.NotFoundException; import io.micronaut.http.server.exceptions.UnsupportedMediaException; +import io.micronaut.json.JsonSyntaxException; import java.util.LinkedHashMap; import java.util.Map; import java.util.NoSuchElementException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import tools.jackson.databind.DatabindException; @Controller public class ErrorHandler { @@ -45,6 +47,23 @@ public HttpResponse> handleRouteMiss( return HttpResponse.notFound(Map.of("error", "Not found")); } + // A body that is not JSON is the caller's mistake, like a query that does not parse: 400 + // with the same envelope, not a 500 with a stack trace and a Sentry event per typo. + @Error(global = true, exception = JsonSyntaxException.class) + public HttpResponse> handleBadJson( + HttpRequest request, JsonSyntaxException ex) { + return HttpResponse.badRequest(Map.of("error", "Request body is not valid JSON")); + } + + // Valid JSON that does not fit the request — a string where a number goes — reaches the + // handler as Jackson's own exception rather than Micronaut's; the caller's mistake all the same. + @Error(global = true, exception = DatabindException.class) + public HttpResponse> handleUnbindableBody( + HttpRequest request, DatabindException ex) { + return HttpResponse.badRequest( + Map.of("error", "Request body does not match the request shape")); + } + @Error(global = true, exception = UnsupportedMediaException.class) public HttpResponse> handleUnsupportedMedia( HttpRequest request, UnsupportedMediaException ex) { diff --git a/domains/games/apis/one_d4/src/test/java/com/muchq/games/one_d4/api/QueryErrorWireTest.java b/domains/games/apis/one_d4/src/test/java/com/muchq/games/one_d4/api/QueryErrorWireTest.java index 75d97924..537a794c 100644 --- a/domains/games/apis/one_d4/src/test/java/com/muchq/games/one_d4/api/QueryErrorWireTest.java +++ b/domains/games/apis/one_d4/src/test/java/com/muchq/games/one_d4/api/QueryErrorWireTest.java @@ -54,7 +54,10 @@ public void tearDown() { private HttpResponse postQuery(String chessql) throws Exception { String escaped = chessql.replace("\\", "\\\\").replace("\"", "\\\""); - String body = "{\"query\":\"" + escaped + "\",\"limit\":10,\"offset\":0}"; + return postBody("{\"query\":\"" + escaped + "\",\"limit\":10,\"offset\":0}"); + } + + private HttpResponse postBody(String body) throws Exception { HttpRequest request = HttpRequest.newBuilder() .uri(URI.create(baseUrl + "/v1/query")) @@ -113,4 +116,29 @@ public void echoedQuoteSurvivesTheJsonEnvelope() throws Exception { JsonNode envelope = MAPPER.readTree(response.body()); assertThat(envelope.get("error").asText()).contains("double quotes").contains("B\"90"); } + + /** + * A body that is not JSON at all is the caller's mistake, and comes back in the same envelope a + * bad query does — not a 500, which pages for every client typo (#1472). The message is fixed + * rather than the parser's: byte offsets and "REDACTED" source markers are not something a caller + * can act on. + */ + @Test + public void malformedJson_returns400WithTheErrorEnvelope() throws Exception { + HttpResponse response = postBody("{\"query\":"); + + assertThat(response.statusCode()).isEqualTo(400); + assertThat(MAPPER.readTree(response.body()).get("error").asText()) + .isEqualTo("Request body is not valid JSON"); + } + + /** Valid JSON of the wrong shape is the same class of mistake, and the same 400. */ + @Test + public void aWronglyTypedField_returns400WithTheErrorEnvelope() throws Exception { + HttpResponse response = postBody("{\"query\":\"motif(pin)\",\"limit\":\"ten\"}"); + + assertThat(response.statusCode()).isEqualTo(400); + assertThat(MAPPER.readTree(response.body()).get("error").asText()) + .isEqualTo("Request body does not match the request shape"); + } } diff --git a/domains/games/apis/one_d4/src/test/java/com/muchq/games/one_d4/api/QueryEventWireTest.java b/domains/games/apis/one_d4/src/test/java/com/muchq/games/one_d4/api/QueryEventWireTest.java index ffc325e0..ecc0ee89 100644 --- a/domains/games/apis/one_d4/src/test/java/com/muchq/games/one_d4/api/QueryEventWireTest.java +++ b/domains/games/apis/one_d4/src/test/java/com/muchq/games/one_d4/api/QueryEventWireTest.java @@ -109,9 +109,8 @@ public void requestsRejectedBeforeTheHandlerWriteNoEvent() throws Exception { assertThat(post("text/plain", A_QUERY).statusCode()).isEqualTo(415); assertThat(captured.list).isEmpty(); - // Malformed JSON is answered before binding too (today as a 500, which is its own problem, - // not this one's); either way no handler ran and no event may say one did. - assertThat(post("application/json", "{\"query\":").statusCode()).isGreaterThanOrEqualTo(400); + // Malformed JSON is answered before binding too; no handler ran and no event may say one did. + assertThat(post("application/json", "{\"query\":").statusCode()).isEqualTo(400); assertThat(captured.list).isEmpty(); } }