-
Notifications
You must be signed in to change notification settings - Fork 0
[chore] 예외 핸들러 추가 #21
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.
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
Conversation
|
""" Walkthrough예외 처리를 위한 공통 컴포넌트가 신규로 도입되었습니다. 표준화된 응답 DTO, 에러 코드 및 비즈니스 예외 클래스, 그리고 글로벌 예외 핸들러가 추가되었으며, 테스트용 예외 컨트롤러도 구현되었습니다. Spring Security 관련 의존성은 비활성화되고, validation starter가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant GlobalExceptionHandler
participant ErrorResponse
Client->>Controller: API 요청 (예외 발생 상황)
Controller-->>GlobalExceptionHandler: 예외 throw
GlobalExceptionHandler->>ErrorResponse: ErrorResponse 생성
GlobalExceptionHandler-->>Client: 표준화된 에러 응답 반환
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Docstrings generation was requested by @buzz0331. * #21 (comment) The following files were modified: * `src/main/java/konkuk/thip/common/dto/BaseResponse.java` * `src/main/java/konkuk/thip/common/dto/ErrorResponse.java` * `src/main/java/konkuk/thip/common/dto/ResponseCode.java` * `src/main/java/konkuk/thip/common/dto/SuccessCode.java` * `src/main/java/konkuk/thip/common/exception/BusinessException.java` * `src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java` * `src/main/java/konkuk/thip/common/exception/InvalidStateException.java` * `src/main/java/konkuk/thip/common/exception/code/ErrorCode.java` * `src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java` * `src/main/java/konkuk/thip/test/TestExceptionController.java`
|
Note Generated docstrings for this pull request at #22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (4)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
5-9: 간결하고 명확한 예외 클래스 구현EntityNotFoundException이 잘 구현되었습니다. BusinessException을 상속하고 ErrorCode를 받는 구조가 적절합니다.
선택적으로 원인 예외를 받는 생성자를 추가하는 것을 고려해보세요:
+ public EntityNotFoundException(ErrorCode errorCode, Throwable cause) { + super(errorCode, cause); + }src/main/java/konkuk/thip/common/exception/InvalidStateException.java (1)
6-6: @Getter 어노테이션 사용 검토현재 클래스에는 별도의 필드가 없는데 @Getter 어노테이션이 필요한지 확인해주세요. 부모 클래스의 필드에 접근하기 위한 목적이라면 적절합니다.
src/main/java/konkuk/thip/common/dto/SuccessCode.java (1)
5-17: 성공 코드 enum 구현 승인SuccessCode enum이 깔끔하게 구현되었습니다. ResponseCode 인터페이스를 적절히 구현하고 있습니다.
향후 개선 사항 제안:
- 국제화를 위해 메시지를 MessageSource로 외부화하는 것을 고려해보세요
- 성공 코드 20000이 어떤 체계를 따르는지 문서화하면 좋을 것 같습니다
- API_SUCCESS(20000, "요청에 성공했습니다."), + API_SUCCESS(20000, "api.success.message"),src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (1)
24-24: 예외 스택 트레이스 로깅을 고려해보세요.현재는 예외 메시지만 로깅하고 있는데, 개발/스테이징 환경에서는 디버깅을 위해 스택 트레이스가 필요할 수 있습니다. 환경별로 로깅 레벨을 조정하는 것을 권장합니다.
다음과 같이 개선할 수 있습니다:
-log.error("[HttpRequestMethodNotSupportedExceptionHandler] {}", e.getMessage()); +log.error("[HttpRequestMethodNotSupportedExceptionHandler] {}", e.getMessage(), e);또는 환경별 조건부 로깅:
+log.error("[HttpRequestMethodNotSupportedExceptionHandler] {}", e.getMessage()); +if (log.isDebugEnabled()) { + log.debug("Exception stack trace: ", e); +}Also applies to: 32-32, 40-40, 48-48, 56-56, 64-64
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
build.gradle(1 hunks)src/main/java/konkuk/thip/common/dto/BaseResponse.java(1 hunks)src/main/java/konkuk/thip/common/dto/ErrorResponse.java(1 hunks)src/main/java/konkuk/thip/common/dto/ResponseCode.java(1 hunks)src/main/java/konkuk/thip/common/dto/SuccessCode.java(1 hunks)src/main/java/konkuk/thip/common/exception/BusinessException.java(1 hunks)src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java(1 hunks)src/main/java/konkuk/thip/common/exception/InvalidStateException.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java(1 hunks)src/main/java/konkuk/thip/test/TestExceptionController.java(1 hunks)
🔇 Additional comments (14)
build.gradle (2)
28-28: Spring Security 의존성 주석 처리에 대한 주의사항개발 단계에서 예외 처리 구현에 집중하기 위해 Spring Security를 임시로 비활성화한 것으로 이해됩니다. 하지만 프로덕션 배포 전에 반드시 보안 설정을 복원해야 합니다.
Also applies to: 39-39
31-31: Validation 의존성 추가 승인
spring-boot-starter-validation의존성 추가가 적절합니다. 이는 새로 추가된 예외 처리 메커니즘과 잘 연동될 것입니다.src/main/java/konkuk/thip/common/dto/ResponseCode.java (2)
3-11: 깔끔한 인터페이스 설계ResponseCode 인터페이스 설계가 매우 좋습니다. 표준화된 응답 구조를 위한 계약을 명확히 정의하고 있으며, default 메서드를 통한 성공 여부 판단 로직도 적절합니다.
8-10: ```shell
#!/bin/bashSuccessCode 정의 및 참조 위치 확인
echo "1) SuccessCode 클래스/인터페이스/enum 정의 검색"
rg -n "class SuccessCode"
rg -n "interface SuccessCode"
rg -n "enum SuccessCode"echo "2) SuccessCode 전체 참조 검색"
rg -n "SuccessCode"echo "3) SuccessCode.java 파일 존재 여부 확인"
fd "SuccessCode.java"</details> <details> <summary>src/main/java/konkuk/thip/common/exception/InvalidStateException.java (1)</summary> `6-15`: **완전한 예외 클래스 구현** InvalidStateException이 잘 구현되었습니다. 두 가지 생성자를 제공하여 원인 예외 처리에 유연성을 제공하는 것이 좋습니다. </details> <details> <summary>src/main/java/konkuk/thip/common/dto/SuccessCode.java (1)</summary> `7-8`: **향후 확장을 위한 구조** enum 끝의 쉼표와 세미콜론이 향후 추가될 성공 코드들을 위한 준비인 것 같습니다. 좋은 접근입니다. </details> <details> <summary>src/main/java/konkuk/thip/common/dto/BaseResponse.java (2)</summary> `7-9`: **클래스 구조가 잘 설계되어 있습니다.** 제네릭 타입을 사용한 응답 래퍼와 Jackson 어노테이션을 통한 JSON 직렬화 설정이 적절합니다. --- `31-33`: **SuccessCode.API_SUCCESS의 존재 여부를 확인해주세요.** 팩토리 메서드에서 `SuccessCode.API_SUCCESS`를 참조하고 있습니다. 해당 상수가 정의되어 있는지 확인이 필요합니다. 다음 스크립트를 실행하여 SuccessCode 클래스와 API_SUCCESS 상수의 존재를 확인해주세요: ```shell #!/bin/bash # SuccessCode 클래스와 API_SUCCESS 상수 확인 ast-grep --pattern 'enum SuccessCode { $$$ API_SUCCESS$$$ $$$ }' # 또는 SuccessCode 파일 전체 확인 fd "SuccessCode.java" --exec cat {}src/main/java/konkuk/thip/common/exception/BusinessException.java (1)
6-18: 예외 클래스 구조가 올바르게 설계되었습니다.ErrorCode를 포함한 비즈니스 예외 클래스가 적절히 구현되었으며, 예외 체이닝도 올바르게 처리되었습니다.
src/main/java/konkuk/thip/common/dto/ErrorResponse.java (1)
28-30: 팩토리 메서드 구현이 적절합니다.ResponseCode 인터페이스를 활용한 팩토리 메서드가 올바르게 구현되었습니다.
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
17-18: 비즈니스 예외 영역이 비어있습니다.60000번대 비즈니스 예외 영역이 정의되어 있지만 실제 에러 코드가 없습니다. 향후 확장을 위한 준비가 잘 되어있습니다.
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (3)
1-15: 패키지와 임포트 구조가 적절합니다.필요한 의존성들이 올바르게 임포트되어 있고, 패키지 구조도 예외 핸들링 목적에 맞게 적절하게 구성되어 있습니다.
16-18: 클래스 어노테이션이 적절합니다.
@RestControllerAdvice로 전역 예외 처리를 올바르게 설정했고,@Slf4j로 로깅 기능을 추가한 것이 좋습니다.
20-66: 추가 예외 처리 케이스를 고려해보세요.현재 구현된 예외 핸들러들은 기본적인 케이스들을 잘 커버하고 있습니다. 하지만 다음과 같은 추가 예외들도 고려해볼 수 있습니다:
HttpMessageNotReadableException: JSON 파싱 오류ConstraintViolationException: Bean Validation 오류AccessDeniedException: 인증/권한 오류 (Security 적용 시)DataIntegrityViolationException: 데이터베이스 제약 조건 위반프로젝트에서 추가로 처리해야 할 예외 타입들이 있는지 확인해보세요:
#!/bin/bash # 프로젝트에서 사용 중인 Spring 관련 예외들을 찾아봅니다 echo "=== Spring Exception 사용 현황 조사 ===" # Spring Web 관련 예외들 echo "1. Spring Web 예외들:" rg -g "*.java" "HttpMessageNotReadableException|ConstraintViolationException|DataIntegrityViolationException|AccessDeniedException" -A 2 # Validation 관련 예외들 echo "2. Validation 예외들:" rg -g "*.java" "@Valid|@Validated" -A 2 # 데이터베이스 관련 예외들 echo "3. Database 예외들:" rg -g "*.java" "DataAccessException|SQLException|JpaSystemException" -A 2
| @JsonPropertyOrder({"success", "code", "message"}) | ||
| public class ErrorResponse { | ||
|
|
||
| @JsonProperty("isSuccess:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSON 프로퍼티 어노테이션에 오타가 있습니다.
@JsonProperty("isSuccess:") 에서 콜론(:)이 불필요합니다. BaseResponse와 일관성을 위해 수정이 필요합니다.
다음과 같이 수정해주세요:
- @JsonProperty("isSuccess:")
+ @JsonProperty("isSuccess")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @JsonProperty("isSuccess:") | |
| @JsonProperty("isSuccess") |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/dto/ErrorResponse.java at line 11, the
@JsonProperty annotation has an unnecessary colon in the property name
"isSuccess:". Remove the colon so it matches the correct property name
"isSuccess" and ensure consistency with BaseResponse.
| API_BAD_REQUEST(40002, "잘못된 요청입니다."), | ||
| API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), | ||
| API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."), | ||
| API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
에러 코드 중복 문제가 있습니다.
API_BAD_REQUEST(12라인)과 API_INVALID_PARAM(14라인) 모두 같은 코드 40002를 사용하고 있습니다. 이는 클라이언트에서 에러를 구분하기 어렵게 만들 수 있습니다.
다음과 같이 코드를 수정해주세요:
API_BAD_REQUEST(40002, "잘못된 요청입니다."),
API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."),
-API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."),
+API_INVALID_PARAM(40004, "파라미터 값 중 유효하지 않은 값이 있습니다."),
API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| API_BAD_REQUEST(40002, "잘못된 요청입니다."), | |
| API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), | |
| API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."), | |
| API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), | |
| API_BAD_REQUEST(40002, "잘못된 요청입니다."), | |
| API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), | |
| API_INVALID_PARAM(40004, "파라미터 값 중 유효하지 않은 값이 있습니다."), | |
| API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/exception/code/ErrorCode.java between lines
12 and 15, the error codes for API_BAD_REQUEST and API_INVALID_PARAM are both
set to 40002, causing duplication. Change one of these error codes to a unique
value to ensure each error code is distinct and easily identifiable by the
client.
| @GetMapping("/business") | ||
| public void business() { | ||
| throw new EntityNotFoundException(ErrorCode.API_BAD_REQUEST); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
부적절한 ErrorCode 사용입니다.
EntityNotFoundException에 API_BAD_REQUEST를 사용하는 것은 의미적으로 맞지 않습니다. 엔티티를 찾을 수 없는 경우에는 404 관련 에러 코드를 사용해야 합니다.
ErrorCode enum에 다음과 같은 코드를 추가하고 사용하세요:
// ErrorCode.java에 추가
ENTITY_NOT_FOUND(40400, "요청한 리소스를 찾을 수 없습니다."),
// TestExceptionController.java에서 사용
throw new EntityNotFoundException(ErrorCode.ENTITY_NOT_FOUND);🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/test/TestExceptionController.java around lines 36
to 39, the thrown EntityNotFoundException uses an inappropriate error code
API_BAD_REQUEST. To fix this, add a new error code ENTITY_NOT_FOUND with code
40400 and message "요청한 리소스를 찾을 수 없습니다." to the ErrorCode enum, then update the
exception throw statement to use ErrorCode.ENTITY_NOT_FOUND instead of
API_BAD_REQUEST.
| public static class DummyRequest { | ||
| @NotBlank | ||
| public String name; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Java 관례에 맞지 않는 필드 접근 제한자입니다.
public 필드는 Java 관례에 맞지 않습니다. private 필드와 getter/setter를 사용하거나 Lombok의 @Data 어노테이션을 사용하세요.
다음과 같이 수정해주세요:
+import lombok.Data;
+
+@Data
public static class DummyRequest {
@NotBlank
- public String name;
+ private String name;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/test/TestExceptionController.java around lines 46
to 49, the DummyRequest class uses a public field which violates Java
conventions. Change the field access modifier from public to private and add
corresponding getter and setter methods, or alternatively, use Lombok's @Data
annotation on the class to generate them automatically.
| @GetMapping("/invalid-param") | ||
| public void invalidParam(@Valid @RequestBody DummyRequest request) { | ||
| // 유효성 검사 실패 유도 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GET 메서드에서 @RequestBody 사용은 부적절합니다.
GET 요청에서는 일반적으로 요청 본문을 사용하지 않습니다. POST 메서드로 변경하거나 @RequestParam을 사용해야 합니다.
다음과 같이 수정해주세요:
- @GetMapping("/invalid-param")
- public void invalidParam(@Valid @RequestBody DummyRequest request) {
+ @PostMapping("/invalid-param")
+ public void invalidParam(@Valid @RequestBody DummyRequest request) {🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/test/TestExceptionController.java around lines 21
to 24, the method uses @RequestBody with a GET mapping, which is inappropriate
because GET requests should not have a request body. To fix this, change the
HTTP method annotation from @GetMapping to @PostMapping, or alternatively
replace @RequestBody with @RequestParam to accept parameters via the query
string.
| @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) | ||
| @ExceptionHandler({RuntimeException.class, IllegalStateException.class}) | ||
| public ErrorResponse runtimeExceptionHandler(Exception e) { | ||
| log.error("[RuntimeExceptionHandler] {}", e.getMessage()); | ||
| return ErrorResponse.of(ErrorCode.API_SERVER_ERROR); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
예외 처리 범위가 너무 광범위할 수 있습니다.
RuntimeException은 매우 광범위한 예외이므로, 다른 구체적인 예외 핸들러들보다 우선순위가 낮아야 합니다. 또한 예기치 않은 예외들을 포착할 수 있으므로 주의가 필요합니다.
더 구체적인 예외 타입들을 우선 처리하고, RuntimeException은 최후의 안전망으로 사용하는 것이 좋습니다.
핸들러 순서를 고려해보세요:
// 더 구체적인 예외들을 먼저 처리
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
@ExceptionHandler(IllegalStateException.class)
public ErrorResponse illegalStateExceptionHandler(IllegalStateException e) {
log.error("[IllegalStateExceptionHandler] {}", e.getMessage(), e);
return ErrorResponse.of(ErrorCode.API_SERVER_ERROR);
}
// RuntimeException을 가장 마지막 핸들러로 처리
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
-@ExceptionHandler({RuntimeException.class, IllegalStateException.class})
+@ExceptionHandler(RuntimeException.class)
public ErrorResponse runtimeExceptionHandler(Exception e) {
log.error("[RuntimeExceptionHandler] Unexpected exception occurred: {}", e.getMessage(), e);
return ErrorResponse.of(ErrorCode.API_SERVER_ERROR);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) | |
| @ExceptionHandler({RuntimeException.class, IllegalStateException.class}) | |
| public ErrorResponse runtimeExceptionHandler(Exception e) { | |
| log.error("[RuntimeExceptionHandler] {}", e.getMessage()); | |
| return ErrorResponse.of(ErrorCode.API_SERVER_ERROR); | |
| } | |
| // 더 구체적인 예외들을 먼저 처리 | |
| @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) | |
| @ExceptionHandler(IllegalStateException.class) | |
| public ErrorResponse illegalStateExceptionHandler(IllegalStateException e) { | |
| log.error("[IllegalStateExceptionHandler] {}", e.getMessage(), e); | |
| return ErrorResponse.of(ErrorCode.API_SERVER_ERROR); | |
| } | |
| // RuntimeException을 가장 마지막 핸들러로 처리 | |
| @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) | |
| @ExceptionHandler(RuntimeException.class) | |
| public ErrorResponse runtimeExceptionHandler(Exception e) { | |
| log.error("[RuntimeExceptionHandler] Unexpected exception occurred: {}", e.getMessage(), e); | |
| return ErrorResponse.of(ErrorCode.API_SERVER_ERROR); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java
around lines 61 to 66, the current exception handler catches broad exceptions
like RuntimeException and IllegalStateException, which can overshadow more
specific handlers. To fix this, reorder the exception handlers so that more
specific exceptions are handled first, and keep the RuntimeException handler as
a last-resort fallback. This ensures that specific exceptions get appropriate
handling before the broad RuntimeException catch-all.
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (11)
src/main/java/konkuk/thip/common/dto/ResponseCode.java (1)
3-11: ResponseCode 인터페이스 설계 승인 및 개선 제안인터페이스 설계가 깔끔하고 명확한 계약을 제공합니다. 하지만
isSuccess()메서드에서instanceof SuccessCode를 사용하는 것은 구체 클래스와의 결합도를 높입니다.더 나은 대안을 고려해보세요:
public interface ResponseCode { int getCode(); String getMessage(); - default boolean isSuccess() { - return this instanceof SuccessCode; - } + boolean isSuccess(); }각 구현체에서
isSuccess()를 직접 구현하도록 하면 결합도를 낮출 수 있습니다.src/main/java/konkuk/thip/common/exception/BusinessException.java (2)
10-12: 생성자에 메시지 전달을 고려해보세요.첫 번째 생성자에서
super(errorCode.getMessage())를 호출하여 RuntimeException의 메시지를 설정하는 것이 좋겠습니다. 이렇게 하면 스택 트레이스에서 더 유용한 정보를 제공할 수 있습니다.public BusinessException(ErrorCode errorCode) { + super(errorCode.getMessage()); this.errorCode = errorCode; }
14-17: 생성자 순서와 일관성을 개선해보세요.두 번째 생성자에서
super(e)를 먼저 호출하고 errorCode를 설정하는 것이 좋겠습니다. 또한 원인 예외와 errorCode 메시지를 함께 활용하는 방법을 고려해보세요.public BusinessException(ErrorCode errorCode, Exception e) { - super(e); + super(errorCode.getMessage(), e); this.errorCode = errorCode; }src/main/java/konkuk/thip/common/dto/BaseResponse.java (2)
8-8: JsonPropertyOrder에서 필드명 일관성을 맞춰주세요.
@JsonPropertyOrder에서 "success"를 사용했지만, 실제 JSON에서는 "isSuccess"로 출력됩니다. 일관성을 위해 "isSuccess"로 수정하는 것이 좋겠습니다.-@JsonPropertyOrder({"success", "code", "message", "data"}) +@JsonPropertyOrder({"isSuccess", "code", "message", "data"})
31-33: 다양한 응답 타입을 지원하는 팩토리 메서드를 추가 고려해보세요.현재는
API_SUCCESS만 사용하는ok메서드만 있습니다. 다른 성공 코드나 커스텀 메시지를 지원하는 메서드를 추가하면 더 유연한 사용이 가능할 것 같습니다.public static <T> BaseResponse<T> ok(T data) { return new BaseResponse<>(SuccessCode.API_SUCCESS, data); } + +public static <T> BaseResponse<T> of(ResponseCode responseCode, T data) { + return new BaseResponse<>(responseCode, data); +}src/main/java/konkuk/thip/common/dto/ErrorResponse.java (1)
8-8: JsonPropertyOrder에서 필드명 일관성을 맞춰주세요.BaseResponse와 마찬가지로 "success" 대신 "isSuccess"를 사용하는 것이 실제 JSON 출력과 일치합니다.
-@JsonPropertyOrder({"success", "code", "message"}) +@JsonPropertyOrder({"isSuccess", "code", "message"})src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
9-16: 에러 코드 체계를 더 체계적으로 정리해보세요.현재 에러 코드들이 논리적 순서와 다르게 배치되어 있습니다. HTTP 상태 코드와 유사한 패턴으로 정리하면 더 직관적일 것 같습니다.
+// 4xxxx: 클라이언트 에러 API_METHOD_NOT_ALLOWED(40500, "허용되지 않는 HTTP 메소드입니다."), -API_SERVER_ERROR(50000, "서버 내부 오류입니다."), - -API_BAD_REQUEST(40002, "잘못된 요청입니다."), API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), -API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."), +API_BAD_REQUEST(40002, "잘못된 요청입니다."), API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), +API_INVALID_PARAM(40004, "파라미터 값 중 유효하지 않은 값이 있습니다."), + +// 5xxxx: 서버 에러 +API_SERVER_ERROR(50000, "서버 내부 오류입니다."),src/main/java/konkuk/thip/test/TestExceptionController.java (2)
46-49: DummyRequest 클래스의 필드 접근성을 개선해주세요.public 필드보다는 private 필드와 getter/setter를 사용하거나, Lombok을 활용하는 것이 좋겠습니다.
public static class DummyRequest { @NotBlank - public String name; + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } }또는 Lombok을 사용:
+@Getter +@Setter public static class DummyRequest { @NotBlank - public String name; + private String name; }
12-14: 테스트 컨트롤러임을 명시하는 문서화를 추가해보세요.이 컨트롤러가 테스트 목적임을 명확히 하고, 프로덕션 환경에서는 비활성화되어야 함을 문서화하는 것이 좋겠습니다.
+/** + * 예외 처리 테스트를 위한 컨트롤러 + * 프로덕션 환경에서는 비활성화되어야 함 + */ @RestController @RequestMapping("/test") public class TestExceptionController {src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (2)
28-34: 검증 예외 처리에서 상세 정보 제공을 고려해보세요.기본적인 처리는 올바르지만, MethodArgumentNotValidException에서 구체적인 검증 실패 필드 정보를 포함하면 더 유용할 것입니다.
다음과 같이 개선할 수 있습니다:
@ExceptionHandler(MethodArgumentNotValidException.class) public ErrorResponse methodArgumentNotValidExceptionHandler(MethodArgumentNotValidException e) { - log.error("[MethodArgumentNotValidExceptionHandler] {}", e.getMessage()); + String fieldErrors = e.getBindingResult().getFieldErrors().stream() + .map(error -> error.getField() + ": " + error.getDefaultMessage()) + .collect(Collectors.joining(", ")); + log.error("[MethodArgumentNotValidExceptionHandler] Validation failed: {}", fieldErrors); return ErrorResponse.of(ErrorCode.API_INVALID_PARAM); }
24-24: 로깅 레벨 조정을 고려해보세요.모든 예외를 ERROR 레벨로 로깅하고 있는데, 클라이언트 오류(4xx)는 WARN 레벨로, 서버 오류(5xx)는 ERROR 레벨로 구분하는 것이 더 적절할 수 있습니다.
// 클라이언트 오류들 (4xx) - WARN 레벨 -log.error("[HttpRequestMethodNotSupportedExceptionHandler] {}", e.getMessage()); +log.warn("[HttpRequestMethodNotSupportedExceptionHandler] {}", e.getMessage()); // 서버 오류들 (5xx) - ERROR 레벨 유지 log.error("[RuntimeExceptionHandler] {}", e.getMessage());Also applies to: 32-32, 40-40, 48-48, 56-56, 64-64
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
build.gradle(1 hunks)src/main/java/konkuk/thip/common/dto/BaseResponse.java(1 hunks)src/main/java/konkuk/thip/common/dto/ErrorResponse.java(1 hunks)src/main/java/konkuk/thip/common/dto/ResponseCode.java(1 hunks)src/main/java/konkuk/thip/common/dto/SuccessCode.java(1 hunks)src/main/java/konkuk/thip/common/exception/BusinessException.java(1 hunks)src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java(1 hunks)src/main/java/konkuk/thip/common/exception/InvalidStateException.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java(1 hunks)src/main/java/konkuk/thip/test/TestExceptionController.java(1 hunks)
🔇 Additional comments (11)
build.gradle (3)
28-28: Spring Security 의존성 주석 처리에 대한 확인 필요Spring Security 의존성이 주석 처리되었습니다. 이것이 임시적인 조치인지 확인이 필요합니다. 만약 향후 보안 기능이 필요하다면 다시 활성화해야 합니다.
31-31: Validation 의존성 추가 승인Spring Boot Validation 의존성이 적절히 추가되었습니다. 이는 새로 추가되는 예외 처리 프레임워크에서 @Valid 어노테이션과 검증 예외를 지원하는데 필요합니다.
39-39: Spring Security 테스트 의존성 주석 처리 확인Spring Security 테스트 의존성도 함께 주석 처리되었으므로, 메인 Security 의존성과 일관성이 있습니다.
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
5-10: EntityNotFoundException 구현 승인예외 클래스가 적절히 구현되었습니다. BusinessException을 상속받고 ErrorCode를 받는 생성자를 제공하여 일관된 예외 처리 패턴을 따르고 있습니다. 클래스명도 의도를 명확히 나타냅니다.
src/main/java/konkuk/thip/common/dto/SuccessCode.java (1)
5-17: SuccessCode 열거형 구현 승인SuccessCode 열거형이 잘 구현되었습니다:
- ResponseCode 인터페이스를 적절히 구현하고 있습니다
- Lombok @Getter 어노테이션으로 보일러플레이트 코드를 줄였습니다
- 한글 메시지가 프로젝트에 적합합니다
- 코드 20000은 합리적인 번호 체계를 따릅니다
- API_SUCCESS 뒤의 쉼표는 향후 추가 코드를 위한 좋은 관례입니다
src/main/java/konkuk/thip/common/exception/InvalidStateException.java (1)
6-15: InvalidStateException 구현 승인InvalidStateException 클래스가 잘 구현되었습니다:
- BusinessException을 적절히 상속받고 있습니다
- 두 개의 생성자로 다양한 사용 사례를 지원합니다
- ErrorCode와 Exception을 받는 생성자는 예외 체이닝을 가능하게 합니다
- @Getter 어노테이션 사용이 다른 클래스들과 일관됩니다
- 클래스명이 목적을 명확히 나타냅니다
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (5)
1-15: 패키지 구조와 imports가 적절합니다.필요한 의존성들이 올바르게 import되어 있고, 패키지 구조도 일관성 있게 설정되어 있습니다.
16-18: 클래스 선언과 어노테이션이 올바르게 설정되었습니다.@RestControllerAdvice와 @slf4j 어노테이션이 적절하게 사용되어 전역 예외 처리와 로깅 기능이 구현되었습니다.
20-26: HTTP 메서드 예외 처리가 적절합니다.HttpRequestMethodNotSupportedException에 대한 처리가 올바르게 구현되어 있습니다. 405 상태 코드와 적절한 에러 코드 사용이 좋습니다.
36-42: 타입 불일치 예외 처리가 적절합니다.MethodArgumentTypeMismatchException에 대한 처리가 올바르게 구현되어 있습니다.
44-50: 누락된 파라미터 예외 처리가 적절합니다.MissingServletRequestParameterException에 대한 처리가 올바르게 구현되어 있습니다.
| @JsonPropertyOrder({"success", "code", "message"}) | ||
| public class ErrorResponse { | ||
|
|
||
| @JsonProperty("isSuccess:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JsonProperty 어노테이션의 오타를 수정해주세요.
@JsonProperty("isSuccess:") 에서 콜론(:)이 잘못 포함되어 있습니다. 이는 JSON 직렬화에 문제를 일으킬 수 있습니다.
-@JsonProperty("isSuccess:")
+@JsonProperty("isSuccess")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @JsonProperty("isSuccess:") | |
| @JsonProperty("isSuccess") |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/dto/ErrorResponse.java at line 11, the
@JsonProperty annotation has a typo with an extra colon in the property name
"isSuccess:". Remove the colon so it reads @JsonProperty("isSuccess") to ensure
correct JSON serialization.
| API_BAD_REQUEST(40002, "잘못된 요청입니다."), | ||
| API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), | ||
| API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."), | ||
| API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
에러 코드 중복을 수정해주세요.
API_BAD_REQUEST와 API_INVALID_PARAM 모두 40002 코드를 사용하고 있습니다. 이는 에러 코드의 유일성을 깨뜨리는 중요한 문제입니다.
API_BAD_REQUEST(40002, "잘못된 요청입니다."),
API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."),
-API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."),
+API_INVALID_PARAM(40004, "파라미터 값 중 유효하지 않은 값이 있습니다."),
API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| API_BAD_REQUEST(40002, "잘못된 요청입니다."), | |
| API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), | |
| API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."), | |
| API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), | |
| API_BAD_REQUEST(40002, "잘못된 요청입니다."), | |
| API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), | |
| API_INVALID_PARAM(40004, "파라미터 값 중 유효하지 않은 값이 있습니다."), | |
| API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/exception/code/ErrorCode.java between lines
12 and 15, the error codes API_BAD_REQUEST and API_INVALID_PARAM both use the
code 40002, causing duplication. To fix this, assign a unique error code to one
of these constants so that each error code is distinct and does not overlap.
| @GetMapping("/invalid-param") | ||
| public void invalidParam(@Valid @RequestBody DummyRequest request) { | ||
| // 유효성 검사 실패 유도 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
HTTP 메서드를 POST로 변경해주세요.
유효성 검사를 위해 RequestBody를 사용하는데 GET 메서드를 사용하고 있습니다. RequestBody는 일반적으로 POST, PUT 등의 메서드와 함께 사용됩니다.
-@GetMapping("/invalid-param")
+@PostMapping("/invalid-param")
public void invalidParam(@Valid @RequestBody DummyRequest request) {
// 유효성 검사 실패 유도
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @GetMapping("/invalid-param") | |
| public void invalidParam(@Valid @RequestBody DummyRequest request) { | |
| // 유효성 검사 실패 유도 | |
| } | |
| @PostMapping("/invalid-param") | |
| public void invalidParam(@Valid @RequestBody DummyRequest request) { | |
| // 유효성 검사 실패 유도 | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/test/TestExceptionController.java around lines 21
to 24, the method invalidParam uses @GetMapping but accepts a @RequestBody,
which is not appropriate for GET requests. Change the annotation from
@GetMapping to @PostMapping to correctly use the POST HTTP method for this
endpoint that processes a request body.
| @GetMapping("/business") | ||
| public void business() { | ||
| throw new EntityNotFoundException(ErrorCode.API_BAD_REQUEST); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
비즈니스 예외에 적절한 ErrorCode를 사용해주세요.
EntityNotFoundException에 API_BAD_REQUEST 코드를 사용하는 것은 의미적으로 맞지 않습니다. 엔티티를 찾을 수 없는 경우에는 404 유형의 코드가 더 적절할 것 같습니다.
ErrorCode에 엔티티 관련 에러 코드를 추가하거나, 기존 코드 중 더 적절한 것을 선택해주세요.
@GetMapping("/business")
public void business() {
- throw new EntityNotFoundException(ErrorCode.API_BAD_REQUEST);
+ // 적절한 ErrorCode로 변경 필요 (ex: ENTITY_NOT_FOUND)
+ throw new EntityNotFoundException(ErrorCode.API_BAD_REQUEST); // TODO: 적절한 코드로 변경
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @GetMapping("/business") | |
| public void business() { | |
| throw new EntityNotFoundException(ErrorCode.API_BAD_REQUEST); | |
| } | |
| @GetMapping("/business") | |
| public void business() { | |
| // 적절한 ErrorCode로 변경 필요 (ex: ENTITY_NOT_FOUND) | |
| throw new EntityNotFoundException(ErrorCode.API_BAD_REQUEST); // TODO: 적절한 코드로 변경 | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/test/TestExceptionController.java around lines 36
to 39, the business() method throws an EntityNotFoundException with an
inappropriate ErrorCode API_BAD_REQUEST. Replace API_BAD_REQUEST with a more
suitable error code representing a "not found" condition, such as a 404-type
error code. If such a code does not exist in ErrorCode, add a new entity-related
error code to represent this case properly.
| // 서버 내부 오류 예외 처리 | ||
| @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) | ||
| @ExceptionHandler({RuntimeException.class, IllegalStateException.class}) | ||
| public ErrorResponse runtimeExceptionHandler(Exception e) { | ||
| log.error("[RuntimeExceptionHandler] {}", e.getMessage()); | ||
| return ErrorResponse.of(ErrorCode.API_SERVER_ERROR); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
추가 예외 타입 처리를 고려해보세요.
현재 RuntimeException과 IllegalStateException만 처리하고 있지만, 다른 일반적인 Spring 예외들도 처리하는 것이 좋을 것 같습니다.
다음과 같은 예외 타입들을 추가로 처리하는 것을 고려해보세요:
// JSON 파싱 오류 처리
@ResponseStatus(HttpStatus.BAD_REQUEST)
@ExceptionHandler(HttpMessageNotReadableException.class)
public ErrorResponse httpMessageNotReadableExceptionHandler(HttpMessageNotReadableException e) {
log.error("[HttpMessageNotReadableExceptionHandler] {}", e.getMessage());
return ErrorResponse.of(ErrorCode.API_INVALID_JSON);
}
// JPA 제약 조건 위반 처리
@ResponseStatus(HttpStatus.BAD_REQUEST)
@ExceptionHandler(ConstraintViolationException.class)
public ErrorResponse constraintViolationExceptionHandler(ConstraintViolationException e) {
log.error("[ConstraintViolationExceptionHandler] {}", e.getMessage());
return ErrorResponse.of(ErrorCode.API_CONSTRAINT_VIOLATION);
}🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java
around lines 60 to 66, the current exception handler only covers
RuntimeException and IllegalStateException. To improve error handling, add new
methods to handle HttpMessageNotReadableException and
ConstraintViolationException. Each method should be annotated with
@ResponseStatus(HttpStatus.BAD_REQUEST) and @ExceptionHandler for the specific
exception, log the error message, and return the appropriate ErrorResponse with
corresponding error codes like API_INVALID_JSON and API_CONSTRAINT_VIOLATION.
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (4)
src/main/java/konkuk/thip/common/dto/SuccessCode.java (1)
7-8: 향후 확장성 고려사항현재는
API_SUCCESS하나만 정의되어 있지만, 향후 다른 성공 시나리오(생성 완료, 업데이트 완료 등)가 필요할 경우를 대비해 확장 가능한 구조로 잘 설계되었습니다.src/main/java/konkuk/thip/common/dto/BaseResponse.java (1)
31-33: 추가 정적 팩토리 메서드 구현을 권장합니다.현재 성공 응답만을 위한
ok()메서드만 제공되고 있습니다. 일관성을 위해 에러 응답을 위한 팩토리 메서드도 추가하는 것이 좋겠습니다.다음 메서드들을 추가하는 것을 권장합니다:
public static <T> BaseResponse<T> ok(T data) { return new BaseResponse<>(SuccessCode.API_SUCCESS, data); } +public static <T> BaseResponse<T> ok() { + return new BaseResponse<>(SuccessCode.API_SUCCESS, null); +} + +public static <T> BaseResponse<T> error(ErrorCode errorCode) { + return new BaseResponse<>(errorCode, null); +} + +public static <T> BaseResponse<T> error(ErrorCode errorCode, T data) { + return new BaseResponse<>(errorCode, data); +}src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (2)
24-24: 로깅 시 민감한 정보 노출을 방지하세요.현재
e.getMessage()를 직접 로깅하고 있는데, 예외 메시지에 민감한 정보(사용자 데이터, 내부 시스템 정보 등)가 포함될 수 있습니다.프로덕션 환경에서는 다음과 같은 방식을 고려해보세요:
- log.error("[HttpRequestMethodNotSupportedExceptionHandler] {}", e.getMessage()); + log.error("[HttpRequestMethodNotSupportedExceptionHandler] Method not supported: {}", e.getMethod());또는 로깅 레벨을 조정하거나 민감한 정보를 마스킹하는 로직을 추가하는 것을 고려해보세요.
Also applies to: 32-32, 40-40, 48-48, 56-56, 64-64
1-68: 추가적인 Spring 공통 예외들에 대한 핸들러를 고려해보세요.현재 구현된 예외 핸들러 외에도 다음과 같은 Spring의 공통 예외들을 처리하는 것을 고려해보세요:
HttpMessageNotReadableException- JSON 파싱 오류ConstraintViolationException- Bean Validation 오류 (@Valid 외의 검증)HttpMediaTypeNotSupportedException- 지원하지 않는 Content-TypeNoHandlerFoundException- 존재하지 않는 엔드포인트다음 스크립트를 실행하여 프로젝트에서 처리해야 할 추가 예외들을 확인해보세요:
#!/bin/bash # Description: 프로젝트에서 사용되는 Spring 예외들을 찾아 누락된 핸들러가 있는지 확인 # Spring의 일반적인 예외들이 프로젝트에서 사용되는지 확인 rg -n "HttpMessageNotReadableException|ConstraintViolationException|HttpMediaTypeNotSupportedException|NoHandlerFoundException" --type java
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
build.gradle(1 hunks)src/main/java/konkuk/thip/common/dto/BaseResponse.java(1 hunks)src/main/java/konkuk/thip/common/dto/ErrorResponse.java(1 hunks)src/main/java/konkuk/thip/common/dto/ResponseCode.java(1 hunks)src/main/java/konkuk/thip/common/dto/SuccessCode.java(1 hunks)src/main/java/konkuk/thip/common/exception/BusinessException.java(1 hunks)src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java(1 hunks)src/main/java/konkuk/thip/common/exception/InvalidStateException.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java(1 hunks)src/main/java/konkuk/thip/test/TestExceptionController.java(1 hunks)
🔇 Additional comments (8)
build.gradle (2)
31-31: validation starter 추가 승인요청 파라미터 검증을 위한 spring-boot-starter-validation 의존성 추가가 적절합니다. 새로운 예외 처리 프레임워크와 잘 어울립니다.
28-28: 아래 스크립트로 보다 광범위하게 Spring Security 사용 여부를 재확인해 보세요:#!/bin/bash echo "=== Spring Security imports across all files ===" rg "import.*springframework\\.security" echo "=== Security annotations 확인 ===" rg "@(EnableWebSecurity|EnableGlobalMethodSecurity|PreAuthorize|PostAuthorize|Secured)" echo "=== SecurityConfig 관련 클래스 및 설정 확인 ===" rg "SecurityConfig|WebSecurityConfig|WebSecurityConfigurerAdapter|SecurityFilterChain" echo "=== authorizeRequests 호출 확인 ===" rg "\\.authorizeRequests\\("src/main/java/konkuk/thip/common/dto/ResponseCode.java (2)
8-10: instanceof 체크 방식의 타입 결합도 검토
isSuccess()메서드가SuccessCode타입을 직접 확인하는 방식이 인터페이스와 구현체 간의 결합도를 높일 수 있습니다. 하지만 현재 구조에서는 명확하고 간단한 방법이므로 적절한 선택으로 보입니다.
3-11: 응답 코드 인터페이스 설계 승인깔끔하고 명확한 인터페이스 설계입니다. 성공/실패 응답 코드를 표준화하는 좋은 접근 방식입니다.
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
5-10: 엔티티 NotFoundException 구현 승인
BusinessException을 상속받아 표준화된 에러 코드를 사용하는 깔끔한 예외 클래스 구현입니다. 단일 책임 원칙을 잘 따르고 있습니다.src/main/java/konkuk/thip/common/exception/InvalidStateException.java (1)
6-15: InvalidStateException 구현 승인예외 체이닝을 지원하는 두 가지 생성자와 Lombok
@Getter사용으로 깔끔하게 구현되었습니다. 중첩된 예외를 처리할 수 있어 디버깅에 도움이 됩니다.src/main/java/konkuk/thip/common/dto/SuccessCode.java (1)
5-17: SuccessCode 열거형 구현 승인
ResponseCode인터페이스를 구현한 깔끔한 성공 응답 코드 정의입니다. 한국어 메시지와 의미 있는 코드(20000) 사용이 적절합니다.src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (1)
16-18: 글로벌 예외 핸들러 구조가 잘 설계되었습니다.
@RestControllerAdvice와@Slf4j어노테이션을 적절히 사용하여 중앙집중식 예외 처리 구조를 구현하였습니다. 일관된 ErrorResponse 반환 패턴도 잘 적용되었습니다.
| public BusinessException(ErrorCode errorCode) { | ||
| this.errorCode = errorCode; | ||
| } | ||
|
|
||
| public BusinessException(ErrorCode errorCode, Exception e) { | ||
| super(e); | ||
| this.errorCode = errorCode; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
생성자 설계 개선이 필요합니다.
현재 구현에서 예외 메시지가 제대로 전달되지 않는 문제가 있습니다. RuntimeException의 message 필드가 설정되지 않아 스택 트레이스에서 의미 있는 메시지를 확인할 수 없습니다.
다음과 같이 개선하는 것을 권장합니다:
public BusinessException(ErrorCode errorCode) {
- this.errorCode = errorCode;
+ super(errorCode.getMessage());
+ this.errorCode = errorCode;
}
public BusinessException(ErrorCode errorCode, Exception e) {
- super(e);
+ super(errorCode.getMessage(), e);
this.errorCode = errorCode;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public BusinessException(ErrorCode errorCode) { | |
| this.errorCode = errorCode; | |
| } | |
| public BusinessException(ErrorCode errorCode, Exception e) { | |
| super(e); | |
| this.errorCode = errorCode; | |
| } | |
| public BusinessException(ErrorCode errorCode) { | |
| super(errorCode.getMessage()); | |
| this.errorCode = errorCode; | |
| } | |
| public BusinessException(ErrorCode errorCode, Exception e) { | |
| super(errorCode.getMessage(), e); | |
| this.errorCode = errorCode; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/exception/BusinessException.java around
lines 10 to 17, the constructors do not properly set the exception message,
causing the RuntimeException message field to be empty and uninformative in
stack traces. To fix this, modify the constructors to pass a meaningful message
to the superclass RuntimeException by calling super() with the error message
from the ErrorCode or the cause exception, ensuring the message field is
properly initialized for better debugging.
| @JsonPropertyOrder({"success", "code", "message"}) | ||
| public class ErrorResponse { | ||
|
|
||
| @JsonProperty("isSuccess:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSON 프로퍼티명 오타를 수정해주세요.
@JsonProperty("isSuccess:") 에서 콜론(:)이 잘못 포함되어 있습니다. 이는 JSON 직렬화 시 잘못된 프로퍼티명을 생성할 수 있습니다.
다음과 같이 수정해주세요:
-@JsonProperty("isSuccess:")
+@JsonProperty("isSuccess")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @JsonProperty("isSuccess:") | |
| @JsonProperty("isSuccess") |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/dto/ErrorResponse.java at line 11, the
@JsonProperty annotation has an incorrect property name with an extra colon
("isSuccess:"). Remove the colon so that the annotation reads
@JsonProperty("isSuccess") to ensure correct JSON serialization property naming.
| API_BAD_REQUEST(40002, "잘못된 요청입니다."), | ||
| API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), | ||
| API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."), | ||
| API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
에러 코드 중복을 수정해주세요.
API_BAD_REQUEST와 API_INVALID_PARAM 모두 40002 코드를 사용하고 있습니다. 이는 코드 충돌을 일으킬 수 있습니다.
다음과 같이 수정하는 것을 권장합니다:
-API_BAD_REQUEST(40002, "잘못된 요청입니다."),
+API_BAD_REQUEST(40000, "잘못된 요청입니다."),
API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."),
API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."),
API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| API_BAD_REQUEST(40002, "잘못된 요청입니다."), | |
| API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), | |
| API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."), | |
| API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), | |
| API_BAD_REQUEST(40000, "잘못된 요청입니다."), | |
| API_MISSING_PARAM(40001, "필수 파라미터가 없습니다."), | |
| API_INVALID_PARAM(40002, "파라미터 값 중 유효하지 않은 값이 있습니다."), | |
| API_INVALID_TYPE(40003, "파라미터 타입이 잘못되었습니다."), |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/exception/code/ErrorCode.java between lines
12 and 15, the error codes API_BAD_REQUEST and API_INVALID_PARAM both use the
code 40002, causing a conflict. Change one of these error codes to a unique
value to avoid duplication, for example, assign a new distinct code to
API_INVALID_PARAM while keeping API_BAD_REQUEST as 40002.
| /* 60000부터 비즈니스 예외 */ | ||
| ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
비즈니스 예외 코드 추가를 권장합니다.
현재 비즈니스 예외를 위한 60000번대 코드 범위가 예약되어 있지만 실제 코드가 정의되지 않았습니다. TestExceptionController에서 사용하는 EntityNotFoundException을 위한 코드를 추가하는 것이 좋겠습니다.
다음과 같이 비즈니스 예외 코드를 추가하는 것을 권장합니다:
/* 60000부터 비즈니스 예외 */
+ENTITY_NOT_FOUND(60001, "요청한 리소스를 찾을 수 없습니다."),
+INVALID_STATE(60002, "잘못된 상태입니다."),
;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* 60000부터 비즈니스 예외 */ | |
| ; | |
| /* 60000부터 비즈니스 예외 */ | |
| ENTITY_NOT_FOUND(60001, "요청한 리소스를 찾을 수 없습니다."), | |
| INVALID_STATE(60002, "잘못된 상태입니다."), | |
| ; |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/exception/code/ErrorCode.java around lines
17 to 18, the 60000 range for business exception codes is reserved but no actual
codes are defined. Add a new error code constant for EntityNotFoundException
within this range to represent that specific business exception. Define it with
a unique code number and a descriptive message to be used in
TestExceptionController.
| @GetMapping("/business") | ||
| public void business() { | ||
| throw new EntityNotFoundException(ErrorCode.API_BAD_REQUEST); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
부적절한 에러 코드 사용을 수정해주세요.
EntityNotFoundException에서 API_BAD_REQUEST 코드를 사용하는 것이 부적절합니다. 엔티티를 찾을 수 없는 경우에는 더 적절한 에러 코드를 사용해야 합니다.
ErrorCode에 적절한 코드를 추가하고 다음과 같이 수정하는 것을 권장합니다:
@GetMapping("/business")
public void business() {
- throw new EntityNotFoundException(ErrorCode.API_BAD_REQUEST);
+ throw new EntityNotFoundException(ErrorCode.ENTITY_NOT_FOUND);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @GetMapping("/business") | |
| public void business() { | |
| throw new EntityNotFoundException(ErrorCode.API_BAD_REQUEST); | |
| } | |
| @GetMapping("/business") | |
| public void business() { | |
| throw new EntityNotFoundException(ErrorCode.ENTITY_NOT_FOUND); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/test/TestExceptionController.java around lines 36
to 39, the EntityNotFoundException is thrown with an inappropriate error code
API_BAD_REQUEST. Update the ErrorCode enum to include a more suitable code
representing "entity not found" and modify the exception to use this new error
code instead of API_BAD_REQUEST.
| public static class DummyRequest { | ||
| @NotBlank | ||
| public String name; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
데이터 클래스 설계를 개선해주세요.
DummyRequest 클래스의 필드가 public으로 선언되어 있고 getter/setter가 없습니다. 이는 데이터 캡슐화 원칙에 위배되며 Jackson 직렬화에 문제를 일으킬 수 있습니다.
다음과 같이 개선하는 것을 권장합니다:
public static class DummyRequest {
@NotBlank
- public String name;
+ private String name;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
}또는 Lombok을 사용하여 더 간단하게 구현할 수 있습니다:
+@Getter
+@Setter
public static class DummyRequest {
@NotBlank
- public String name;
+ private String name;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static class DummyRequest { | |
| @NotBlank | |
| public String name; | |
| } | |
| public static class DummyRequest { | |
| @NotBlank | |
| private String name; | |
| public String getName() { | |
| return name; | |
| } | |
| public void setName(String name) { | |
| this.name = name; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/test/TestExceptionController.java around lines 46
to 49, the DummyRequest class has a public field without getters or setters,
violating encapsulation and potentially causing Jackson serialization issues.
Refactor the class by making the field private and adding public getter and
setter methods, or alternatively, use Lombok annotations like @Data to generate
them automatically.
| @GetMapping("/invalid-param") | ||
| public void invalidParam(@Valid @RequestBody DummyRequest request) { | ||
| // 유효성 검사 실패 유도 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
HTTP 메서드와 요청 방식이 일치하지 않습니다.
@GetMapping으로 정의된 엔드포인트에서 @RequestBody를 사용하고 있습니다. GET 요청은 일반적으로 요청 본문을 가지지 않습니다.
다음과 같이 수정하는 것을 권장합니다:
-@GetMapping("/invalid-param")
-public void invalidParam(@Valid @RequestBody DummyRequest request) {
+@PostMapping("/invalid-param")
+public void invalidParam(@Valid @RequestBody DummyRequest request) {
// 유효성 검사 실패 유도
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @GetMapping("/invalid-param") | |
| public void invalidParam(@Valid @RequestBody DummyRequest request) { | |
| // 유효성 검사 실패 유도 | |
| } | |
| @PostMapping("/invalid-param") | |
| public void invalidParam(@Valid @RequestBody DummyRequest request) { | |
| // 유효성 검사 실패 유도 | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/test/TestExceptionController.java around lines 21
to 24, the method is annotated with @GetMapping but uses @RequestBody, which is
incompatible because GET requests should not have a body. Change the annotation
from @GetMapping to @PostMapping or another HTTP method that supports a request
body to align the HTTP method with the use of @RequestBody.
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java
Outdated
Show resolved
Hide resolved
hd0rable
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨어욧!! 👍🏻👍🏻 토끼가 말이참 많네여 ㅡㅡ
| @JsonPropertyOrder({"success", "code", "message", "data"}) | ||
| public class BaseResponse<T> { | ||
|
|
||
| @JsonProperty("isSuccess") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 이건 어떨때 json이랑 매핑하는건가요??
저희 명세서는 success로 되어있어서 물어봅니닷!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 isSuccess가 더 명시적일 것 같아서 수정하였습니다! 명세는 어차피 구현 후에 반환된 응답 붙여 넣을꺼라 형식만 맞추려고했습니다!
| @JsonPropertyOrder({"success", "code", "message"}) | ||
| public class ErrorResponse { | ||
|
|
||
| @JsonProperty("isSuccess:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기두욥!
| @Slf4j | ||
| @RestControllerAdvice | ||
| public class GlobalExceptionHandler { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: NoHandlerFoundException.class 예외처리도 추가하는 것은 어떨까요?
| //요청한 API가 없을때 예외처리 | |
| @ExceptionHandler(NoHandlerFoundException.class) | |
| public ErrorResponse handleNoHandlerFoundException(NoHandlerFoundException e) { | |
| return new ErrorResponse(NOT_FOUND_API); | |
| } |
이 예외처리를 해두면 프론트엔드분들과 협업할때도 그렇구, 포스트맨에서도 확인할때 정의하지않은 경로로 요청했을때나 오타같은 것들을 바로 확인할수 있어서 좋더라고요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 예외처리를 처음에 핸들러로 추가해놨는데 테스트에서 핸들링이 안되더라구요.
확인해보니 핸들러가 없을때 예외가 발생하게 하려면 application.yml에
spring:
mvc:
throw-exception-if-no-handler-found: true
web:
resources:
add-mappings: false다음과 같은 옵션에 들어가야 하는데 Spring Boot 3.0 버전에서는 deprecated 된 것을 확인했습니다 🥲
혹시 희진님은 이전 프로젝트에서 어떻게 핸들링하셨나욥?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 어떻게 테스트에서 핸들링이 안되셨는지..
이 예외처리 핸들러 추가하고 정의하지않은 경로에대해 요청을 날리면 핸들링이 되는것으로 알고 있어서요..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 핸들링만추가하고 딱히 yml에 다른 설정같은건 하지않았어요!
저도 Spring Boot 3.0 버전 사용했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인해보니까 spring.mvc.throw-exception-if-no-handler-found는 deprecated 되었는데 spring.web.resoucres.app-mappings는 설정해줘야 에러를 발생시켜 주네요! 핸들링해서 다시 커밋 날렸습니다 확인 부탁드릴게요~
hd0rable
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨어욧~~👍🏻💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (3)
59-65: 비즈니스 예외의 HTTP 상태 코드를 동적으로 처리하도록 개선이 필요합니다.모든 BusinessException을 400 Bad Request로 처리하고 있는데, 예외의 성격에 따라 다른 상태 코드가 더 적절할 수 있습니다. 예를 들어, EntityNotFoundException은 404 Not Found가 더 적합합니다.
67-73: RuntimeException 처리 범위가 너무 광범위할 수 있습니다.RuntimeException은 매우 광범위한 예외이므로, 다른 구체적인 예외 핸들러들보다 우선순위가 낮아야 합니다. 또한 예기치 않은 예외들을 포착할 수 있으므로 주의가 필요합니다.
67-73: 추가 예외 타입 처리를 고려해보세요.HttpMessageNotReadableException, ConstraintViolationException 등의 일반적인 Spring 예외들도 처리하는 것이 좋을 것 같습니다.
🧹 Nitpick comments (1)
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (1)
21-25: 로깅 일관성을 위해 로그 추가를 고려해보세요.다른 예외 핸들러들과 달리 이 핸들러에는 로깅이 없습니다. 일관성을 위해 로그를 추가하는 것이 좋겠습니다.
@ResponseStatus(HttpStatus.NOT_FOUND) @ExceptionHandler(NoHandlerFoundException.class) public ErrorResponse noHandlerExceptionHandler(NoHandlerFoundException e) { + log.error("[NoHandlerExceptionHandler] {}", e.getMessage()); return ErrorResponse.of(ErrorCode.API_NOT_FOUND); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
🔇 Additional comments (4)
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (4)
1-16: 패키지 선언과 import 구문이 적절합니다.필요한 Spring 예외 처리 관련 클래스들이 모두 적절히 import되어 있습니다.
17-19: 클래스 선언과 어노테이션이 적절합니다.@RestControllerAdvice와 @slf4j 어노테이션이 적절히 사용되어 전역 예외 처리 클래스로서 올바르게 구성되었습니다.
27-33: HTTP 메소드 예외 처리가 적절합니다.허용되지 않은 HTTP 메소드에 대한 예외 처리가 올바른 상태 코드(405)와 함께 적절히 구현되었습니다.
35-57: 유효성 검사 관련 예외 처리가 잘 구현되었습니다.파라미터 유효성 검사, 타입 불일치, 파라미터 누락에 대한 예외 처리가 적절한 HTTP 상태 코드와 함께 일관성 있게 구현되었습니다.
| dependencies { | ||
| implementation 'org.springframework.boot:spring-boot-starter-data-jpa' | ||
| implementation 'org.springframework.boot:spring-boot-starter-security' | ||
| // implementation 'org.springframework.boot:spring-boot-starter-security' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3 : security 의존성은 왜 주석처리 하신건가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Postman 테스트를 위해서 일시적으로 주석처리해두었습니다! 다음 PR에서 다시 주석해제 해둘 예정입니다!
| int getCode(); | ||
|
|
||
| String getMessage(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p2 : HttpStatus 도 추가하는 건 어떤가요?? 어떤 HttpStatus 값의 error 인지 enum 클래스에서 바로 알 수 있는 장점이 있을 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 좋은 것 같습니다! 수정하겠습니다.
|



#️⃣ 연관된 이슈
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
기타