From 5d82b0d39c60ea311dd3ce0b504ed3b77f056608 Mon Sep 17 00:00:00 2001 From: Jeremy Grelle Date: Wed, 25 Oct 2023 17:47:35 -0400 Subject: [PATCH 1/6] Tck: Test error handling with Flux controller methods --- .../micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java diff --git a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java new file mode 100644 index 00000000000..6f2efde2808 --- /dev/null +++ b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java @@ -0,0 +1,2 @@ +package io.micronaut.http.server.tck.tests;public class ErrorHandlerFluxTest { +} From 9c58238eabcf30206c3ffb07d505d281305dbb62 Mon Sep 17 00:00:00 2001 From: Jeremy Grelle Date: Thu, 26 Oct 2023 07:02:41 -0400 Subject: [PATCH 2/6] Missing test body. --- .../tck/tests/ErrorHandlerFluxTest.java | 134 +++++++++++++++++- 1 file changed, 133 insertions(+), 1 deletion(-) diff --git a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java index 6f2efde2808..5c0c301d76e 100644 --- a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java +++ b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java @@ -1,2 +1,134 @@ -package io.micronaut.http.server.tck.tests;public class ErrorHandlerFluxTest { +package io.micronaut.http.server.tck.tests; + +import io.micronaut.context.annotation.Requires; +import io.micronaut.core.async.annotation.SingleResult; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.annotation.Controller; +import io.micronaut.http.annotation.Error; +import io.micronaut.http.annotation.Get; +import io.micronaut.http.client.exceptions.ResponseClosedException; +import io.micronaut.http.tck.AssertionUtils; +import io.micronaut.http.tck.HttpResponseAssertion; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import reactor.core.publisher.Flux; + +import java.io.IOException; + +import static io.micronaut.http.tck.TestScenario.asserts; + +@SuppressWarnings({ + "java:S5960", // We're allowed assertions, as these are used in tests only + "checkstyle:MissingJavadocType", + "checkstyle:DesignForExtension" +}) +public class ErrorHandlerFluxTest { + + public static final String SPEC_NAME = "ErrorHandlerFluxTest"; + + @Test + void testErrorHandlerWithFluxThrownException() throws IOException { + asserts(SPEC_NAME, + HttpRequest.GET("/errors/flux-exception"), + (server, request) -> AssertionUtils.assertThrows(server, request, HttpResponseAssertion.builder() + .status(HttpStatus.BAD_REQUEST) + .body("Your request is erroneous: Cannot process request.") + .build())); + } + + @Test + void testErrorHandlerWithFluxSingleResultThrownException() throws IOException { + asserts(SPEC_NAME, + HttpRequest.GET("/errors/flux-single-exception"), + (server, request) -> AssertionUtils.assertThrows(server, request, HttpResponseAssertion.builder() + .status(HttpStatus.BAD_REQUEST) + .body("Your request is erroneous: Cannot process request.") + .build())); + } + + @Test + void testErrorHandlerWithFluxChunkedSignaledImmediateError() throws IOException { + asserts(SPEC_NAME, + HttpRequest.GET("/errors/flux-chunked-immediate-error"), + (server, request) -> AssertionUtils.assertThrows(server, request, HttpResponseAssertion.builder() + .status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Internal Server Error: Cannot process request.") + .build())); + } + + @Test + void testErrorHandlerWithFluxChunkedSignaledDelayedError() throws IOException { + asserts(SPEC_NAME, + HttpRequest.GET("/errors/flux-chunked-delayed-error"), + (server, request) -> { + Executable e = () -> server.exchange(request); + Assertions.assertThrows(ResponseClosedException.class, e); + }); + } + + @Test + void testErrorHandlerWithFluxSingleResultSignaledError() throws IOException { + asserts(SPEC_NAME, + HttpRequest.GET("/errors/flux-single-error"), + (server, request) -> AssertionUtils.assertThrows(server, request, HttpResponseAssertion.builder() + .status(HttpStatus.BAD_REQUEST) + .body("Your request is erroneous: Cannot process request.") + .build())); + } + + @Requires(property = "spec.name", value = SPEC_NAME) + @Controller("/errors") + static class ErrorController { + + @Get("/flux-exception") + Flux fluxException() { + throw new MyTestException("Cannot process request."); + } + + @Get("/flux-single-exception") + @SingleResult + Flux fluxSingleException() { + throw new MyTestException("Cannot process request."); + } + + @Get("/flux-single-error") + @SingleResult + Flux fluxSingleError() { + return Flux.error(new MyTestException("Cannot process request.")); + } + + @Get("/flux-chunked-immediate-error") + Flux fluxChunkedImmediateError() { + return Flux.error(new MyTestException("Cannot process request.")); + } + + @Get("/flux-chunked-delayed-error") + Flux fluxChunkedDelayedError() { + return Flux.just("1", "2", "3").handle((data, sink) -> { + if (data.equals("3")) { + sink.error(new MyTestException("Cannot process request.")); + } else { + sink.next(data); + } + }); + } + + @Error(global = true) + public HttpResponse entityNotFoundHandler(HttpRequest request, MyTestException exception) { + var error = "Your request is erroneous: " + exception.getMessage(); + return HttpResponse.status(HttpStatus.BAD_REQUEST, "Bad request") + .body(error); + } + + } + + static class MyTestException extends RuntimeException { + + public MyTestException(String message) { + super(message); + } + } } From bec1f1583d89ac736f67e15759d09318173a2d12 Mon Sep 17 00:00:00 2001 From: Jeremy Grelle Date: Thu, 26 Oct 2023 09:48:27 -0400 Subject: [PATCH 3/6] Make test more flexible to work with JDK client. --- .../micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java index 5c0c301d76e..df75f1685e9 100644 --- a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java +++ b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java @@ -8,7 +8,7 @@ import io.micronaut.http.annotation.Controller; import io.micronaut.http.annotation.Error; import io.micronaut.http.annotation.Get; -import io.micronaut.http.client.exceptions.ResponseClosedException; +import io.micronaut.http.client.exceptions.HttpClientException; import io.micronaut.http.tck.AssertionUtils; import io.micronaut.http.tck.HttpResponseAssertion; import org.junit.jupiter.api.Assertions; @@ -65,7 +65,7 @@ void testErrorHandlerWithFluxChunkedSignaledDelayedError() throws IOException { HttpRequest.GET("/errors/flux-chunked-delayed-error"), (server, request) -> { Executable e = () -> server.exchange(request); - Assertions.assertThrows(ResponseClosedException.class, e); + Assertions.assertThrows(HttpClientException.class, e); }); } From 326fdd21062813b7ed895fafe8820402ddf1e17f Mon Sep 17 00:00:00 2001 From: Jeremy Grelle Date: Thu, 26 Oct 2023 09:50:30 -0400 Subject: [PATCH 4/6] Give error handler method a more appropriate name. --- .../micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java index df75f1685e9..45469f4a3d1 100644 --- a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java +++ b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java @@ -117,7 +117,7 @@ Flux fluxChunkedDelayedError() { } @Error(global = true) - public HttpResponse entityNotFoundHandler(HttpRequest request, MyTestException exception) { + public HttpResponse handleMyTestException(HttpRequest request, MyTestException exception) { var error = "Your request is erroneous: " + exception.getMessage(); return HttpResponse.status(HttpStatus.BAD_REQUEST, "Bad request") .body(error); From 06e9578b3ae12fca9b012a5ab65d0dc32de6612a Mon Sep 17 00:00:00 2001 From: Jeremy Grelle Date: Thu, 26 Oct 2023 11:12:58 -0400 Subject: [PATCH 5/6] Add missing license header. --- .../server/tck/tests/ErrorHandlerFluxTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java index 45469f4a3d1..b36eaa74427 100644 --- a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java +++ b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java @@ -1,3 +1,18 @@ +/* + * Copyright 2017-2023 original authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package io.micronaut.http.server.tck.tests; import io.micronaut.context.annotation.Requires; From 828830fbc0dd86288a2a6e3e11745d2c3c06d9c8 Mon Sep 17 00:00:00 2001 From: Jeremy Grelle Date: Thu, 26 Oct 2023 14:01:29 -0400 Subject: [PATCH 6/6] Test improvements The test is updated such that the `@Error` handler returns a more unusual status code (418) as a means of ensuring in the assertions that the handler was definitely invoked. A comment is added to the `testErrorHandlerWithFluxChunkedSignaledImmediateError` test method that explains the currently demonstrated behavior vs what would be preferred in order to be less surprising in behavior. --- .../http/server/tck/tests/ErrorHandlerFluxTest.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java index b36eaa74427..857a2d08908 100644 --- a/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java +++ b/http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/ErrorHandlerFluxTest.java @@ -49,7 +49,7 @@ void testErrorHandlerWithFluxThrownException() throws IOException { asserts(SPEC_NAME, HttpRequest.GET("/errors/flux-exception"), (server, request) -> AssertionUtils.assertThrows(server, request, HttpResponseAssertion.builder() - .status(HttpStatus.BAD_REQUEST) + .status(HttpStatus.I_AM_A_TEAPOT) .body("Your request is erroneous: Cannot process request.") .build())); } @@ -59,13 +59,17 @@ void testErrorHandlerWithFluxSingleResultThrownException() throws IOException { asserts(SPEC_NAME, HttpRequest.GET("/errors/flux-single-exception"), (server, request) -> AssertionUtils.assertThrows(server, request, HttpResponseAssertion.builder() - .status(HttpStatus.BAD_REQUEST) + .status(HttpStatus.I_AM_A_TEAPOT) .body("Your request is erroneous: Cannot process request.") .build())); } @Test void testErrorHandlerWithFluxChunkedSignaledImmediateError() throws IOException { + //NOTE - This demonstrates the current behavior of the error handler not getting invoked + //when writing a chunked response, even if the error is signaled before any data to be + //written to the response body. It would be ideal if in this case the error handler could + //still be invoked with the exception from the error signal. asserts(SPEC_NAME, HttpRequest.GET("/errors/flux-chunked-immediate-error"), (server, request) -> AssertionUtils.assertThrows(server, request, HttpResponseAssertion.builder() @@ -89,7 +93,7 @@ void testErrorHandlerWithFluxSingleResultSignaledError() throws IOException { asserts(SPEC_NAME, HttpRequest.GET("/errors/flux-single-error"), (server, request) -> AssertionUtils.assertThrows(server, request, HttpResponseAssertion.builder() - .status(HttpStatus.BAD_REQUEST) + .status(HttpStatus.I_AM_A_TEAPOT) .body("Your request is erroneous: Cannot process request.") .build())); } @@ -134,7 +138,7 @@ Flux fluxChunkedDelayedError() { @Error(global = true) public HttpResponse handleMyTestException(HttpRequest request, MyTestException exception) { var error = "Your request is erroneous: " + exception.getMessage(); - return HttpResponse.status(HttpStatus.BAD_REQUEST, "Bad request") + return HttpResponse.status(HttpStatus.I_AM_A_TEAPOT, "Bad request") .body(error); }