✅ [Deploy] v0.5.1 배포#144
Conversation
INCR과 EXPIRE가 별도 왕복이라 중간 실패 시 TTL 없는 카운터가 남을 수 있었다. 두 명령을 단일 스크립트로 묶어 한 번의 왕복으로 실행한다. MULTI/EXEC는 INCR 반환값을 읽어 EXPIRE 여부를 분기할 수 없어 사용하지 않았다. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sendSms가 failKey를 지우지 않아 실패 카운터가 코드 수명을 넘어 이월됐다. 카운터 TTL은 첫 실패 기준, 코드 TTL은 발급 기준이라 두 창이 정렬되지 않아 4회 실패 후 새 코드를 받으면 오답 한 번에 즉시 무효화됐다. 새 코드에 새 시도 예산을 부여한다. 쿨다운(1분)이 재발급 남용을 막는다. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
♻️ refactor: SMS 인증 무차별 대입 방어 추가
♻️ refactor: LSTM 로그 제거
- SmsSendResponse.expiredAt 주석 오류 정정 (3분 -> 5분) - 데드코드 generateSmsToken 제거 (호출처 없음, 인증 우회 소지) - resetPassword 트랜잭션 애노테이션 스프링으로 통일 - resetPassword에서 sms_required 플래그 삭제로 잠금 해제 일관성 확보 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
♻️ refactor: Auth 도메인 디테일/일관성 보완 (#142)
📝 WalkthroughWalkthroughSMS 인증에 실패 시도 제한과 Redis TTL 카운터가 추가되었습니다. 웹소켓은 연결 이벤트, 재시도, 복구, 수신 지연 감시를 지원합니다. Docker Compose 명령과 서비스 도메인 설정도 변경되었습니다. ChangesSMS 인증 및 Redis 카운터
웹소켓 연결 생명주기
배포 및 도메인 설정
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WebsocketConnect
participant UpbitTickerHandler
participant UpbitWebSocket
WebsocketConnect->>UpbitWebSocket: 연결 실행 및 결과 대기
UpbitWebSocket-->>UpbitTickerHandler: 연결 또는 메시지 전달
UpbitTickerHandler-->>WebsocketConnect: 연결 상태 이벤트 발행
WebsocketConnect->>UpbitTickerHandler: 무수신 세션 종료
WebsocketConnect->>UpbitWebSocket: 재시도 또는 복구 연결
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/com/example/scoi/global/config/SecurityConfig.java (1)
74-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value실제 도메인으로 교체 완료 → TODO 주석 정리 필요.
프로덕션 도메인(
https://scoi.store)으로 이미 교체되었는데 "배포 시 실제 프론트엔드 도메인으로 변경 필요"라는 TODO 주석이 그대로 남아있습니다. 혼란을 줄 수 있으므로 제거하는 것을 권장합니다.♻️ 제안 diff
- // TODO: 배포 시 실제 프론트엔드 도메인으로 변경 필요 - // 개발 환경: localhost 허용 - // 프로덕션 환경: 실제 도메인으로 제한 + // 개발 환경: localhost 허용 + // 프로덕션 환경: https://scoi.store 로 제한🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/scoi/global/config/SecurityConfig.java` around lines 74 - 81, Remove the obsolete deployment TODO comment above the allowed origins configuration in SecurityConfig, while preserving the environment context comments and the existing allowed origin values.src/main/java/com/example/scoi/domain/websocket/WebsocketConnect.java (1)
37-39: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win워치독과 2시간 복구 재시도가 단일 스레드 스케줄러를 공유합니다.
scheduler는newSingleThreadScheduledExecutor()로, 10초 주기 워치독 검사(onConnected)와recover()의 2시간 지연 재연결 작업이 동일한 단일 스레드에서 실행됩니다.closeSession()의session.close()가 지연되면(구현에 따라 블로킹 가능) 이후 워치독 실행과 예약된 복구 작업까지 함께 지연될 수 있습니다.♻️ 제안: 스레드 풀 확장
- private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); + private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(2);Also applies to: 76-100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/scoi/domain/websocket/WebsocketConnect.java` around lines 37 - 39, Update the scheduler used by WebsocketConnect’s watchdog and recover retry tasks so they no longer share a single executor thread; configure a scheduled thread pool with enough independent threads for the 10-second onConnected watchdog checks and the delayed recover() reconnect work to proceed when session.close() blocks, while preserving the existing scheduling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/example/scoi/domain/websocket/handler/UpbitTickerHandler.java`:
- Around line 69-80: Prevent duplicate DisconnectedEvent publication in
UpbitTickerHandler by making handleTransportError only log the transport failure
and leaving event publication to afterConnectionClosed. Preserve the existing
disconnection event behavior for normal closures and ensure each connection
termination triggers one event.
In `@src/main/java/com/example/scoi/domain/websocket/WebsocketConnect.java`:
- Around line 47-63: Update the connection-success handling associated with
connectWithRetry and upbitTickerHandler so a newly established websocket cannot
coexist with the existing currentSession. When afterConnectionEstablished()
receives a new session, close and clear the previous currentSession before
replacing it, or reject the new session when one is already active; preserve a
single active session for /topic/ticker delivery.
---
Nitpick comments:
In `@src/main/java/com/example/scoi/domain/websocket/WebsocketConnect.java`:
- Around line 37-39: Update the scheduler used by WebsocketConnect’s watchdog
and recover retry tasks so they no longer share a single executor thread;
configure a scheduled thread pool with enough independent threads for the
10-second onConnected watchdog checks and the delayed recover() reconnect work
to proceed when session.close() blocks, while preserving the existing scheduling
behavior.
In `@src/main/java/com/example/scoi/global/config/SecurityConfig.java`:
- Around line 74-81: Remove the obsolete deployment TODO comment above the
allowed origins configuration in SecurityConfig, while preserving the
environment context comments and the existing allowed origin values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fff14438-9cc5-4bdb-bfbb-c6adb3e6d9ce
📒 Files selected for processing (16)
.github/workflows/cd.ymlsrc/main/java/com/example/scoi/domain/auth/dto/AuthResDTO.javasrc/main/java/com/example/scoi/domain/auth/exception/code/AuthErrorCode.javasrc/main/java/com/example/scoi/domain/auth/service/AuthService.javasrc/main/java/com/example/scoi/domain/websocket/WebsocketConnect.javasrc/main/java/com/example/scoi/domain/websocket/code/WebsocketErrorCode.javasrc/main/java/com/example/scoi/domain/websocket/event/WebsocketConnectionEvent.javasrc/main/java/com/example/scoi/domain/websocket/exception/WebsocketException.javasrc/main/java/com/example/scoi/domain/websocket/handler/UpbitTickerHandler.javasrc/main/java/com/example/scoi/domain/websocket/service/WebSocketService.javasrc/main/java/com/example/scoi/global/config/RedisConfig.javasrc/main/java/com/example/scoi/global/config/SecurityConfig.javasrc/main/java/com/example/scoi/global/config/SwaggerConfig.javasrc/main/java/com/example/scoi/global/redis/RedisUtil.javasrc/test/java/com/example/scoi/domain/auth/service/AuthServiceTest.javasrc/test/java/com/example/scoi/global/redis/RedisUtilIntegrationTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/example/scoi/domain/websocket/service/WebSocketService.java
|
|
||
| @Override | ||
| public void afterConnectionClosed(WebSocketSession session, CloseStatus status) throws Exception { | ||
| log.warn("[ Websocket ]: 업비트 웹소켓 연결이 종료되었습니다. CloseStatus: {}", status); | ||
| eventPublisher.publishEvent(new DisconnectedEvent()); | ||
| } | ||
|
|
||
| @Override | ||
| public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { | ||
| log.error("[ Websocket ]: 웹소켓 에러 발생: {}", exception.getMessage()); | ||
| eventPublisher.publishEvent(new DisconnectedEvent()); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
handleTransportError와 afterConnectionClosed가 DisconnectedEvent를 중복 발행합니다.
Spring 공식 문서에 따르면 afterConnectionClosed는 Invoked after the WebSocket connection has been closed by either side, or after a transport error has occurred. 즉 전송 오류 발생 시 handleTransportError 호출 이후에도 afterConnectionClosed가 항상 뒤따라 호출됩니다.
현재 두 메서드가 모두 DisconnectedEvent를 발행하므로, 오류로 인한 연결 종료 시 WebsocketConnect.onDisconnected()가 두 번 트리거되어 connectWithRetry()가 중복(비동기) 호출됩니다. currentSession은 마지막으로 성립된 연결로만 덮어써지므로, 먼저 열린 연결이 정리되지 않고 누수될 위험이 있습니다.
🔧 제안: 중복 이벤트 발행 방지
`@Override`
public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception {
log.error("[ Websocket ]: 웹소켓 에러 발생: {}", exception.getMessage());
- eventPublisher.publishEvent(new DisconnectedEvent());
+ // afterConnectionClosed가 전송 오류 이후에도 항상 호출되므로 여기서는 이벤트를 발행하지 않음
}📝 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.
| @Override | |
| public void afterConnectionClosed(WebSocketSession session, CloseStatus status) throws Exception { | |
| log.warn("[ Websocket ]: 업비트 웹소켓 연결이 종료되었습니다. CloseStatus: {}", status); | |
| eventPublisher.publishEvent(new DisconnectedEvent()); | |
| } | |
| @Override | |
| public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { | |
| log.error("[ Websocket ]: 웹소켓 에러 발생: {}", exception.getMessage()); | |
| eventPublisher.publishEvent(new DisconnectedEvent()); | |
| } | |
| `@Override` | |
| public void afterConnectionClosed(WebSocketSession session, CloseStatus status) throws Exception { | |
| log.warn("[ Websocket ]: 업비트 웹소켓 연결이 종료되었습니다. CloseStatus: {}", status); | |
| eventPublisher.publishEvent(new DisconnectedEvent()); | |
| } | |
| `@Override` | |
| public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { | |
| log.error("[ Websocket ]: 웹소켓 에러 발생: {}", exception.getMessage()); | |
| // afterConnectionClosed가 전송 오류 이후에도 항상 호출되므로 여기서는 이벤트를 발행하지 않음 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/example/scoi/domain/websocket/handler/UpbitTickerHandler.java`
around lines 69 - 80, Prevent duplicate DisconnectedEvent publication in
UpbitTickerHandler by making handleTransportError only log the transport failure
and leaving event publication to afterConnectionClosed. Preserve the existing
disconnection event behavior for normal closures and ensure each connection
termination triggers one event.
| @Async | ||
| @Retryable( | ||
| retryFor = WebsocketException.class, | ||
| maxAttempts = 10, | ||
| backoff = @Backoff(delay = 1000, multiplier = 2, maxDelay = 60000) | ||
| ) | ||
| public void connectWithRetry() { | ||
| log.info("[ Websocket ]: 업비트 웹소켓 연결 시도 중... URL: {}", PUBLIC_URL); | ||
| try { | ||
| // execute()의 리턴값인 Future를 대기(get)하여 타임아웃이나 연결 거부 시 예외를 발생시키도록 유도 | ||
| webSocketClient.execute(upbitTickerHandler, PUBLIC_URL).get(5, TimeUnit.SECONDS); | ||
| } catch (Exception e) { | ||
| log.error("[ Websocket ]: 웹소켓 연결 실패: {}", e.getMessage()); | ||
| // 예외를 던져서 @Retryable이 AOP 기반 백오프 및 재시도를 수행하도록 트리거 | ||
| throw new WebsocketException(WebsocketErrorCode.UPBIT_WEBSOCKET_ERROR); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files and their surrounding context.
git ls-files | rg 'src/main/java/com/example/scoi/domain/websocket/(WebsocketConnect|UpbitTickerHandler)\.java$|src/main/java/.*/websocket/.*\.java$'
for f in \
src/main/java/com/example/scoi/domain/websocket/WebsocketConnect.java \
src/main/java/com/example/scoi/domain/websocket/UpbitTickerHandler.java
do
if [ -f "$f" ]; then
echo "===== $f ====="
wc -l "$f"
ast-grep outline "$f" --view expanded || true
fi
doneRepository: UMCSCOI/Backend
Length of output: 1442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the full relevant source slices with line numbers.
for f in \
src/main/java/com/example/scoi/domain/websocket/WebsocketConnect.java \
src/main/java/com/example/scoi/domain/websocket/UpbitTickerHandler.java
do
if [ -f "$f" ]; then
echo "===== $f ====="
cat -n "$f" | sed -n '1,260p'
fi
doneRepository: UMCSCOI/Backend
Length of output: 5144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any session lifecycle or duplicate-connection handling.
rg -n --hidden --glob '!**/target/**' \
'currentSession|afterConnectionEstablished|WebSocketSession|execute\(upbitTickerHandler|`@Retryable`|WebsocketException|UPBIT_WEBSOCKET_ERROR' \
src/main/javaRepository: UMCSCOI/Backend
Length of output: 3017
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the relevant files if they exist.
for f in \
src/main/java/com/example/scoi/domain/websocket/WebsocketConnect.java \
src/main/java/com/example/scoi/domain/websocket/UpbitTickerHandler.java
do
if [ -f "$f" ]; then
echo "===== $f ====="
cat -n "$f" | sed -n '1,220p'
fi
doneRepository: UMCSCOI/Backend
Length of output: 5144
🌐 Web query:
Spring WebSocketClient execute CompletableFuture cancel handshake timeout behavior duplicate session afterConnectionEstablished
💡 Result:
In Spring Framework, the WebSocketClient.execute method returns a CompletableFuture that represents the asynchronous handshake process [1][2]. If you cancel this CompletableFuture, the behavior depends on the underlying implementation (e.g., StandardWebSocketClient, ReactorNettyWebSocketClient). Generally, canceling the future does not automatically guarantee that the underlying network connection or handshake process is aborted synchronously, as this depends on whether the implementation checks for cancellation during the connection setup [3][4]. Regarding "duplicate sessions afterConnectionEstablished": 1. Race Condition: If a timeout or cancellation signal is sent just as the handshake completes, there is a risk that the client-side code triggers the handshake failure logic while the server-side successfully upgrades the connection. This can lead to a state where the client perceives a timeout/cancellation but the connection is actually established, potentially leading to dangling or duplicate sessions if the application layer attempts to retry connection establishment without properly closing the previous, orphaned session [5]. 2. Resource Management: Because WebSocket connections involve an upgrade from HTTP, a timeout might abort the handshake attempt on the client side, but the TCP connection may remain open until the server or a network timeout closes it [6]. If your application logic retries the connection in response to the timeout without explicit resource cleanup, multiple sessions may appear to be active. To mitigate this: - Ensure your WebSocketHandler implementation of afterConnectionEstablished is prepared for edge cases where the session might already be closing [7]. - If using Reactor Netty, be aware that certain timeouts may inadvertently abort even a successfully established connection [6]. - Always explicitly call session.close in your error or cancellation handlers if a session object was partially initialized or returned to ensure no resources are leaked [1][4]. Top Results: [1][4][5][6]
Citations:
- 1: https://docs.spring.io/spring-framework/docs/6.2.14/javadoc-api/org/springframework/web/socket/client/AbstractWebSocketClient.html
- 2: https://docs.spring.io/spring-framework/docs/6.2.3/javadoc-api/org/springframework/web/socket/client/WebSocketClient.html
- 3: https://github.com/spring-projects/spring-framework/blob/master/spring-websocket/src/main/java/org/springframework/web/socket/client/AbstractWebSocketClient.java
- 4: https://docs.enterprise.spring.io/spring-framework/docs/6.1.25/javadoc-api/org/springframework/web/socket/client/standard/StandardWebSocketClient.html
- 5: https://medium.com/@zoomphant/pitfalls-using-websocket-in-springboot-f4edf9cfe733
- 6: add ability to configure response timeout for the web-socket handshake operation reactor/reactor-netty#1100
- 7: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/socket/WebSocketHandler.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the handler lifecycle and session management in detail.
f=src/main/java/com/example/scoi/domain/websocket/handler/UpbitTickerHandler.java
wc -l "$f"
cat -n "$f" | sed -n '1,220p'Repository: UMCSCOI/Backend
Length of output: 4756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f=src/main/java/com/example/scoi/domain/websocket/handler/UpbitTickerHandler.java
wc -l "$f"
cat -n "$f" | sed -n '1,220p'Repository: UMCSCOI/Backend
Length of output: 4756
중복 연결을 막는 가드가 필요합니다.
connectWithRetry()가 재시도되는 동안 이전 execute(...)가 늦게 성공하면 afterConnectionEstablished()가 다시 호출되고, 지금은 기존 currentSession을 닫지 않은 채 새 세션으로 덮어씁니다. 그 결과 두 세션이 동시에 살아남아 /topic/ticker가 중복 전송될 수 있으니, 연결 성공 시 기존 세션을 정리하거나 handler에서 중복 세션을 거부해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/example/scoi/domain/websocket/WebsocketConnect.java` around
lines 47 - 63, Update the connection-success handling associated with
connectWithRetry and upbitTickerHandler so a newly established websocket cannot
coexist with the existing currentSession. When afterConnectionEstablished()
receives a new session, close and clear the previous currentSession before
replacing it, or reject the new session when one is already active; preserve a
single active session for /topic/ticker delivery.
3 participants
📍 PR 타입 (하나 이상 선택)
❗️ 관련 이슈 링크
Closed #
📌 개요
🔁 변경 사항
📸 스크린샷
👀 기타 더 이야기해볼 점
✅ 체크 리스트
Summary by CodeRabbit
새로운 기능
개선 사항
테스트