Skip to content
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

[Feat] 전역적 예외 및 핸들러 추가 #4

Merged
merged 10 commits into from
Feb 8, 2025
11 changes: 11 additions & 0 deletions src/main/java/com/evenly/blok/global/exception/BlokException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.evenly.blok.global.exception;

import lombok.AllArgsConstructor;
import lombok.Getter;

@Getter
@AllArgsConstructor
public class BlokException extends RuntimeException { // TODO: 서비스명에 맞게 변경

private final ErrorCode errorCode;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.evenly.blok.global.exception;

import lombok.Getter;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;

@Getter
@RequiredArgsConstructor
public enum CommonErrorCode implements ErrorCode {

INTERNAL_SERVER_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "서버 관리자에게 문의하세요."),
INVALID_REQUEST_VALUE(HttpStatus.BAD_REQUEST, "유효하지 않은 요청값입니다."),
INVALID_REQUEST_URL(HttpStatus.BAD_REQUEST, "유효하지 않은 경로입니다."),
;

private final HttpStatus status;
private final String message;
}
10 changes: 10 additions & 0 deletions src/main/java/com/evenly/blok/global/exception/ErrorCode.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.evenly.blok.global.exception;

import org.springframework.http.HttpStatus;

public interface ErrorCode {

HttpStatus getStatus();

String getMessage();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.evenly.blok.global.exception;

import com.evenly.blok.global.exception.dto.ErrorResponse;
import com.evenly.blok.global.exception.dto.ServerErrorResponse;
import com.evenly.blok.global.exception.dto.ValidationErrorResponse;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.servlet.resource.NoResourceFoundException;

@RestControllerAdvice
public class GlobalExceptionHandler {
Copy link
Member

Choose a reason for hiding this comment

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

저는 전 프로젝트에서 @ExceptionHandler(EntityNotFoundException.class) 이런식으로 각 Exception에 대한 예외 Handling을 따로 적용했던거 같은데 어떻게 생각하시는지 궁금합니다!

또한, 중요한 예외의 경우 log도 추가했었는데 저희 컨벤션에 warn 부터는 logging이 되어야한다는 부분이 있어서 이 부분도 고려해봐주시면 좋을거 같아요!
그래서 같은 맥락으로 exception의 이름도 추가하고, timestamp도 추가하면 나중에 logging 시 좋을거 같아서 의견 남겨요!

아래는 제가 생각하는 예시입니다.!!

	@ExceptionHandler(EntityNotFoundException.class)
	public ResponseEntity<ErrorResponse> handleEntityNotFoundException(EntityNotFoundException e) {
		String timestamp = getCurrentTimestamp();
		log.error(LOG_FORMAT, timestamp, e.getClass().getSimpleName(), e.getMessage());
		return ResponseEntity.status(NOT_FOUND)
			.body(ErrorResponse.of(e.getClass().getSimpleName(), NOT_FOUND.value(), e.getMessage()));
	}

Copy link
Collaborator Author

@helenason helenason Feb 2, 2025

Choose a reason for hiding this comment

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

1. @ExceptionHandler 파라미터 선언 관련

@ExceptionHandler의 파라미터가 존재하지 않아도 선언한 메서드의 파라미터를 통해 예외 타입을 지정할 수 있는데요! (스프링에서도 메서드 시그니처 파라미터를 통해서만 예외 타입을 지정하여 실수를 방지할 것을 추천하기도 합니다) 저도 마찬가지로 중복 코드를 선호하지 않아서, 파라미터만으로 예외 타입을 선언하도록 하였습니다 :) 이 내용을 기반으로 다시 한번 경호님의 의견을 여쭙습니다!

2. 로그 기준 및 로그에 담을 정보 관련

로그의 경우 컨벤션을 추후 확정하는 것으로 알고 있었어서 기존에는 추가하지 않았었는데요, 로그 관련하여서도 다양한 이야기를 해볼 수 있을 것 같아요! (추후 회의를 통해 명확한 기준을 정하는 것도 좋을 것 같습니다)

  1. 개인적으로 예측가능한 예외(ex. 400, 401, 404 등)는 warn보다는 info에 가깝다고 생각해요! 저희가 '대처'해야할 예외가 아닌 클라이언트에게 정보 전달을 위해 직접 '의도'한 예외니까요. 그래서 추후 filter나 interceptor를 통해 로그를 작성하면 되겠다고 생각하고 있었는데요. 이러한 기준과 방법에 대해 어떻게 생각하시는지 궁금합니다!
  2. timestamp의 경우는 로그가 발생할 때 아래와 같이 가장 상단에 기본으로 제공되어서 로그에 timestamp를 추가하는 것은 중복이라고 판단하여 제외시켰습니다! 이에 대한 의견도 궁금해요오
2024-10-10 20:35:23 [http-nio-8080-exec-4] INFO  c.z.c.l.config.LoggingInterceptor - LoggingInfoSuccessResponse[identifier=e472b64f-5b04-4ce4-ab08-0c96c43fdd45, httpMethod=GET, uri=/offerings, requestBody=, statusCode=200, latency=238ms]

3. 에러 응답 관련

저는 고민 끝에 아래와 같은 형태로 모든 비즈니스 예외에 대해 BlokException(커스텀 예외)를 반환하도록 하였는데요.

@ExceptionHandler
public ResponseEntity<ErrorResponse> handle(BlokException ex) {
    ErrorCode errorCode = ex.getErrorCode();
    return ResponseEntity
            .status(errorCode.getStatus())
            .body(errorCode.getResponse());
}

경호님이 위에서 제시해주신 예시를 기반으로 저의 의견을 소개드리고 싶어요!

  1. response의 body에 status를 포함하는 것은 불필요하다고 생각하였습니다. response의 status를 통해 충분히 제공되는 정보이기 때문에 body에 status를 추가로 포함하는 것은 중복이라고 생각해요!
  2. 저는 애플리케이션 단에서 예외를 한가지의 커스텀 예외(BlokException)로 발생시켰기 때문에 response의 body에 exception의 이름을 담는 것이 무의미하다고 판단하였어요! 모든 예외의 이름이 'BlokException'이기 때문에요. 또한, 클라이언트는 서버에서 발생한 예외의 이름을 알 필요가 없다고 생각하기도 하였습니다. 클라이언트에게는 이해하기 쉽게 저희가 예외를 가공하여 전달하는 게 좋은 것 같아요!

제가 고민했던 부분들을 경호님이 쏙쏙 짚어주셔서 적다보니 내용이 길어졌는데요 하하 (다음엔 PR description에 고민한 내용을 최대한 적어보겠습니닷..)

불필요한 중복을 최대한 쳐내기 위해 고민 끝에 내린 결론은, 예외를 반환할 때 헤더에 status를 포함하고, 바디에 에러메시지를 포함하자는 결론이었습니다. 하지만 반대로 중복된 정보는 오히려 정보를 얻기 수월하다는 장점이 있을 수 있겠어요. 따라서 애플리케이션 단에서 예외를 발생시킬 방법과, 클라이언트에게 전달할 예외 응답과 관련하여 저희 셋이 논의할 필요가 있어 보여요. 그에 따라 response의 형태도 달라질 것 같아요. 여러분들의 전반적인 의견이 궁금합니다!

Copy link
Member

Choose a reason for hiding this comment

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

너무나 친절한 설명 감사드립니다!!
ExceptionHandler, log timestamp, status 모두 결국엔 중복에 대한 고민 결과라고 생각이 들어서 함께 의견 남겨요!
결론적으로는 채영님 의견을 들어보니 충분히 공감되는 부분이라서 그대로 진행해도 좋을거 같다고 생각했어요!

제가 저 exception을 명시하거나 timestamp, status를 과거에 왜 찍었는지 생각했을 때도 결국엔 좀더 관리 측면에서 직관적이라 편하다는 니즈가 있었던거 같아요.
그래서 이 부분은 저희가 진행하다가 필요하면 다시 고민해볼 문제가 맞고 지금은 불필요한 부분 같다는게 채영님의 의견을 듣고 난 후의 생각이었습니다!! 너무나 친절한 설명 조와요~~

Copy link
Member

Choose a reason for hiding this comment

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

두분께서 충분한 고민을 거쳐 의견을 정리해주셔서 너무 감사드립니다!!

추후 논의를 위한 고민해볼만한 몇 가지 포인트들 한 번 정리해봤어요.

  • 에러를 로깅하는 과정에서 중복된 데이터를 제거 및 꼭 필요한 데이터만 첨부

    • 제거했을 때, 응답 사이즈의 차이가 서비스에 어느정도의 영향을 주는지
    • 프론트에서 API 연동 작업을 진행할 때 불편함이 발생하지는 않을지
    • 에러 로깅의 목적 ⇒ 가볍고 중복을 방지하기는 것 보단 최대한 상세한 정보를 담고, 백/프론트 양쪽에서 이슈 트래킹을 원할하도록
  • Response Status 에만 상태값을 의지

    • 일부 프론트엔드 프레임워크에서 응답 처리 중 HTTP 상태가 손실되는 케이스 존재
  • 공통 에러 클래스를 완벽하게 통일

// 기본 에러 응답
{
  "message": "잘못된 요청 파라미터",
  "timestamp": "2025-02-03T10:30:45Z"
}

// 필드 유효성 검사 상세 에러
{
  "message": "유효성 검사 실패",
  "timestamp": "2025-02-03T10:30:45Z",
  "errors": [
    {
      "field": "email",
      "message": "유효한 이메일 주소여야 합니다",
      "value": "invalid.email"
    },
    {
      "field": "age",
      "message": "0보다 큰 값이어야 합니다",
      "value": "-5"
    }
  ]
}

// 서버 에러 (개발 환경용 디버그 정보 포함)
{
  "message": "내부 서버 오류",
  "timestamp": "2025-02-03T10:30:45Z",
  "error": {
    "code": "SERVER_001",
    "details": "데이터베이스 연결 실패",
    "debug": {
      "exception": "SQLException",
      "stackTrace": "...",
      "correlationId": "cor_abc123"
    }
  }
}

// 요청 제한 초과 에러
{
  "message": "너무 많은 요청",
  "timestamp": "2025-02-03T10:30:45Z",
  "error": {
    "code": "RATE_LIMIT_001",
    "details": "요청 제한 초과",
    "retryAfter": 60,
    "limit": 100,
    "current": 101
  }
}
  • 이렇듯 Exception 케이스별로 꼭 담고있어야할 정보가 상이하는 경우가 많고, 동일한 에러 내에서도 다른 필드값을 보고 프론트에서의 에러화면이 달라질 수 있어, 이러한 여러 케이스들을 어떻게 처리하면 좋을지 함께 고민해보면 좋을 것 같습니다!

Copy link
Collaborator Author

@helenason helenason Feb 6, 2025

Choose a reason for hiding this comment

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

오 좋은 의견 정말 감사해요!

응답이 많아지면 복잡도가 높아져 클라이언트 입장에서 오히려 읽기 불편할 것이라 생각했다가, 이전에 경호님 코멘트를 받고 변경에 아주 민감한 API응답은 DB 설계할 때와 같이 최대한 변경되지 않도록 세세한 정보를 담는 방향이 더 적절할 수 있다는 생각을 매일같이 하고 있었는데요 ㅋㅋㅋㅋ 찬기님이 주신 다른 근거들을 접하니 더더욱 그렇게 생각하게 되었습니다!! 특히 아래 부분이 인상적이었어요.

에러 로깅의 목적 ⇒ 가볍고 중복을 방지하기는 것 보단 최대한 상세한 정보를 담고, 백/프론트 양쪽에서 이슈 트래킹을 원할하도록

일부 프론트엔드 프레임워크에서 응답 처리 중 HTTP 상태가 손실되는 케이스 존재

이렇게 세세한 정보를 담도록 한다면 에러 응답뿐 아니라 공통 응답 클래스도 더더욱 필요해지겠네요. 첨부해주신 예시 참고해서 코드 수정해보겠습니다 👊


@ExceptionHandler
public ErrorResponse handle(BlokException ex) {
ErrorCode errorCode = ex.getErrorCode();
return ErrorResponse.of(errorCode);
}

@ExceptionHandler
public ErrorResponse handle(NoResourceFoundException ex) {
ErrorCode errorCode = CommonErrorCode.INVALID_REQUEST_URL;
return ErrorResponse.of(errorCode);
}

@ExceptionHandler
public ErrorResponse handle(MethodArgumentNotValidException ex) {
ErrorCode errorCode = CommonErrorCode.INVALID_REQUEST_VALUE;
return ValidationErrorResponse.of(errorCode, ex);
}

@ExceptionHandler
public ErrorResponse handle(Exception ex) {
// TODO: 로그 추가
ex.printStackTrace();
ErrorCode errorCode = CommonErrorCode.INTERNAL_SERVER_ERROR;
return ServerErrorResponse.of(errorCode, ex);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.evenly.blok.global.exception.dto;

import com.evenly.blok.global.exception.ErrorCode;
import java.time.LocalDateTime;
import lombok.Getter;
import org.springframework.http.HttpStatus;

@Getter
public class ErrorResponse {

private final HttpStatus status;
private final String message;
private final LocalDateTime timestamp;

protected ErrorResponse(ErrorCode errorCode) {
this.status = errorCode.getStatus();
this.message = errorCode.getMessage();
this.timestamp = LocalDateTime.now();
}

public static ErrorResponse of(ErrorCode errorCode) {
return new ErrorResponse(errorCode);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.evenly.blok.global.exception.dto;

import com.evenly.blok.global.exception.ErrorCode;
import lombok.Getter;

@Getter
public class ServerErrorResponse extends ErrorResponse {

private final ErrorDetail error;

private ServerErrorResponse(ErrorCode errorCode, ErrorDetail error) {
super(errorCode);
this.error = error;
}

public static ServerErrorResponse of(ErrorCode errorCode, Exception ex) {
ErrorDetail errorDetail = new ErrorDetail(ex);
return new ServerErrorResponse(errorCode, errorDetail);
}

private record ErrorDetail(String exception,
String message,
String stackTrace) {

private ErrorDetail(Exception ex) {
this(ex.getClass().getSimpleName(), ex.getMessage(), ex.getStackTrace()[0].getClassName());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.evenly.blok.global.exception.dto;

import com.evenly.blok.global.exception.ErrorCode;
import java.util.List;
import lombok.Getter;
import org.springframework.validation.FieldError;
import org.springframework.web.bind.MethodArgumentNotValidException;

@Getter
public class ValidationErrorResponse extends ErrorResponse {

private final List<FieldErrorDetail> errors;

private ValidationErrorResponse(ErrorCode errorCode, List<FieldErrorDetail> errors) {
super(errorCode);
this.errors = errors;
}

public static ValidationErrorResponse of(ErrorCode errorCode, MethodArgumentNotValidException ex) {
List<FieldError> fieldErrors = ex.getBindingResult().getFieldErrors();
List<FieldErrorDetail> fieldErrorDetails = fieldErrors.stream()
.map(FieldErrorDetail::new)
.toList();
return new ValidationErrorResponse(errorCode, fieldErrorDetails);
}

private record FieldErrorDetail(String field,
String message,
Object value) {

private FieldErrorDetail(FieldError fieldError) {
this(fieldError.getField(), fieldError.getDefaultMessage(), fieldError.getRejectedValue());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.evenly.blok.global.response;

import com.evenly.blok.global.exception.dto.ErrorResponse;
import org.springframework.core.MethodParameter;
import org.springframework.http.MediaType;
import org.springframework.http.converter.HttpMessageConverter;
import org.springframework.http.server.ServerHttpRequest;
import org.springframework.http.server.ServerHttpResponse;
import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.servlet.mvc.method.annotation.ResponseBodyAdvice;

@RestControllerAdvice
public class GlobalResponseHandler implements ResponseBodyAdvice<Object> {

@Override
public boolean supports(MethodParameter returnType,
Class<? extends HttpMessageConverter<?>> converterType) {
return true;
}

@Override
public Object beforeBodyWrite(Object body,
MethodParameter returnType,
MediaType selectedContentType,
Class<? extends HttpMessageConverter<?>> selectedConverterType,
ServerHttpRequest request,
ServerHttpResponse response) {

if (body instanceof ErrorResponse errorResponse) {
response.setStatusCode(errorResponse.getStatus());
}
if (body instanceof SuccessResponse successResponse) {
response.setStatusCode(successResponse.getStatus());
}
return body;
}
}
28 changes: 28 additions & 0 deletions src/main/java/com/evenly/blok/global/response/SuccessResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.evenly.blok.global.response;

import java.time.LocalDateTime;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;

@Getter
@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
public class SuccessResponse {

private static final HttpStatus DEFAULT_HTTP_STATUS = HttpStatus.OK;
private static final String DEFAULT_MESSAGE = "요청이 성공적으로 처리되었습니다.";

private final HttpStatus status;
private final String message;
private final LocalDateTime timestamp;
private final Object data;

public static SuccessResponse of(Object data) {
return new SuccessResponse(DEFAULT_HTTP_STATUS, DEFAULT_MESSAGE, LocalDateTime.now(), data);
}

public static SuccessResponse of(HttpStatus status, Object data) {
return new SuccessResponse(status, DEFAULT_MESSAGE, LocalDateTime.now(), data);
}
}