-
Notifications
You must be signed in to change notification settings - Fork 0
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
ISSUE-3 API 응답 Spec & 예외 Setting #9
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package org.doorip.core | ||
|
||
import org.doorip.domain.InvalidRequestValueException | ||
import org.springframework.stereotype.Service | ||
|
||
@Service | ||
class TestService { | ||
|
||
fun throwDooripException() { | ||
throw InvalidRequestValueException | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,47 @@ | ||||||||||||||
package org.doorip.domain | ||||||||||||||
|
||||||||||||||
sealed class DooripException( | ||||||||||||||
val code: String, | ||||||||||||||
message: String, | ||||||||||||||
cause: Throwable? = null, | ||||||||||||||
) : RuntimeException(message, cause) | ||||||||||||||
|
||||||||||||||
// Client Exception | ||||||||||||||
sealed class ClientException( | ||||||||||||||
code: String, | ||||||||||||||
message: String, | ||||||||||||||
) : DooripException(code, message) | ||||||||||||||
|
||||||||||||||
class UnauthorizedException( | ||||||||||||||
code: String, | ||||||||||||||
message: String, | ||||||||||||||
) : ClientException(code, message) | ||||||||||||||
|
||||||||||||||
class UnauthenticatedException( | ||||||||||||||
code: String, | ||||||||||||||
message: String, | ||||||||||||||
) : ClientException(code, message) | ||||||||||||||
|
||||||||||||||
data object InvalidRequestValueException : ClientException("e4000", "잘못된 요청입니다.") { private fun readResolve(): Any = InvalidRequestValueException } | ||||||||||||||
data object MethodNotAllowedException : ClientException("e4050", "잘못된 HTTP method 요청입니다.") { private fun readResolve(): Any = MethodNotAllowedException } | ||||||||||||||
data object ConflictException : ClientException("e4090", "이미 존재하는 리소스입니다.") { private fun readResolve(): Any = ConflictException } | ||||||||||||||
Comment on lines
+25
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 예외 클래스의
불필요한 코드를 제거하여 코드를 간결하게 유지하는 것을 권장합니다. data object InvalidRequestValueException : ClientException("e4000", "잘못된 요청입니다.") {
- private fun readResolve(): Any = InvalidRequestValueException
} 📝 Committable suggestion
Suggested change
|
||||||||||||||
|
||||||||||||||
// Server Exception | ||||||||||||||
sealed class ServerException( | ||||||||||||||
code: String, | ||||||||||||||
message: String, | ||||||||||||||
) : DooripException(code, message) | ||||||||||||||
|
||||||||||||||
data object NotFoundException : ServerException("e4040", "대상을 찾을 수 없습니다.") { private fun readResolve(): Any = NotFoundException } | ||||||||||||||
data object InternalServerException : ServerException("e5000", "서버 내부 오류입니다.") { private fun readResolve(): Any = InternalServerException } | ||||||||||||||
|
||||||||||||||
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 예외 클래스의
위에서 제안한 것처럼 data object NotFoundException : ServerException("e4040", "대상을 찾을 수 없습니다.") {
- private fun readResolve(): Any = NotFoundException
}
|
||||||||||||||
// Critical Exception | ||||||||||||||
sealed class CriticalException( | ||||||||||||||
code: String, | ||||||||||||||
message: String, | ||||||||||||||
cause: Throwable? = null, | ||||||||||||||
) : DooripException(code, message, cause) | ||||||||||||||
|
||||||||||||||
class UnknownException( | ||||||||||||||
cause: Throwable? = null, | ||||||||||||||
) : CriticalException("e6000", "정의되지 않은 예외입니다. (로그 확인이 필요합니다.)", cause) |
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||||||||||||||||||
package org.doorip.api | ||||||||||||||||||||||
|
||||||||||||||||||||||
import org.doorip.api.dto.ApiResponse | ||||||||||||||||||||||
import org.doorip.core.TestService | ||||||||||||||||||||||
import org.springframework.http.ResponseEntity | ||||||||||||||||||||||
import org.springframework.stereotype.Controller | ||||||||||||||||||||||
import org.springframework.web.bind.annotation.GetMapping | ||||||||||||||||||||||
import org.springframework.web.bind.annotation.ResponseBody | ||||||||||||||||||||||
Comment on lines
+6
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive)
현재
-@Controller
+@RestController
class TestController(
private val testService: TestService,
) {
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@Controller | ||||||||||||||||||||||
class TestController( | ||||||||||||||||||||||
private val testService: TestService, | ||||||||||||||||||||||
) { | ||||||||||||||||||||||
|
||||||||||||||||||||||
@ResponseBody | ||||||||||||||||||||||
@GetMapping("/api/test") | ||||||||||||||||||||||
fun test() = "doorip ok" | ||||||||||||||||||||||
Comment on lines
+15
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 라우트 핸들러 메서드의 반환 타입 일관성 확인 필요
@GetMapping("/api/test")
-fun test() = "doorip ok"
+fun test(): ResponseEntity<ApiResponse<String>> {
+ return ApiResponse.ok("doorip ok")
+} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@GetMapping("/api/test/ok") | ||||||||||||||||||||||
fun ok(): ResponseEntity<ApiResponse<Unit>> { | ||||||||||||||||||||||
return ApiResponse.ok() | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
@ResponseBody | ||||||||||||||||||||||
@GetMapping("/api/test/ex") | ||||||||||||||||||||||
fun exception() { | ||||||||||||||||||||||
testService.throwDooripException() | ||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+24
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 예외 발생 메서드 호출 이름 수정 필요
앞서 제안한 대로 메서드명을 변경했다면, 여기에서도 동일하게 적용해주세요. @GetMapping("/api/test/ex")
fun exception() {
- testService.throwDooripException()
+ testService.testException()
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,38 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
package org.doorip.api.dto | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.springframework.http.HttpStatus | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.springframework.http.ResponseEntity | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
data class ApiResponse<T>( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
val status: Int, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
val code: String, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
val message: String, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
val data: T?, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+6
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) API 응답 클래스에 문서화를 추가해보세요 API 응답의 표준화를 위한 좋은 구조네요! 다만, 각 필드의 의미와 사용 방법에 대한 KDoc 문서화를 추가하면 더 좋을 것 같아요. 😊 아래와 같이 문서화를 추가해보는건 어떨까요? +/**
+ * API 응답을 표준화하기 위한 데이터 클래스
+ *
+ * @param status HTTP 상태 코드
+ * @param code 비즈니스 응답 코드
+ * @param message 응답 메시지
+ * @param data 응답 데이터 (nullable)
+ */
data class ApiResponse<T>(
val status: Int,
val code: String,
val message: String,
val data: T?,
) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
companion object { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fun <T> ok( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
data: T? = null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
): ResponseEntity<ApiResponse<T>> = ResponseEntity.ok( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ApiResponse( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
status = 200, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
code = "s2000", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
message = "요청이 성공했습니다.", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
data = data, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+14
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 성공 코드를 상수로 분리하면 어떨까요? 성공 응답을 생성하는 메소드가 잘 구현되어 있네요! 코드의 재사용성과 유지보수성을 높이기 위해 상태 코드와 메시지를 상수로 분리하는 것은 어떨까요? 🤔 다음과 같이 변경해보는 건 어떨까요? + companion object {
+ private const val SUCCESS_STATUS = 200
+ private const val SUCCESS_CODE = "s2000"
+ private const val SUCCESS_MESSAGE = "요청이 성공했습니다."
+
fun <T> ok(
data: T? = null,
): ResponseEntity<ApiResponse<T>> = ResponseEntity.ok(
ApiResponse(
- status = 200,
- code = "s2000",
- message = "요청이 성공했습니다.",
+ status = SUCCESS_STATUS,
+ code = SUCCESS_CODE,
+ message = SUCCESS_MESSAGE,
data = data,
),
)
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fun <T> created( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
data: T? = null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
): ResponseEntity<ApiResponse<T>> = ResponseEntity.status( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
HttpStatus.CREATED, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
).body( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ApiResponse( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
status = 201, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
code = "s2010", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
message = "요청이 성공했습니다.", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
data = data, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+25
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) created() 메소드도 동일한 패턴으로 개선하면 좋겠어요
+ private const val CREATED_STATUS = 201
+ private const val CREATED_CODE = "s2010"
+ private const val CREATED_MESSAGE = "요청이 성공했습니다."
+
fun <T> created(
data: T? = null,
): ResponseEntity<ApiResponse<T>> = ResponseEntity.status(
HttpStatus.CREATED,
).body(
ApiResponse(
- status = 201,
- code = "s2010",
- message = "요청이 성공했습니다.",
+ status = CREATED_STATUS,
+ code = CREATED_CODE,
+ message = CREATED_MESSAGE,
data = data,
),
) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,7 @@ | ||||||||||||||||||||||
package org.doorip.api.dto | ||||||||||||||||||||||
|
||||||||||||||||||||||
data class ExceptionResponse( | ||||||||||||||||||||||
val status: Int, | ||||||||||||||||||||||
val code: String, | ||||||||||||||||||||||
val message: String?, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
Comment on lines
+3
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive)
현재
data class ExceptionResponse(
val status: Int,
val code: String,
- val message: String?,
+ val message: String,
) 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,58 @@ | ||||||||||||||||||
package org.doorip.api.exception | ||||||||||||||||||
|
||||||||||||||||||
import org.doorip.api.dto.ExceptionResponse | ||||||||||||||||||
import org.doorip.domain.DooripException | ||||||||||||||||||
import org.doorip.domain.InvalidRequestValueException | ||||||||||||||||||
import org.doorip.domain.MethodNotAllowedException | ||||||||||||||||||
import org.doorip.domain.UnknownException | ||||||||||||||||||
import org.springframework.http.ResponseEntity | ||||||||||||||||||
import org.springframework.validation.BindException | ||||||||||||||||||
import org.springframework.web.HttpRequestMethodNotSupportedException | ||||||||||||||||||
import org.springframework.web.bind.MethodArgumentNotValidException | ||||||||||||||||||
import org.springframework.web.bind.annotation.ControllerAdvice | ||||||||||||||||||
import org.springframework.web.bind.annotation.ExceptionHandler | ||||||||||||||||||
import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException | ||||||||||||||||||
import org.springframework.web.servlet.resource.NoResourceFoundException | ||||||||||||||||||
|
||||||||||||||||||
typealias ExceptionResponseEntity = ResponseEntity<ExceptionResponse> | ||||||||||||||||||
|
||||||||||||||||||
@ControllerAdvice | ||||||||||||||||||
internal class ApiExceptionHandler( | ||||||||||||||||||
private val exceptionResponseFactory: ExceptionResponseFactory, | ||||||||||||||||||
) { | ||||||||||||||||||
Comment on lines
+19
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
현재
-internal class ApiExceptionHandler(
+class ApiExceptionHandler(
private val exceptionResponseFactory: ExceptionResponseFactory,
) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
@ExceptionHandler(MethodArgumentTypeMismatchException::class) | ||||||||||||||||||
protected fun handleException(ex: MethodArgumentTypeMismatchException): ExceptionResponseEntity { | ||||||||||||||||||
return exceptionResponseFactory.create(InvalidRequestValueException) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
@ExceptionHandler(MethodArgumentNotValidException::class) | ||||||||||||||||||
protected fun handleException(ex: MethodArgumentNotValidException): ExceptionResponseEntity { | ||||||||||||||||||
return exceptionResponseFactory.create(InvalidRequestValueException) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
@ExceptionHandler(BindException::class) | ||||||||||||||||||
protected fun handleException(ex: BindException): ExceptionResponseEntity { | ||||||||||||||||||
return exceptionResponseFactory.create(InvalidRequestValueException) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
@ExceptionHandler(NoResourceFoundException::class) | ||||||||||||||||||
protected fun handleException(ex: NoResourceFoundException): ExceptionResponseEntity { | ||||||||||||||||||
return exceptionResponseFactory.create(InvalidRequestValueException) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
@ExceptionHandler(HttpRequestMethodNotSupportedException::class) | ||||||||||||||||||
protected fun handleException(ex: HttpRequestMethodNotSupportedException): ExceptionResponseEntity { | ||||||||||||||||||
return exceptionResponseFactory.create(MethodNotAllowedException) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
@ExceptionHandler(DooripException::class) | ||||||||||||||||||
protected fun handleException(ex: DooripException): ExceptionResponseEntity { | ||||||||||||||||||
return exceptionResponseFactory.create(ex) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
@ExceptionHandler(Exception::class) | ||||||||||||||||||
protected fun handleException(ex: Exception): ExceptionResponseEntity { | ||||||||||||||||||
return exceptionResponseFactory.create(UnknownException(ex)) | ||||||||||||||||||
} | ||||||||||||||||||
Comment on lines
+55
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 예상치 못한 예외 처리에서 로그 추가를 권장합니다
예외를 로깅하여 추후 문제 발생 시 원인을 파악하기 쉽게 만들어주세요. @ExceptionHandler(Exception::class)
protected fun handleException(ex: Exception): ExceptionResponseEntity {
logger.error("Unhandled exception caught", ex)
return exceptionResponseFactory.create(UnknownException(ex))
} 로그 관리를 위한 로깅 프레임워크의 활용을 고려해주세요. |
||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,46 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
package org.doorip.api.exception | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.api.dto.ExceptionResponse | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.domain.ClientException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.domain.ConflictException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.domain.CriticalException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.domain.DooripException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.domain.MethodNotAllowedException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.domain.NotFoundException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.domain.ServerException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.domain.UnauthenticatedException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.doorip.domain.UnauthorizedException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.springframework.http.HttpStatus | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.springframework.http.ResponseEntity | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.springframework.stereotype.Component | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@Component | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
internal class ExceptionResponseFactory { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fun create(exception: DooripException): ResponseEntity<ExceptionResponse> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
val httpStatus = exception.getHttpStatus() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
val exceptionResponse = ExceptionResponse( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
status = httpStatus.value(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
code = exception.code, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
message = exception.message, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ResponseEntity.status(httpStatus) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.body(exceptionResponse) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+17
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 예외 처리에 로깅을 추가하면 좋을 것 같아요 예외 응답을 생성하는 팩토리가 깔끔하게 구현되어 있네요! 하지만 예외가 발생했을 때 디버깅을 위해 로깅을 추가하면 좋을 것 같아요. 😊 로깅을 추가하는 예시입니다: +import org.slf4j.LoggerFactory
+
@Component
internal class ExceptionResponseFactory {
+ private val logger = LoggerFactory.getLogger(javaClass)
+
fun create(exception: DooripException): ResponseEntity<ExceptionResponse> {
+ logger.error("예외 발생: ${exception.message}", exception)
+
val httpStatus = exception.getHttpStatus()
val exceptionResponse = ExceptionResponse(
status = httpStatus.value(), 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
internal fun DooripException.getHttpStatus(): HttpStatus = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
when (this) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
is UnauthorizedException -> HttpStatus.FORBIDDEN | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
is UnauthenticatedException -> HttpStatus.UNAUTHORIZED | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
MethodNotAllowedException -> HttpStatus.METHOD_NOT_ALLOWED | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ConflictException -> HttpStatus.CONFLICT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
NotFoundException -> HttpStatus.NOT_FOUND | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
is ClientException -> HttpStatus.BAD_REQUEST | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
is ServerException, is CriticalException -> HttpStatus.INTERNAL_SERVER_ERROR | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+34
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 예외 타입을 그룹화하여 가독성을 높여보면 어떨까요? HTTP 상태 코드 매핑이 잘 되어 있네요! 비슷한 성격의 예외들을 그룹화하면 코드의 의도를 더 잘 파악할 수 있을 것 같아요. 🤓 다음과 같이 구조화해보는 건 어떨까요? internal fun DooripException.getHttpStatus(): HttpStatus =
when (this) {
+ // 인증/인가 관련 예외
is UnauthorizedException -> HttpStatus.FORBIDDEN
is UnauthenticatedException -> HttpStatus.UNAUTHORIZED
+ // 요청 관련 예외
MethodNotAllowedException -> HttpStatus.METHOD_NOT_ALLOWED
ConflictException -> HttpStatus.CONFLICT
-
NotFoundException -> HttpStatus.NOT_FOUND
-
is ClientException -> HttpStatus.BAD_REQUEST
+
+ // 서버 관련 예외
is ServerException, is CriticalException -> HttpStatus.INTERNAL_SERVER_ERROR
} 📝 Committable suggestion
Suggested change
|
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.
🧹 Nitpick (assertive)
예외 발생 메서드의 명칭 변경 고려
throwDooripException
메서드의 이름이 구체적이지 않아 다른 예외를 던질 경우 확장성이 떨어질 수 있습니다.메서드명을
testException
또는simulateException
등으로 변경하여 테스트 용도임을 명확히 해주세요.📝 Committable suggestion