Skip to content

Commit

Permalink
fix(web): Support @ResponseStatus on parent classes/interfaces (#87)
Browse files Browse the repository at this point in the history
Previously the GenericExceptionHandler only looked for the @ResponseStatus
annotation on the exception class itself.

With this change set, the handler will look on the exception class
_as well as_ any implemented interfaces or super classes.

An HTTP 500 will be returned iff no @ResponseStatus annotation is found.
  • Loading branch information
ajordens authored Jul 15, 2017
1 parent 485f4f5 commit 206d2d6
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.autoconfigure.web.DefaultErrorAttributes;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.http.HttpStatus;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.web.bind.annotation.ControllerAdvice;
Expand Down Expand Up @@ -94,21 +95,23 @@ public void handleRetrofitError(RetrofitError e, HttpServletResponse response, H

@ExceptionHandler(Exception.class)
public void handleException(Exception e, HttpServletResponse response, HttpServletRequest request) throws IOException {
logger.error("Internal Server Error", e);

storeException(request, response, e);

ResponseStatus responseStatus = e.getClass().getAnnotation(ResponseStatus.class);
ResponseStatus responseStatus = AnnotationUtils.findAnnotation(e.getClass(), ResponseStatus.class);

if (responseStatus != null) {
HttpStatus httpStatus = responseStatus.value();
logger.error(httpStatus.getReasonPhrase(), e);

String message = e.getMessage();
if (message == null || message.trim().isEmpty()) {
message = responseStatus.reason();
}
response.sendError(responseStatus.value().value(), message);
response.sendError(httpStatus.value(), message);
} else {
logger.error("Internal Server Error", e);
response.sendError(HttpStatus.INTERNAL_SERVER_ERROR.value(), e.getMessage());
}

}

private void storeException(HttpServletRequest request, HttpServletResponse response, Exception ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.netflix.spinnaker.kork.web.exceptions

import groovy.transform.InheritConstructors
import org.springframework.http.HttpStatus
import org.springframework.web.bind.annotation.ResponseStatus
import spock.lang.Specification
Expand All @@ -39,23 +38,74 @@ class GenericExceptionHandlersSpec extends Specification {
genericExceptionHandlers.handleException(exception, response, request)
then:
1 * response.sendError(404, expectedReason)
1 * response.sendError(expectedStatusCode, expectedReason)
where:
exception || expectedReason
new MyException() || "Default Reason!"
new MyException("") || "Default Reason!"
new MyException(" ") || "Default Reason!"
new MyException("My Message") || "My Message"
exception || expectedStatusCode || expectedReason
new E1() || 404 || "Default Reason!"
new E1("") || 404 || "Default Reason!"
new E1(" ") || 404 || "Default Reason!"
new E1("E1 Reason") || 404 || "E1 Reason"
new E2() || 404 || "Default Reason!"
new E2("E2 Reason") || 404 || "E2 Reason"
new E3() || 404 || "Default Reason!"
new E4() || 400 || "My Other Reason!" // favor @ResponseStatus on interface over super class
new E5("E5 Reason") || 400 || "E5 Reason"
new NullPointerException("It's an NPE!") || 500 || "It's an NPE!"
}
@ResponseStatus(value = HttpStatus.NOT_FOUND, reason = "Default Reason!")
class MyException extends RuntimeException {
MyException() {
class E1 extends RuntimeException {
E1() {
super()
}
MyException(String message) {
E1(String message) {
super(message)
}
}
class E2 extends E1 {
E2() {
super()
}
E2(String message) {
super(message)
}
}
class E3 extends E2 {
E3() {
super()
}
E3(String message) {
super(message)
}
}
@ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "My Other Reason!")
interface I1 {
}
class E4 extends E3 implements I1 {
E4() {
super()
}
E4(String message) {
super(message)
}
}
class E5 extends E4 {
E5() {
super()
}
E5(String message) {
super(message)
}
}
Expand Down

0 comments on commit 206d2d6

Please sign in to comment.