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] slack 알림 구현 #206

Merged
merged 3 commits into from
Feb 13, 2024
Merged

[FEAT] slack 알림 구현 #206

merged 3 commits into from
Feb 13, 2024

Conversation

unanchoi
Copy link
Contributor

@unanchoi unanchoi commented Nov 9, 2023

`## 📝 Summary

500 error 시에 slack 알림이 가도록 구현했습니다.

👩‍💻 Contents

  • slack API dependency 추가 및 Token, Channel 설정 값 추가
  • SlackService 구현
  • ControllerAdvice에 500 Error시에 알림 보내도록 수정.

📝 Review Note

  • 현재 ExceptionHandler에서 응답을 보낼 때, ex.getCode(), ex.getMessage() 두 가지를 사용하고 있습니다.
    하지만 Custom Exception에서 생성자를 만들 때, message에 code를 넣기 때문에 둘은 똑같이 동작하고 있습니다.
@Getter
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class BaseException extends RuntimeException {
    HttpStatus statusCode;
    String code;

    public BaseException(HttpStatus statusCode, String code) {
        super(code);
        this.statusCode = statusCode;
        this.code = code;
    }
}

현재 구현 상황에서는 500 Error가 발생해도, slack에 클라이언트에게 응답가는 형태의 code와 동일하게 메세지가 전송되는 상황입니다.

클라이언트에 응답을 보낼 때는 Custom Code를 보내기 위해서, 이러한 형태로 구현된 것에 문제가 없지만, 서버에서는 500 Error에 대한 로깅을 위해서, client에 보내주는 error code와 구체적인 error message를 분리시키는 것이 좋을 것 같다고 생각이 들었습니다.

하지만 이게 custom exception을 handling 할 때는 가능한데, IllegalArgumentException 같은 exception을 handling 할 때, 어려워지는 것 같은데 관련해서 어떻게 생각하시는지 comment 남겨 주시면 감사하겠습니다.

📣 Related Issue

📬 Reference

Copy link
Member

@yeseul106 yeseul106 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 !!!

그리고 Review Note 확인했는데요. client에 보내주는 custom error code와 구체적인 error message를 분리시키는게 좋을 것 같다는 의견에 동의합니다 ! 그리고 IllegalArgumentException을 다루는 것이 어렵다는 부분에 대해 고민을 해보던 중 궁금한 게 생겨서 여쭤봐요 !

지금 저희가 개발한 코드 상으로는 IllegalArgumentException이 발생하는 대부분의 경우는 우리가 직접 메세지를 커스텀해서 에러를 직접 던지는게 아니라 자바 어플리케이션 자체에서 터지는 예외라고 생각이 되는데, 이런 경우에는 지금도 케쳡 커스텀 에러 코드를 내려보내고 있진 않고 있잖아요 ! 단지 ControllerExceptionAdvice 클래스의 handleIllegalArgument 메서드에서 실제 발생하는 IllegalArgumentException 내의 정의된 메세지를 내려주고 있는 것 같은데,,!

커스텀 에러 코드랑 구체적인 에러 메세지를 분리하는 걸 생각하기 전에 IllegalArgumentException을 케이스에 맞게 커스텀 에러 코드를 내려줄 수 있는 효율적인 방법이 있을까요 ? 케이스 분기처리를 try-catch 문으로 다 잡아서 커스텀 해서 IllegalArgumentException을 던져야 하나 궁금해져서 여쭤봅니다 !

ChatPostMessageResponse response = slack.methods(SLACK_TOKEN).chatPostMessage(req -> req
.channel(channel)
.text("[" + getProfiles() + "]" + text));
System.out.println(response);
Copy link
Member

Choose a reason for hiding this comment

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

p5;

확인을 위한 출력문이었을까요 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 그러네요 지우겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RestControllerAdvice가 있는 Exception Handling을 하는 클래스에서 따로 처리해도 좋을 것 같고, 만약 slack에 띄울 게 아니라면 logging이나 모니터링 tool 을 도입해도 좋을 것 같습니다!

Copy link
Member

@Seokyeong237 Seokyeong237 left a comment

Choose a reason for hiding this comment

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

저도 error code와 구체적인 error message를 분리시키는 것에 완전 동의합니다~!
예슬언니가 말씀해주신 것처럼 IllegalArgumentException를 케이스에 맞게 처리하는 로직이 따로 필요할 것 같아요!

@unanchoi unanchoi merged commit 5137678 into develop Feb 13, 2024
1 check passed
@unanchoi unanchoi deleted the feat#205/slack branch February 13, 2024 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] slack 알림 추가
3 participants