Skip to content

Suggest improvements for GlobalControllerExceptionHandler #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Suggest improvements for GlobalControllerExceptionHandler #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
214 changes: 170 additions & 44 deletions src/main/java/com/fleetops/controller/GlobalControllerExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,95 +8,221 @@
* <li>Constraint/validation violations -> 400 Bad Request</li>
* <li>Not found exceptions -> 404 Not Found</li>
* <li>Conflicts such as duplicate resources -> 409 Conflict</li>
* <li>Service layer exceptions -> 500 Internal Server Error</li>
* <li>Unhandled errors -> 500 Internal Server Error</li>
* </ul>
* The corresponding response bodies follow a simple error format (see ErrorResponse in OpenAPI).
*/

import com.fleetops.exception.LicensePlateAlreadyExistsException;
import com.fleetops.exception.NotFoundExceptionBase;
import com.fleetops.exception.ServiceException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.validation.ConstraintViolationException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.HttpMessageNotReadableException;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.AuthenticationException;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.context.request.WebRequest;

import java.time.LocalDateTime;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

@RestControllerAdvice
public class GlobalControllerExceptionHandler {

@ExceptionHandler(RuntimeException.class)
public ResponseEntity<Map<String, Object>> handleRuntimeException(RuntimeException ex) {
private static final Logger logger = LoggerFactory.getLogger(GlobalControllerExceptionHandler.class);

/**
* Creates a standardized error response body.
*
* @param status HTTP status
* @param error Error type description
* @param message Error message
* @param request Web request for additional context
* @return Map containing error details
*/
private Map<String, Object> createErrorResponse(HttpStatus status, String error, String message, WebRequest request) {
Map<String, Object> body = new HashMap<>();
body.put("timestamp", LocalDateTime.now());
body.put("status", HttpStatus.INTERNAL_SERVER_ERROR.value());
body.put("error", "Internal Server Error");
body.put("message", ex.getMessage());
body.put("status", status.value());
body.put("error", error);
body.put("message", message);
body.put("path", request.getDescription(false).replace("uri=", ""));
return body;
}

/**
* Creates a standardized error response body with additional details.
*
* @param status HTTP status
* @param error Error type description
* @param message Error message
* @param request Web request for additional context
* @param details Additional error details
* @return Map containing error details
*/
private Map<String, Object> createErrorResponse(HttpStatus status, String error, String message, WebRequest request, Map<String, Object> details) {
Map<String, Object> body = createErrorResponse(status, error, message, request);
if (details != null && !details.isEmpty()) {
body.putAll(details);
}
return body;
}

@ExceptionHandler(ServiceException.class)
public ResponseEntity<Map<String, Object>> handleServiceException(ServiceException ex, WebRequest request) {
logger.error("Service exception occurred: {}", ex.getMessage(), ex);
Map<String, Object> body = createErrorResponse(
HttpStatus.INTERNAL_SERVER_ERROR,
"Internal Server Error",
"A service error occurred. Please try again later.",
request
);
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(body);
}

@ExceptionHandler(NotFoundExceptionBase.class)
public ResponseEntity<Map<String, Object>> handleNotFound(NotFoundExceptionBase ex) {
Map<String, Object> body = new HashMap<>();
body.put("timestamp", LocalDateTime.now());
body.put("status", HttpStatus.NOT_FOUND.value());
body.put("error", "Not Found");
body.put("message", ex.getMessage());
@ExceptionHandler(RuntimeException.class)
public ResponseEntity<Map<String, Object>> handleRuntimeException(RuntimeException ex, WebRequest request) {
logger.error("Unexpected runtime exception occurred: {}", ex.getMessage(), ex);
Map<String, Object> body = createErrorResponse(
HttpStatus.INTERNAL_SERVER_ERROR,
"Internal Server Error",
"An unexpected error occurred. Please try again later.",
request
);
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(body);
}

@ExceptionHandler(NotFoundExceptionBase.class)
public ResponseEntity<Map<String, Object>> handleNotFound(NotFoundExceptionBase ex, WebRequest request) {
logger.debug("Resource not found: {}", ex.getMessage());
Map<String, Object> body = createErrorResponse(
HttpStatus.NOT_FOUND,
"Not Found",
ex.getMessage(),
request
);
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(body);
}

@ExceptionHandler(LicensePlateAlreadyExistsException.class)
public ResponseEntity<Map<String, Object>> handleConflict(LicensePlateAlreadyExistsException ex) {
Map<String, Object> body = new HashMap<>();
body.put("timestamp", LocalDateTime.now());
body.put("status", HttpStatus.CONFLICT.value());
body.put("error", "Conflict");
body.put("message", ex.getMessage());
public ResponseEntity<Map<String, Object>> handleLicensePlateConflict(LicensePlateAlreadyExistsException ex, WebRequest request) {
logger.warn("License plate conflict: {}", ex.getMessage());
Map<String, Object> body = createErrorResponse(
HttpStatus.CONFLICT,
"Conflict",
ex.getMessage(),
request
);
return ResponseEntity.status(HttpStatus.CONFLICT).body(body);
}

@ExceptionHandler(DataIntegrityViolationException.class)
public ResponseEntity<Map<String, Object>> handleDataIntegrityViolation(DataIntegrityViolationException ex, WebRequest request) {
logger.warn("Data integrity violation: {}", ex.getMessage());
Map<String, Object> body = createErrorResponse(
HttpStatus.CONFLICT,
"Conflict",
"The operation conflicts with existing data constraints.",
request
);
return ResponseEntity.status(HttpStatus.CONFLICT).body(body);
}

@ExceptionHandler(MethodArgumentNotValidException.class)
public ResponseEntity<Map<String, Object>> handleValidation(MethodArgumentNotValidException ex) {
Map<String, Object> body = new HashMap<>();
body.put("timestamp", LocalDateTime.now());
body.put("status", HttpStatus.BAD_REQUEST.value());
body.put("error", "Bad Request");
body.put("message", "Validation failed");
public ResponseEntity<Map<String, Object>> handleValidation(MethodArgumentNotValidException ex, WebRequest request) {
logger.debug("Validation failed: {}", ex.getMessage());

Map<String, String> fieldErrors = ex.getBindingResult()
.getFieldErrors()
.stream()
.collect(Collectors.toMap(
fieldError -> fieldError.getField(),
fieldError -> fieldError.getDefaultMessage() != null ? fieldError.getDefaultMessage() : "Invalid value",
(existing, replacement) -> existing
));

Map<String, Object> details = new HashMap<>();
details.put("fieldErrors", fieldErrors);

Map<String, Object> body = createErrorResponse(
HttpStatus.BAD_REQUEST,
"Validation Failed",
"Request validation failed for one or more fields.",
request,
details
);
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(body);
}

@ExceptionHandler(ConstraintViolationException.class)
public ResponseEntity<Map<String, Object>> handleConstraintViolation(ConstraintViolationException ex) {
Map<String, Object> body = new HashMap<>();
body.put("timestamp", LocalDateTime.now());
body.put("status", HttpStatus.BAD_REQUEST.value());
body.put("error", "Bad Request");
body.put("message", "Constraint violation");
body.put("violations", ex.getConstraintViolations().stream()
.collect(Collectors.toMap(
v -> v.getPropertyPath().toString(),
v -> v.getMessage(),
(a, b) -> a
)));
return ResponseEntity.badRequest().body(body);
public ResponseEntity<Map<String, Object>> handleConstraintViolation(ConstraintViolationException ex, WebRequest request) {
logger.debug("Constraint violation: {}", ex.getMessage());

Map<String, String> violations = ex.getConstraintViolations().stream()
.collect(Collectors.toMap(
v -> v.getPropertyPath().toString(),
v -> v.getMessage(),
(existing, replacement) -> existing
));

Map<String, Object> details = new HashMap<>();
details.put("violations", violations);

Map<String, Object> body = createErrorResponse(
HttpStatus.BAD_REQUEST,
"Constraint Violation",
"Request contains constraint violations.",
request,
details
);
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(body);
}

@ExceptionHandler(HttpMessageNotReadableException.class)
public ResponseEntity<Map<String, Object>> handleBadJson(HttpMessageNotReadableException ex) {
Map<String, Object> body = new HashMap<>();
body.put("timestamp", LocalDateTime.now());
body.put("status", HttpStatus.BAD_REQUEST.value());
body.put("error", "Bad Request");
body.put("message", "Malformed JSON request");
return ResponseEntity.badRequest().body(body);
public ResponseEntity<Map<String, Object>> handleBadJson(HttpMessageNotReadableException ex, WebRequest request) {
logger.debug("Malformed JSON request: {}", ex.getMessage());
Map<String, Object> body = createErrorResponse(
HttpStatus.BAD_REQUEST,
"Bad Request",
"Malformed JSON request. Please check your request format.",
request
);
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(body);
}

@ExceptionHandler(AuthenticationException.class)
public ResponseEntity<Map<String, Object>> handleAuthenticationException(AuthenticationException ex, WebRequest request) {
logger.warn("Authentication failed: {}", ex.getMessage());
Map<String, Object> body = createErrorResponse(
HttpStatus.UNAUTHORIZED,
"Unauthorized",
"Authentication is required to access this resource.",
request
);
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(body);
}

@ExceptionHandler(AccessDeniedException.class)
public ResponseEntity<Map<String, Object>> handleAccessDeniedException(AccessDeniedException ex, WebRequest request) {
logger.warn("Access denied: {}", ex.getMessage());
Map<String, Object> body = createErrorResponse(
HttpStatus.FORBIDDEN,
"Forbidden",
"You do not have permission to access this resource.",
request
);
return ResponseEntity.status(HttpStatus.FORBIDDEN).body(body);
}
}
54 changes: 48 additions & 6 deletions src/test/java/com/fleetops/controller/GlobalControllerExceptionHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

import com.fleetops.exception.LicensePlateAlreadyExistsException;
import com.fleetops.exception.NotFoundExceptionBase;
import com.fleetops.exception.ServiceException;
import jakarta.validation.Valid;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
Expand Down Expand Up @@ -35,23 +37,29 @@ void notFound_IsMappedTo404Json() throws Exception {
.andExpect(status().isNotFound())
.andExpect(jsonPath("$.status").value(404))
.andExpect(jsonPath("$.error").value("Not Found"))
.andExpect(jsonPath("$.message", containsString("missing")));
.andExpect(jsonPath("$.message", containsString("missing")))
.andExpect(jsonPath("$.timestamp").exists())
.andExpect(jsonPath("$.path").exists());
}

@Test
void conflict_IsMappedTo409Json() throws Exception {
mockMvc.perform(get("/throw/conflict"))
.andExpect(status().isConflict())
.andExpect(jsonPath("$.status").value(409))
.andExpect(jsonPath("$.error").value("Conflict"));
.andExpect(jsonPath("$.error").value("Conflict"))
.andExpect(jsonPath("$.timestamp").exists())
.andExpect(jsonPath("$.path").exists());
}

@Test
void runtime_IsMappedTo500Json() throws Exception {
mockMvc.perform(get("/throw/runtime"))
.andExpect(status().isInternalServerError())
.andExpect(jsonPath("$.status").value(500))
.andExpect(jsonPath("$.error").value("Internal Server Error"));
.andExpect(jsonPath("$.error").value("Internal Server Error"))
.andExpect(jsonPath("$.timestamp").exists())
.andExpect(jsonPath("$.path").exists());
}

@Test
Expand All @@ -63,15 +71,21 @@ void methodArgumentNotValid_IsMappedTo400Json() throws Exception {
.content(json))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.status").value(400))
.andExpect(jsonPath("$.error").value("Bad Request"));
.andExpect(jsonPath("$.error").value("Validation Failed"))
.andExpect(jsonPath("$.fieldErrors").exists())
.andExpect(jsonPath("$.timestamp").exists())
.andExpect(jsonPath("$.path").exists());
}

@Test
void constraintViolation_IsMappedTo400Json() throws Exception {
mockMvc.perform(get("/throw/constraint/-1"))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.status").value(400))
.andExpect(jsonPath("$.error").value("Bad Request"));
.andExpect(jsonPath("$.error").value("Constraint Violation"))
.andExpect(jsonPath("$.violations").exists())
.andExpect(jsonPath("$.timestamp").exists())
.andExpect(jsonPath("$.path").exists());
}

@Test
Expand All @@ -82,7 +96,29 @@ void malformedJson_IsMappedTo400Json() throws Exception {
.content(badJson))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.status").value(400))
.andExpect(jsonPath("$.error").value("Bad Request"));
.andExpect(jsonPath("$.error").value("Bad Request"))
.andExpect(jsonPath("$.timestamp").exists())
.andExpect(jsonPath("$.path").exists());
}

@Test
void serviceException_IsMappedTo500Json() throws Exception {
mockMvc.perform(get("/throw/service"))
.andExpect(status().isInternalServerError())
.andExpect(jsonPath("$.status").value(500))
.andExpect(jsonPath("$.error").value("Internal Server Error"))
.andExpect(jsonPath("$.timestamp").exists())
.andExpect(jsonPath("$.path").exists());
}

@Test
void dataIntegrityViolation_IsMappedTo409Json() throws Exception {
mockMvc.perform(get("/throw/dataintegrity"))
.andExpect(status().isConflict())
.andExpect(jsonPath("$.status").value(409))
.andExpect(jsonPath("$.error").value("Conflict"))
.andExpect(jsonPath("$.timestamp").exists())
.andExpect(jsonPath("$.path").exists());
}

// Minimal controller solely for triggering exceptions handled by the advice
Expand All @@ -105,6 +141,12 @@ private static class ThrowingController {
public String constraint(@PathVariable long id) {
throw new jakarta.validation.ConstraintViolationException("invalid", java.util.Collections.emptySet());
}

@GetMapping("/throw/service")
public String service() { throw new ServiceException("Service failed", new RuntimeException("Database error")); }

@GetMapping("/throw/dataintegrity")
public String dataIntegrity() { throw new DataIntegrityViolationException("Data constraint violated"); }
}

private static class SampleDto {
Expand Down