diff --git a/src/main/java/com/retailsvc/http/BadRequestException.java b/src/main/java/com/retailsvc/http/BadRequestException.java index 4abaa69..043ad14 100644 --- a/src/main/java/com/retailsvc/http/BadRequestException.java +++ b/src/main/java/com/retailsvc/http/BadRequestException.java @@ -21,15 +21,28 @@ public final class BadRequestException extends RuntimeException { private final String keyword; public BadRequestException(String detail) { - this(DEFAULT_STATUS, detail, null, null); + this(DEFAULT_STATUS, detail, null, null, null); + } + + public BadRequestException(String detail, Throwable cause) { + this(DEFAULT_STATUS, detail, null, null, cause); } public BadRequestException(int status, String detail) { - this(status, detail, null, null); + this(status, detail, null, null, null); + } + + public BadRequestException(int status, String detail, Throwable cause) { + this(status, detail, null, null, cause); } public BadRequestException(int status, String detail, String pointer, String keyword) { - super(Objects.requireNonNull(detail, "detail must not be null")); + this(status, detail, pointer, keyword, null); + } + + public BadRequestException( + int status, String detail, String pointer, String keyword, Throwable cause) { + super(Objects.requireNonNull(detail, "detail must not be null"), cause); if (status < 400 || status > 499) { throw new IllegalArgumentException("status must be 4xx, got " + status); } diff --git a/src/main/java/com/retailsvc/http/Handlers.java b/src/main/java/com/retailsvc/http/Handlers.java index 59a3c7d..53f8409 100644 --- a/src/main/java/com/retailsvc/http/Handlers.java +++ b/src/main/java/com/retailsvc/http/Handlers.java @@ -64,12 +64,21 @@ public static ExceptionHandler defaultExceptionHandler() { HTTP_BAD_REQUEST, ProblemDetailRenderer.renderJson(ProblemDetail.forValidation(ve.error())), "application/problem+json"); - case BadRequestException bre -> - Response.bytes( - bre.status(), - ProblemDetailRenderer.renderJson(ProblemDetail.forBadRequest(bre)), - "application/problem+json"); - case NotFoundException _ -> Response.notFound(); + case BadRequestException bre -> { + if (bre.getCause() != null && LOG.isDebugEnabled()) { + LOG.debug("BadRequestException cause", bre.getCause()); + } + yield Response.bytes( + bre.status(), + ProblemDetailRenderer.renderJson(ProblemDetail.forBadRequest(bre)), + "application/problem+json"); + } + case NotFoundException nfe -> { + if (nfe.getCause() != null && LOG.isDebugEnabled()) { + LOG.debug("NotFoundException cause", nfe.getCause()); + } + yield Response.notFound(); + } case MethodNotAllowedException mna -> Response.status(HTTP_BAD_METHOD) .withHeader( diff --git a/src/main/java/com/retailsvc/http/NotFoundException.java b/src/main/java/com/retailsvc/http/NotFoundException.java index ea3f82e..d653167 100644 --- a/src/main/java/com/retailsvc/http/NotFoundException.java +++ b/src/main/java/com/retailsvc/http/NotFoundException.java @@ -4,4 +4,8 @@ public final class NotFoundException extends RuntimeException { public NotFoundException(String message) { super(message); } + + public NotFoundException(String message, Throwable cause) { + super(message, cause); + } } diff --git a/src/test/java/com/retailsvc/http/BadRequestExceptionTest.java b/src/test/java/com/retailsvc/http/BadRequestExceptionTest.java index 395d474..eae87c1 100644 --- a/src/test/java/com/retailsvc/http/BadRequestExceptionTest.java +++ b/src/test/java/com/retailsvc/http/BadRequestExceptionTest.java @@ -43,6 +43,23 @@ void rejectsNon4xxStatus() { .hasMessageContaining("4xx"); } + @Test + void preservesCause() { + Throwable cause = new IllegalStateException("root"); + + BadRequestException defaultStatus = new BadRequestException("bad", cause); + BadRequestException withStatus = new BadRequestException(422, "bad", cause); + BadRequestException full = new BadRequestException(422, "bad", "/x", "unique", cause); + + assertThat(defaultStatus).hasCause(cause); + assertThat(defaultStatus.status()).isEqualTo(400); + assertThat(withStatus).hasCause(cause); + assertThat(withStatus.status()).isEqualTo(422); + assertThat(full).hasCause(cause); + assertThat(full.pointer()).contains("/x"); + assertThat(full.keyword()).contains("unique"); + } + @Test void rejectsNullDetail() { assertThatThrownBy(() -> new BadRequestException(null)) diff --git a/src/test/java/com/retailsvc/http/HandlersDefaultExceptionTest.java b/src/test/java/com/retailsvc/http/HandlersDefaultExceptionTest.java index 36d0fdd..93c9136 100644 --- a/src/test/java/com/retailsvc/http/HandlersDefaultExceptionTest.java +++ b/src/test/java/com/retailsvc/http/HandlersDefaultExceptionTest.java @@ -2,17 +2,44 @@ import static org.assertj.core.api.Assertions.assertThat; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import com.retailsvc.http.spec.HttpMethod; import com.retailsvc.http.validate.ValidationError; import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.Set; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; class HandlersDefaultExceptionTest { private static final TypeMapper JSON = new GsonTypeMapper(); + private Logger handlersLogger; + private Level originalLevel; + private ListAppender appender; + + @BeforeEach + void attachAppender() { + handlersLogger = (Logger) LoggerFactory.getLogger(Handlers.class); + originalLevel = handlersLogger.getLevel(); + handlersLogger.setLevel(Level.DEBUG); + appender = new ListAppender<>(); + appender.start(); + handlersLogger.addAppender(appender); + } + + @AfterEach + void detachAppender() { + handlersLogger.detachAppender(appender); + handlersLogger.setLevel(originalLevel); + } + @Test void validationExceptionRendersProblemJson() { Response resp = @@ -70,6 +97,50 @@ void methodNotAllowedReturns405WithAllowHeader() { assertThat(resp.headers().get("Allow")).contains("GET").contains("POST"); } + @Test + void badRequestCauseLoggedAtDebug() { + Throwable cause = new IllegalStateException("root"); + + Handlers.defaultExceptionHandler().handle(new BadRequestException("bad", cause)); + + assertThat(appender.list) + .anySatisfy( + event -> { + assertThat(event.getLevel()).isEqualTo(Level.DEBUG); + assertThat(event.getThrowableProxy().getClassName()) + .isEqualTo(IllegalStateException.class.getName()); + }); + } + + @Test + void badRequestWithoutCauseDoesNotLog() { + Handlers.defaultExceptionHandler().handle(new BadRequestException("bad")); + + assertThat(appender.list).isEmpty(); + } + + @Test + void notFoundCauseLoggedAtDebug() { + Throwable cause = new IllegalStateException("root"); + + Handlers.defaultExceptionHandler().handle(new NotFoundException("missing", cause)); + + assertThat(appender.list) + .anySatisfy( + event -> { + assertThat(event.getLevel()).isEqualTo(Level.DEBUG); + assertThat(event.getThrowableProxy().getClassName()) + .isEqualTo(IllegalStateException.class.getName()); + }); + } + + @Test + void notFoundWithoutCauseDoesNotLog() { + Handlers.defaultExceptionHandler().handle(new NotFoundException("missing")); + + assertThat(appender.list).isEmpty(); + } + @Test void unknownExceptionReturns500() { Response resp = Handlers.defaultExceptionHandler().handle(new RuntimeException("kaboom")); diff --git a/src/test/java/com/retailsvc/http/NotFoundExceptionTest.java b/src/test/java/com/retailsvc/http/NotFoundExceptionTest.java new file mode 100644 index 0000000..aa0a093 --- /dev/null +++ b/src/test/java/com/retailsvc/http/NotFoundExceptionTest.java @@ -0,0 +1,26 @@ +package com.retailsvc.http; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class NotFoundExceptionTest { + + @Test + void carriesMessage() { + NotFoundException e = new NotFoundException("missing"); + + assertThat(e.getMessage()).isEqualTo("missing"); + assertThat(e.getCause()).isNull(); + } + + @Test + void preservesCause() { + Throwable cause = new IllegalStateException("root"); + + NotFoundException e = new NotFoundException("missing", cause); + + assertThat(e.getMessage()).isEqualTo("missing"); + assertThat(e).hasCause(cause); + } +}