From b94b036378f955cec87bedc6c8045f61777f09c7 Mon Sep 17 00:00:00 2001 From: Andre Praca Date: Wed, 31 Jan 2024 14:43:27 +0200 Subject: [PATCH 1/3] Pick status code from `ErrorResponse` interfaces unhandled exceptions that extend spring-boot's `ErrorResponse` have a status code defined and we should use that instead of forcing it to be `500` --- .../org/zalando/problem/spring/common/AdviceTrait.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/problem-spring-common/src/main/java/org/zalando/problem/spring/common/AdviceTrait.java b/problem-spring-common/src/main/java/org/zalando/problem/spring/common/AdviceTrait.java index 2ca57aea..cbeedf8c 100644 --- a/problem-spring-common/src/main/java/org/zalando/problem/spring/common/AdviceTrait.java +++ b/problem-spring-common/src/main/java/org/zalando/problem/spring/common/AdviceTrait.java @@ -1,7 +1,9 @@ package org.zalando.problem.spring.common; import org.apiguardian.api.API; +import org.springframework.http.HttpStatusCode; import org.springframework.http.ResponseEntity; +import org.springframework.web.ErrorResponse; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.ResponseStatus; @@ -41,11 +43,19 @@ public interface AdviceTrait { default ThrowableProblem toProblem(final Throwable throwable) { final StatusType status = Optional.ofNullable(resolveResponseStatus(throwable)) .map(ResponseStatusAdapter::new) + .or(() -> Optional.ofNullable(resolveStatusFromErrorResponse(throwable))) .orElse(Status.INTERNAL_SERVER_ERROR); return toProblem(throwable, status); } + default StatusType resolveStatusFromErrorResponse(final Throwable type) { + if (!(type instanceof ErrorResponse)) return null; + + final HttpStatusCode code = ((ErrorResponse) type).getStatusCode(); + return Status.valueOf(code.value()); + } + @API(status = MAINTAINED) default ResponseStatus resolveResponseStatus(final Throwable type) { @Nullable final ResponseStatus candidate = findMergedAnnotation(type.getClass(), ResponseStatus.class); From f3df68048c67776529a98c81f417011feac959f0 Mon Sep 17 00:00:00 2001 From: Andre Praca Date: Wed, 31 Jan 2024 15:07:31 +0200 Subject: [PATCH 2/3] Add tests --- .../spring/common/AdviceTraitTest.java | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/problem-spring-common/src/test/java/org/zalando/problem/spring/common/AdviceTraitTest.java b/problem-spring-common/src/test/java/org/zalando/problem/spring/common/AdviceTraitTest.java index 7329371d..4281061a 100644 --- a/problem-spring-common/src/test/java/org/zalando/problem/spring/common/AdviceTraitTest.java +++ b/problem-spring-common/src/test/java/org/zalando/problem/spring/common/AdviceTraitTest.java @@ -2,7 +2,10 @@ import org.junit.jupiter.api.Test; import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.ProblemDetail; import org.springframework.http.ResponseEntity; +import org.springframework.web.ErrorResponse; import org.springframework.web.bind.annotation.ResponseStatus; import org.zalando.problem.Problem; import org.zalando.problem.Status; @@ -29,6 +32,24 @@ public TestException(String message) { } } + class SpringExpception extends Throwable implements ErrorResponse { + final HttpStatus status; + + SpringExpception(HttpStatus status) { + this.status = status; + } + + @Override + public HttpStatusCode getStatusCode() { + return status; + } + + @Override + public ProblemDetail getBody() { + return ProblemDetail.forStatusAndDetail(status, status.getReasonPhrase()); + } + } + @ResponseStatus(value = HttpStatus.RESET_CONTENT, reason = "reason") class TestExceptionWithReason extends RuntimeException { public TestExceptionWithReason(String message) { @@ -56,6 +77,16 @@ void buildsOnResponseStatusThrowable() { assertThat(result.getDetail(), is("Message")); } + @Test + void buildsOnErrorResponseThrowable() { + final Problem result = unit.toProblem(new SpringExpception(HttpStatus.NOT_FOUND)); + + assertThat(result.getStatus().getStatusCode(), is(404)); + assertThat(result.getType().toString(), is("about:blank")); + assertThat(result.getTitle(), is("Not Found")); + assertThat(result.getDetail(), is("Message")); + } + @Test void buildsOnResponseStatusWithReasonThrowable() { final Problem result = unit.toProblem(new TestExceptionWithReason("Message")); @@ -181,4 +212,4 @@ void processDoesNothing() { assertThat(result.getStatusCode(), is(HttpStatus.OK)); assertThat(result.getBody().getStatus(), is(Status.RESET_CONTENT)); } -} \ No newline at end of file +} From 194186005c9da96d53884592ae2b8c9e43c41453 Mon Sep 17 00:00:00 2001 From: Andre Praca Date: Wed, 31 Jan 2024 15:16:23 +0200 Subject: [PATCH 3/3] Fix test --- .../java/org/zalando/problem/spring/common/AdviceTraitTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/problem-spring-common/src/test/java/org/zalando/problem/spring/common/AdviceTraitTest.java b/problem-spring-common/src/test/java/org/zalando/problem/spring/common/AdviceTraitTest.java index 4281061a..92563585 100644 --- a/problem-spring-common/src/test/java/org/zalando/problem/spring/common/AdviceTraitTest.java +++ b/problem-spring-common/src/test/java/org/zalando/problem/spring/common/AdviceTraitTest.java @@ -84,7 +84,6 @@ void buildsOnErrorResponseThrowable() { assertThat(result.getStatus().getStatusCode(), is(404)); assertThat(result.getType().toString(), is("about:blank")); assertThat(result.getTitle(), is("Not Found")); - assertThat(result.getDetail(), is("Message")); } @Test