Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package kr.inventory.domain.chat.controller.support;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.time.OffsetDateTime;
import kr.inventory.domain.chat.constant.ChatConstants;
import kr.inventory.domain.chat.controller.dto.request.ChatSendMessageRequest;
import kr.inventory.domain.chat.controller.dto.response.ChatRealtimeEventResponse;
import kr.inventory.domain.chat.controller.dto.response.ChatRealtimeEventType;
import kr.inventory.domain.chat.entity.enums.ChatMessageStatus;
import kr.inventory.domain.chat.exception.ChatException;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.messaging.Message;
import org.springframework.messaging.handler.annotation.MessageExceptionHandler;
import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
import org.springframework.messaging.simp.annotation.SendToUser;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.validation.FieldError;
import org.springframework.web.bind.MethodArgumentNotValidException;

@Slf4j
@ControllerAdvice
@RequiredArgsConstructor
public class ChatWebSocketExceptionHandler {

private final ObjectMapper objectMapper;

@MessageExceptionHandler(Exception.class)
@SendToUser(destinations = ChatConstants.USER_QUEUE_DESTINATION, broadcast = false)
public ChatRealtimeEventResponse handleException(Exception exception, Message<?> message) {
ChatSendMessageRequest request = extractRequest(message);
String sessionId = SimpMessageHeaderAccessor.getSessionId(message.getHeaders());
String destination = SimpMessageHeaderAccessor.getDestination(message.getHeaders());

log.warn(
"Handled chat WebSocket exception. sessionId={}, destination={}, threadId={}, clientMessageId={}, reason={}",
sessionId,
destination,
request != null ? request.threadId() : null,
request != null ? request.clientMessageId() : null,
exception.getMessage(),
exception
);

return new ChatRealtimeEventResponse(
ChatRealtimeEventType.CHAT_FAILED,
request != null ? request.threadId() : null,
null,
request != null ? request.clientMessageId() : null,
ChatMessageStatus.FAILED,
null,
resolveErrorMessage(exception),
OffsetDateTime.now()
);
}

private ChatSendMessageRequest extractRequest(Message<?> message) {
if (message == null) {
return null;
}

Object payload = message.getPayload();
if (payload instanceof ChatSendMessageRequest request) {
return request;
}

try {
if (payload instanceof byte[] bytes) {
return objectMapper.readValue(bytes, ChatSendMessageRequest.class);
}

if (payload instanceof String text) {
return objectMapper.readValue(text, ChatSendMessageRequest.class);
}
} catch (Exception ignored) {
return null;
}
Comment on lines +75 to +77

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

catch (Exception ignored) 블록에서 예외를 무시하고 있습니다. 레포지토리 스타일 가이드 53번 라인("실패를 숨기지 않는다: 의미 없는 catch 후 무시 금지")에 따라, 예외를 무시하는 것은 잠재적인 버그를 찾기 어렵게 만들 수 있습니다. 페이로드 역직렬화 실패는 정상적인 흐름에서 발생할 수 있으므로, 디버깅을 위해 최소한 debug 레벨로 로그를 남기는 것이 좋습니다.

Suggested change
} catch (Exception ignored) {
return null;
}
} catch (Exception e) {
log.debug("Failed to deserialize WebSocket message payload.", e);
return null;
}
References
  1. 의미 없는 catch 후 예외를 무시하는 것을 금지하고 있습니다. 현재 코드에서 예외를 무시하고 있어 잠재적인 문제를 파악하기 어렵습니다. (link)


return null;
}

private String resolveErrorMessage(Exception exception) {
if (exception instanceof ChatException chatException) {
return chatException.getMessage();
}

if (exception instanceof MethodArgumentNotValidException validationException) {
FieldError fieldError = validationException.getBindingResult().getFieldError();
if (fieldError != null && fieldError.getDefaultMessage() != null) {
return fieldError.getDefaultMessage();
}
}

String message = exception.getMessage();
if (message == null || message.isBlank()) {
return "채팅 요청을 처리하지 못했습니다.";
}

return message;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,25 @@ public Message<?> preSend(Message<?> message, MessageChannel channel) {
log.debug("[StompInterceptor] Received STOMP message - command: {}, destination: {}, sessionId: {}",
accessor.getCommand(), accessor.getDestination(), accessor.getSessionId());

if (!requiresAuthentication(accessor.getCommand()) || accessor.getUser() != null) {
log.debug("[StompInterceptor] Skipping auth - command: {}, requiresAuth: {}, hasUser: {}",
accessor.getCommand(), requiresAuthentication(accessor.getCommand()), accessor.getUser() != null);
if (!requiresAuthentication(accessor.getCommand())) {
return message;
}

if (accessor.getUser() != null) {
return message;
}

Authentication sessionAuthentication = resolveSessionAuthentication(accessor);
if (sessionAuthentication != null) {
accessor.setUser(sessionAuthentication);
return message;
}

String accessToken = resolveToken(accessor);
if (!StringUtils.hasText(accessToken)) {
log.warn("[StompInterceptor] No access token found - command: {}, sessionId: {}",
log.warn("[StompInterceptor] Rejecting STOMP frame without access token - command: {}, sessionId: {}",
accessor.getCommand(), accessor.getSessionId());
return message;
return null;
}

if (!jwtProvider.validateToken(accessToken)) {
Expand Down Expand Up @@ -74,6 +82,7 @@ public Message<?> preSend(Message<?> message, MessageChannel channel) {

Authentication authentication = jwtProvider.getAuthentication(accessToken);
accessor.setUser(authentication);
storeSessionAuthentication(accessor, authentication);
return message;
}

Expand All @@ -83,6 +92,29 @@ private boolean requiresAuthentication(StompCommand command) {
|| StompCommand.SUBSCRIBE.equals(command);
}

private Authentication resolveSessionAuthentication(StompHeaderAccessor accessor) {
Map<String, Object> sessionAttributes = accessor.getSessionAttributes();
if (sessionAttributes == null || sessionAttributes.isEmpty()) {
return null;
}

Object value = sessionAttributes.get(WebSocketConstants.SESSION_AUTHENTICATION);
if (value instanceof Authentication authentication) {
return authentication;
}

return null;
}

private void storeSessionAuthentication(StompHeaderAccessor accessor, Authentication authentication) {
Map<String, Object> sessionAttributes = accessor.getSessionAttributes();
if (sessionAttributes == null) {
return;
}

sessionAttributes.put(WebSocketConstants.SESSION_AUTHENTICATION, authentication);
}

private String resolveToken(StompHeaderAccessor accessor) {
String authorization = firstNativeHeader(
accessor,
Expand Down Expand Up @@ -134,4 +166,4 @@ private String stripBearerPrefix(String rawValue) {
}
return value;
}
}
}
45 changes: 42 additions & 3 deletions src/main/java/kr/inventory/global/config/WebSocketConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@

import kr.inventory.global.auth.interceptor.JwtHandshakeInterceptor;
import kr.inventory.global.auth.interceptor.StompAuthChannelInterceptor;
import kr.inventory.global.constant.WebSocketConstants;
import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.messaging.simp.config.ChannelRegistration;
import org.springframework.messaging.simp.config.MessageBrokerRegistry;
import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
import org.springframework.web.socket.config.annotation.EnableWebSocketMessageBroker;
import org.springframework.web.socket.config.annotation.StompEndpointRegistry;
import org.springframework.web.socket.config.annotation.WebSocketMessageBrokerConfigurer;
import org.springframework.web.socket.config.annotation.WebSocketTransportRegistration;

@Configuration
@EnableWebSocketMessageBroker
Expand All @@ -17,28 +21,63 @@ public class WebSocketConfig implements WebSocketMessageBrokerConfigurer {

private final JwtHandshakeInterceptor jwtHandshakeInterceptor;
private final StompAuthChannelInterceptor stompAuthChannelInterceptor;
private final CorsProperties corsProperties;

@Override
public void registerStompEndpoints(StompEndpointRegistry registry) {
String[] allowedOriginPatterns = corsProperties.getAllowedOrigins().toArray(String[]::new);

registry.addEndpoint("/ws")
.addInterceptors(jwtHandshakeInterceptor)
.setAllowedOriginPatterns("*");
.setAllowedOriginPatterns(allowedOriginPatterns);

registry.addEndpoint("/ws")
.addInterceptors(jwtHandshakeInterceptor)
.setAllowedOriginPatterns("*")
.setAllowedOriginPatterns(allowedOriginPatterns)
.withSockJS();
}

@Override
public void configureClientInboundChannel(ChannelRegistration registration) {
registration.interceptors(stompAuthChannelInterceptor);
registration.taskExecutor()
.corePoolSize(WebSocketConstants.INBOUND_CORE_POOL_SIZE)
.maxPoolSize(WebSocketConstants.INBOUND_MAX_POOL_SIZE)
.queueCapacity(WebSocketConstants.CHANNEL_QUEUE_CAPACITY);
}

@Override
public void configureClientOutboundChannel(ChannelRegistration registration) {
registration.taskExecutor()
.corePoolSize(WebSocketConstants.OUTBOUND_CORE_POOL_SIZE)
.maxPoolSize(WebSocketConstants.OUTBOUND_MAX_POOL_SIZE)
.queueCapacity(WebSocketConstants.CHANNEL_QUEUE_CAPACITY);
}

@Override
public void configureMessageBroker(MessageBrokerRegistry registry) {
registry.enableSimpleBroker("/topic", "/queue");
registry.enableSimpleBroker("/topic", "/queue")
.setHeartbeatValue(WebSocketConstants.SIMPLE_BROKER_HEARTBEAT)
.setTaskScheduler(webSocketBrokerTaskScheduler());
registry.setApplicationDestinationPrefixes("/app");
registry.setUserDestinationPrefix("/user");
}

@Override
public void configureWebSocketTransport(WebSocketTransportRegistration registry) {
registry.setMessageSizeLimit(WebSocketConstants.MESSAGE_SIZE_LIMIT)
.setSendBufferSizeLimit(WebSocketConstants.SEND_BUFFER_SIZE_LIMIT)
.setSendTimeLimit(WebSocketConstants.SEND_TIME_LIMIT_MS)
.setTimeToFirstMessage(WebSocketConstants.TIME_TO_FIRST_MESSAGE_MS);
}

@Bean
public ThreadPoolTaskScheduler webSocketBrokerTaskScheduler() {
ThreadPoolTaskScheduler scheduler = new ThreadPoolTaskScheduler();
scheduler.setPoolSize(2);
scheduler.setThreadNamePrefix("ws-broker-heartbeat-");
Comment on lines +77 to +78

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

webSocketBrokerTaskScheduler 빈 설정에서 2"ws-broker-heartbeat-"와 같은 매직 넘버와 매직 스트링이 사용되었습니다. 스타일 가이드 26, 80, 139번 라인에 따라, 이러한 값들은 WebSocketConstants 클래스에 상수로 정의하여 관리하는 것이 가독성과 유지보수성 측면에서 더 좋습니다.

참고: WebSocketConstantsBROKER_HEARTBEAT_POOL_SIZEBROKER_HEARTBEAT_THREAD_PREFIX 상수를 추가해야 합니다.

Suggested change
scheduler.setPoolSize(2);
scheduler.setThreadNamePrefix("ws-broker-heartbeat-");
scheduler.setPoolSize(WebSocketConstants.BROKER_HEARTBEAT_POOL_SIZE);
scheduler.setThreadNamePrefix(WebSocketConstants.BROKER_HEARTBEAT_THREAD_PREFIX);
References
  1. 매직넘버/문자열은 상수화하고 의미 있는 이름을 부여해야 합니다. 현재 코드에 하드코딩된 값이 있어 이를 상수로 추출하는 것이 좋습니다. (link)

scheduler.setRemoveOnCancelPolicy(true);
scheduler.initialize();
return scheduler;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

public final class WebSocketConstants {

private WebSocketConstants() {
}

// Authorization 헤더
public static final String AUTHORIZATION_HEADER = "Authorization";
public static final String AUTHORIZATION_HEADER_LOWER_CASE = "authorization";
Expand All @@ -12,6 +15,7 @@ public final class WebSocketConstants {
// WebSocket 세션 속성
public static final String SESSION_AUTHORIZATION = "WS_AUTHORIZATION";
public static final String SESSION_ACCESS_TOKEN = "WS_ACCESS_TOKEN";
public static final String SESSION_AUTHENTICATION = "WS_AUTHENTICATION";

// Query 파라미터
public static final String TOKEN_PARAM = "token";
Expand All @@ -35,4 +39,17 @@ public final class WebSocketConstants {
// 에러 메시지
public static final String ERROR_NO_USER_INFO = "인증된 사용자 정보를 찾을 수 없습니다.";
public static final String ERROR_INVALID_USER_ID = "사용자 식별자를 해석할 수 없습니다.";
}

// WebSocket 설정 상수
public static final long[] SIMPLE_BROKER_HEARTBEAT = {10000L, 10000L};
public static final int INBOUND_CORE_POOL_SIZE = 4;
public static final int INBOUND_MAX_POOL_SIZE = 16;
public static final int OUTBOUND_CORE_POOL_SIZE = 4;
public static final int OUTBOUND_MAX_POOL_SIZE = 16;
public static final int CHANNEL_QUEUE_CAPACITY = 1000;
public static final int MESSAGE_SIZE_LIMIT = 64 * 1024;
public static final int SEND_BUFFER_SIZE_LIMIT = 512 * 1024;
public static final int SEND_TIME_LIMIT_MS = 15_000;
public static final int TIME_TO_FIRST_MESSAGE_MS = 60_000;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package kr.inventory.global.websocket;

import lombok.extern.slf4j.Slf4j;
import org.springframework.context.event.EventListener;
import org.springframework.messaging.simp.stomp.StompHeaderAccessor;
import org.springframework.stereotype.Component;
import org.springframework.web.socket.messaging.SessionConnectEvent;
import org.springframework.web.socket.messaging.SessionConnectedEvent;
import org.springframework.web.socket.messaging.SessionDisconnectEvent;
import org.springframework.web.socket.messaging.SessionSubscribeEvent;

@Slf4j
@Component
public class WebSocketSessionEventListener {

@EventListener
public void onSessionConnect(SessionConnectEvent event) {
StompHeaderAccessor accessor = StompHeaderAccessor.wrap(event.getMessage());
log.info("[WebSocket] CONNECT sessionId={}, user={}, destination={}",
accessor.getSessionId(),
accessor.getUser() != null ? accessor.getUser().getName() : null,
accessor.getDestination());
}

@EventListener
public void onSessionConnected(SessionConnectedEvent event) {
StompHeaderAccessor accessor = StompHeaderAccessor.wrap(event.getMessage());
log.info("[WebSocket] CONNECTED sessionId={}, user={}",
accessor.getSessionId(),
accessor.getUser() != null ? accessor.getUser().getName() : null);
}

@EventListener
public void onSessionSubscribe(SessionSubscribeEvent event) {
StompHeaderAccessor accessor = StompHeaderAccessor.wrap(event.getMessage());
log.info("[WebSocket] SUBSCRIBE sessionId={}, user={}, destination={}",
accessor.getSessionId(),
accessor.getUser() != null ? accessor.getUser().getName() : null,
accessor.getDestination());
}

@EventListener
public void onSessionDisconnect(SessionDisconnectEvent event) {
StompHeaderAccessor accessor = StompHeaderAccessor.wrap(event.getMessage());
log.info("[WebSocket] DISCONNECT sessionId={}, user={}, closeStatus={}",
accessor.getSessionId(),
accessor.getUser() != null ? accessor.getUser().getName() : null,
event.getCloseStatus());
}
}
Comment on lines +14 to +50

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

onSessionConnect, onSessionConnected, onSessionSubscribe, onSessionDisconnect 각 메소드에서 사용자 이름을 가져오는 로직 (accessor.getUser() != null ? accessor.getUser().getName() : null)이 중복되고 있습니다. 스타일 가이드 13번 라인(DRY 원칙)에 따라, 이 로직을 별도의 private 헬퍼 메소드로 추출하여 중복을 제거하고 코드의 가독성을 높일 수 있습니다.

아래와 같이 헬퍼 메소드를 추가하고 각 이벤트 리스너에서 호출하는 것을 제안합니다.

private String getUserName(StompHeaderAccessor accessor) {
    return java.util.Optional.ofNullable(accessor.getUser())
                             .map(java.security.Principal::getName)
                             .orElse(null);
}

적용 예시:

@EventListener
public void onSessionConnect(SessionConnectEvent event) {
    StompHeaderAccessor accessor = StompHeaderAccessor.wrap(event.getMessage());
    log.info("[WebSocket] CONNECT sessionId={}, user={}, destination={}",
            accessor.getSessionId(),
            getUserName(accessor),
            accessor.getDestination());
}
References
  1. 반복되는 로직은 중복을 제거해야 합니다. 현재 여러 메소드에 걸쳐 사용자 이름을 가져오는 로직이 반복되고 있어 이를 공통화할 필요가 있습니다. (link)

Loading