Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package life.mosu.mosuserver.global.exception;

import com.fasterxml.jackson.annotation.JsonInclude;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Getter;

@Getter
@Builder
@AllArgsConstructor
@JsonInclude(JsonInclude.Include.NON_NULL)
public class ErrorResponse {

private int status;
private String message;
private Object errors;
private String code;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package life.mosu.mosuserver.global.exception;

import org.springframework.http.HttpStatus;

public class ErrorResponseFactory {

public static ErrorResponse of(HttpStatus status, String message, Object errors) {
return ErrorResponse.builder()
.status(status.value())
.message(message)
.errors(errors)
.build();
}

public static ErrorResponse of(HttpStatus status, String message, Object errors, String code) {
return ErrorResponse.builder()
.status(status.value())
.message(message)
.errors(errors)
.code(code)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import jakarta.persistence.EntityNotFoundException;
import java.util.LinkedHashMap;
import java.util.Map;
import life.mosu.mosuserver.infra.notify.NotifyClientAdapter;
import life.mosu.mosuserver.infra.notify.dto.discord.DiscordExceptionNotifyEventRequest;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand All @@ -15,145 +18,146 @@

@Slf4j
@RestControllerAdvice
@RequiredArgsConstructor
public class GlobalExceptionHandler {

/**
* DTO 유효성 검사 실패
*
* @return 400 Bad Request
*/
private final NotifyClientAdapter<DiscordExceptionNotifyEventRequest> notifier;

private void notifyIfNeeded(Exception ex) {
try {
DiscordExceptionNotifyEventRequest request = DiscordExceptionNotifyEventRequest.of(
ex.getCause().toString(),
ex.getMessage()
Comment on lines +29 to +30

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

ex.getCause() can be null for many exceptions, which will lead to a NullPointerException when toString() is called on it. This will be caught by the outer try-catch block, but it will prevent the notification from being sent correctly and will log an unnecessary error. You should handle the null case gracefully. For instance, you could use the exception's class name if the cause is not present.

Suggested change
ex.getCause().toString(),
ex.getMessage()
ex.getCause() != null ? ex.getCause().toString() : ex.getClass().getSimpleName(),
ex.getMessage()

);
notifier.send(request);
} catch (Exception notifyEx) {
log.error("[Discord Notify Error]", notifyEx);
}
}

@ExceptionHandler(MethodArgumentNotValidException.class)
public ResponseEntity<Map<String, Object>> handleMethodArgumentNotValidException(
public ResponseEntity<ErrorResponse> handleMethodArgumentNotValidException(
MethodArgumentNotValidException ex) {
Map<String, Object> response = new LinkedHashMap<>();
Map<String, String> errors = new LinkedHashMap<>();

errors.put("message", "입력값이 올바르지 않습니다.");
notifyIfNeeded(ex);

Map<String, String> errors = new LinkedHashMap<>();
ex.getBindingResult().getFieldErrors().forEach(error -> {
errors.put(error.getField(), error.getDefaultMessage());
});
response.put("status", HttpStatus.BAD_REQUEST.value());
response.put("message", "유효성 검사에 실패했습니다.");
response.put("errors", errors);

ErrorResponse response = ErrorResponse.builder()
.status(HttpStatus.BAD_REQUEST.value())
.message("유효성 검사에 실패했습니다.")
.errors(errors)
.build();

return ResponseEntity.badRequest().body(response);
}

/**
* 잘못된 파라미터 요청
*
* @return 400 Bad Request
*/
@ExceptionHandler(IllegalArgumentException.class)
public ResponseEntity<Map<String, Object>> handleIllegalArgumentException(
public ResponseEntity<ErrorResponse> handleIllegalArgumentException(
IllegalArgumentException ex) {
Map<String, Object> response = new LinkedHashMap<>();
response.put("status", HttpStatus.BAD_REQUEST.value());
response.put("errors", "잘못된 요청입니다.");
response.put("message", ex.getMessage());
notifyIfNeeded(ex);

ErrorResponse response = ErrorResponse.builder()
.status(HttpStatus.BAD_REQUEST.value())
.message(ex.getMessage())
.errors("잘못된 요청입니다.")
.build();
Comment on lines +62 to +66

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

You've introduced a convenient ErrorResponseFactory in this pull request, but it's not being used here. To improve code consistency and conciseness, you should use the factory to create ErrorResponse instances. This can be applied to all other exception handlers in this class as well.

Suggested change
ErrorResponse response = ErrorResponse.builder()
.status(HttpStatus.BAD_REQUEST.value())
.message(ex.getMessage())
.errors("잘못된 요청입니다.")
.build();
ErrorResponse response = ErrorResponseFactory.of(
HttpStatus.BAD_REQUEST,
ex.getMessage(),
"잘못된 요청입니다."
);


return ResponseEntity.badRequest().body(response);
}

/**
* 엔티티가 존재하지 않을 때
*
* @return 404 Not Found
*/
@ExceptionHandler(EntityNotFoundException.class)
public ResponseEntity<Map<String, Object>> handleEntityNotFoundException(
EntityNotFoundException ex) {
Map<String, Object> response = new LinkedHashMap<>();
response.put("status", HttpStatus.NOT_FOUND.value());
response.put("message", "요청한 리소스가 존재하지 않습니다.");
response.put("errors", ex.getMessage());
public ResponseEntity<ErrorResponse> handleEntityNotFoundException(EntityNotFoundException ex) {
notifyIfNeeded(ex);

ErrorResponse response = ErrorResponse.builder()
.status(HttpStatus.NOT_FOUND.value())
.message("요청한 리소스가 존재하지 않습니다.")
.errors(ex.getMessage())
.build();

return ResponseEntity.status(HttpStatus.NOT_FOUND).body(response);
}

/**
* 인증 예외 처리
*
* @return 401 Unauthorized
*/
@ExceptionHandler(AuthenticationException.class)
public ResponseEntity<Map<String, Object>> handleAuthenticationException(
AuthenticationException ex) {
Map<String, Object> response = new LinkedHashMap<>();
response.put("status", HttpStatus.UNAUTHORIZED.value());
response.put("message", "인증에 실패했습니다");
response.put("errors", ex.getMessage());
public ResponseEntity<ErrorResponse> handleAuthenticationException(AuthenticationException ex) {
notifyIfNeeded(ex);

ErrorResponse response = ErrorResponse.builder()
.status(HttpStatus.UNAUTHORIZED.value())
.message("인증에 실패했습니다")
.errors(ex.getMessage())
.build();

return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(response);
}

/**
* 커스텀 AuthenticationException 처리
*
* @return 401 Unauthorized
*/
@ExceptionHandler(life.mosu.mosuserver.global.exception.AuthenticationException.class)
public ResponseEntity<Map<String, Object>> handleAuthenticationException(
public ResponseEntity<ErrorResponse> handleCustomAuthenticationException(
life.mosu.mosuserver.global.exception.AuthenticationException ex) {
Map<String, Object> response = new LinkedHashMap<>();
response.put("status", HttpStatus.UNAUTHORIZED.value());
response.put("message", "인증에 실패했습니다");
response.put("errors", ex.getMessage());
notifyIfNeeded(ex);

ErrorResponse response = ErrorResponse.builder()
.status(HttpStatus.UNAUTHORIZED.value())
.message("인증에 실패했습니다")
.errors(ex.getMessage())
.build();

return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(response);
}

/**
* 권한이 없는 경우 예외 처리
*
* @return 403 Forbidden
*/
@ExceptionHandler(AccessDeniedException.class)
public ResponseEntity<Map<String, Object>> handleAccessDeniedException(
AccessDeniedException ex) {
Map<String, Object> response = new LinkedHashMap<>();
response.put("status", HttpStatus.FORBIDDEN.value());
response.put("message", "인가를 실패 했습니다");
response.put("errors", ex.getMessage());
public ResponseEntity<ErrorResponse> handleAccessDeniedException(AccessDeniedException ex) {
notifyIfNeeded(ex);

ErrorResponse response = ErrorResponse.builder()
.status(HttpStatus.FORBIDDEN.value())
.message("인가를 실패 했습니다")
.errors(ex.getMessage())
.build();

return ResponseEntity.status(HttpStatus.FORBIDDEN).body(response);
}

/**
* @return 409 Bad Request
* @RequestBody JSON 파싱 실패 (필드명 불일치, 데이터 타입 불일치, JSON 형식 오류 등)
*/
@ExceptionHandler(HttpMessageNotReadableException.class)
public ResponseEntity<Map<String, Object>> handleHttpMessageNotReadableException(
public ResponseEntity<ErrorResponse> handleHttpMessageNotReadableException(
HttpMessageNotReadableException ex) {
Map<String, Object> response = new LinkedHashMap<>();
response.put("status", HttpStatus.CONFLICT.value());
response.put("message", "필드명 또는 데이터 타입이 일치하지 않습니다.");
response.put("errors", ex.getMessage());
notifyIfNeeded(ex);

ErrorResponse response = ErrorResponse.builder()
.status(HttpStatus.CONFLICT.value())
.message("필드명 또는 데이터 타입이 일치하지 않습니다.")
.errors(ex.getMessage())
.build();

return ResponseEntity.status(HttpStatus.CONFLICT).body(response);
}

@ExceptionHandler(Exception.class)
public ResponseEntity<Map<String, Object>> handleGeneralException(Exception ex) {
System.out.println("Exception: " + ex.getMessage());
Map<String, Object> response = new LinkedHashMap<>();
response.put("status", HttpStatus.INTERNAL_SERVER_ERROR.value());
response.put("message", "서버 오류가 발생했습니다.");
response.put("errors", ErrorCode.SERVER_ERROR);
public ResponseEntity<ErrorResponse> handleGeneralException(Exception ex) {
notifyIfNeeded(ex);

ErrorResponse response = ErrorResponse.builder()
.status(HttpStatus.INTERNAL_SERVER_ERROR.value())
.message("서버 오류가 발생했습니다.")
.errors(ErrorCode.SERVER_ERROR)
.build();

return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(response);
}

@ExceptionHandler(CustomRuntimeException.class)
public ResponseEntity<Map<String, Object>> handleCustomRuntimeException(
CustomRuntimeException ex) {
Map<String, Object> response = new LinkedHashMap<>();
response.put("status", ex.getStatus().value());
response.put("message", ex.getMessage());
response.put("code", ex.getCode());
public ResponseEntity<ErrorResponse> handleCustomRuntimeException(CustomRuntimeException ex) {
notifyIfNeeded(ex);

ErrorResponse response = ErrorResponse.builder()
.status(ex.getStatus().value())
.message(ex.getMessage())
.code(ex.getCode())
.build();

return ResponseEntity.status(ex.getStatus()).body(response);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,17 @@ public static DiscordExceptionNotifyEventRequest of(
return new DiscordExceptionNotifyEventRequest(exceptionCause, exceptionMessage, meta);
}

public static DiscordExceptionNotifyEventRequest of(
String exceptionCause,
String exceptionMessage
) {
return new DiscordExceptionNotifyEventRequest(exceptionCause, exceptionMessage, null);
}

public String getMessage() {
return "❌ **알림 전송 실패**\n"
+ String.format("- ⚠️ exception Cause : `%s`\n", exceptionCause)
+ String.format("- 📨 exception Message: `%s`\n", exceptionMessage)
+ String.format("- 📋 meta: `%s`", meta);
+ String.format("- 📋 meta: `%s`", meta == null ? "없음" : meta);
Comment on lines 25 to +28

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Discord notification message has a couple of areas for improvement:

  1. The title "❌ 알림 전송 실패" (Notification sending failed) is misleading. These notifications are for general server errors, not just for failures in the notification system itself. A more appropriate title would be "❌ 서버 에러 발생" (Server error occurred).
  2. When exceptionCause or exceptionMessage are null, they are rendered as the string "null" in the message. It would be more readable to display a placeholder like "없음" (None), similar to how meta is handled.
Suggested change
return "❌ **알림 전송 실패**\n"
+ String.format("- ⚠️ exception Cause : `%s`\n", exceptionCause)
+ String.format("- 📨 exception Message: `%s`\n", exceptionMessage)
+ String.format("- 📋 meta: `%s`", meta);
+ String.format("- 📋 meta: `%s`", meta == null ? "없음" : meta);
return "❌ **서버 에러 발생**\n"
+ String.format("- ⚠️ exception Cause : `%s`\n", exceptionCause == null ? "없음" : exceptionCause)
+ String.format("- 📨 exception Message: `%s`\n", exceptionMessage == null ? "없음" : exceptionMessage)
+ String.format("- 📋 meta: `%s`", meta == null ? "없음" : meta);

}
}