✅ [Deploy] v0.5.1 배포#140
Closed
rlawngjs0313 wants to merge 15 commits into
Closed
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 로그 제거
📝 WalkthroughWalkthroughSMS 인증 실패 카운터와 시도 제한을 추가하고, 웹소켓 재연결·워치독·연결 이벤트 처리를 확장했습니다. 배포 명령과 서비스 도메인 설정을 변경하고 시세 상세 로그를 제거했습니다. ChangesSMS 인증 시도 제한
웹소켓 연결 복구
배포 및 서비스 주소 정리
Estimated code review effort: 4 (Complex) | ~50 minutes Sequence Diagram(s)sequenceDiagram
participant ApplicationReadyEvent
participant WebsocketConnect
participant webSocketClient
participant UpbitTickerHandler
ApplicationReadyEvent->>WebsocketConnect: init()
WebsocketConnect->>webSocketClient: execute(...).get(timeout)
webSocketClient-->>UpbitTickerHandler: ConnectedEvent
UpbitTickerHandler-->>WebsocketConnect: publish connection event
WebsocketConnect->>UpbitTickerHandler: getLastMessageTime()
WebsocketConnect->>UpbitTickerHandler: closeSession() after 30 seconds
UpbitTickerHandler-->>WebsocketConnect: DisconnectedEvent
WebsocketConnect->>webSocketClient: connectWithRetry()
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
kingmingyu
approved these changes
Jul 10, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/test/java/com/example/scoi/global/redis/RedisUtilIntegrationTest.java (1)
24-42: 🧹 Nitpick | 🔵 TrivialCI에 Redis 미가동 시 테스트가 조용히 스킵됨.
@EnabledIf("redisAvailable")로 인해 CI 파이프라인에 Redis 서비스가 없으면 이 통합 테스트가 실패 없이 스킵됩니다. 파이프라인에 Redis 컨테이너가 실제로 구성되어 있는지 확인하는 것을 권장합니다(그렇지 않으면 이 계약 검증이 실질적으로 수행되지 않습니다).🤖 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/test/java/com/example/scoi/global/redis/RedisUtilIntegrationTest.java` around lines 24 - 42, Ensure the CI pipeline explicitly provisions and starts a Redis container/service for RedisUtilIntegrationTest, and verify it is reachable at localhost:6379 before running tests. Keep the redisAvailable guard only if intended for local environments; configure CI so the test executes rather than being silently skipped.
🤖 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 70-80: DisconnectedEvent가 중복 발행되지 않도록 UpbitTickerHandler의
handleTransportError에서는 eventPublisher.publishEvent(new DisconnectedEvent()) 호출을
제거하고 오류 로그만 남기세요. 연결 종료 이벤트 발행은 afterConnectionClosed에서만 유지하세요.
In `@src/main/java/com/example/scoi/domain/websocket/WebsocketConnect.java`:
- Around line 37-39: Make watchdogTask thread-safe across onConnected() and
onDisconnected(): declare it volatile and synchronize the
check/cancel/reschedule operations, or use an AtomicReference to perform atomic
updates. Ensure concurrent callbacks cannot leave an uncancelled task or
schedule duplicate watchdog tasks.
- Around line 53-63: connectWithRetry에서 webSocketClient.execute(...)가 반환하는
Future를 변수로 보관하고, get 타임아웃 발생 시 해당 Future를 명시적으로 cancel(true)하여 진행 중인 연결 작업을
중단하세요. 취소 후 기존 WebsocketException을 발생시켜 `@Retryable` 재시도를 유지하고, 이미 완료된 경우에는 불필요한
취소가 발생하지 않도록 처리하세요.
---
Nitpick comments:
In `@src/test/java/com/example/scoi/global/redis/RedisUtilIntegrationTest.java`:
- Around line 24-42: Ensure the CI pipeline explicitly provisions and starts a
Redis container/service for RedisUtilIntegrationTest, and verify it is reachable
at localhost:6379 before running tests. Keep the redisAvailable guard only if
intended for local environments; configure CI so the test executes rather than
being silently skipped.
🪄 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: e1ca5974-a516-497b-9d09-36a88662de51
📒 Files selected for processing (15)
.github/workflows/cd.ymlsrc/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
Comment on lines
+37
to
+39
| // 워치독(Half-Open 헬스체크) 전용 스케줄러 | ||
| private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); | ||
| private ScheduledFuture<?> watchdogTask; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
watchdogTask 필드의 스레드 안전성 문제.
watchdogTask는 volatile이 아니며, onConnected()/onDisconnected()는 이벤트 발행 스레드(WebSocket 컨테이너 스레드 또는 @Async 스레드)에서 호출될 수 있어 서로 다른 스레드에서 동시 실행될 가능성이 있습니다. check-then-act 패턴(null 체크 → cancel → 재할당)이 원자적이지 않아, 경쟁 상태에서 워치독 취소가 누락되거나 중복 스케줄될 수 있습니다.
최소한 volatile 선언과 함께, 가능하면 synchronized 블록이나 AtomicReference로 감싸는 것을 권장합니다.
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, Make watchdogTask thread-safe across onConnected() and
onDisconnected(): declare it volatile and synchronize the
check/cancel/reschedule operations, or use an AtomicReference to perform atomic
updates. Ensure concurrent callbacks cannot leave an uncancelled task or
schedule duplicate watchdog tasks.
Comment on lines
+53
to
+63
| 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
타임아웃 시 진행 중인 연결 시도가 취소되지 않음.
Future.get(timeout)은 타임아웃 발생 시 대기만 중단할 뿐 내부적으로 진행 중인 연결 작업을 취소하지 않습니다. 타임아웃으로 WebsocketException이 발생해 재시도가 이루어지는 동안, 이전 execute() 호출이 백그라운드에서 나중에 성공하면 두 개 이상의 활성 웹소켓 연결이 동시에 존재할 수 있습니다. 이 경우 afterConnectionEstablished가 여러 번 호출되어 ConnectedEvent가 중복 발행되고, 티커 데이터가 중복으로 STOMP 브로드캐스트될 위험이 있습니다.
🔧 제안: 타임아웃 시 future를 명시적으로 취소
public void connectWithRetry() {
log.info("[ Websocket ]: 업비트 웹소켓 연결 시도 중... URL: {}", PUBLIC_URL);
+ java.util.concurrent.Future<WebSocketSession> future = null;
try {
- // execute()의 리턴값인 Future를 대기(get)하여 타임아웃이나 연결 거부 시 예외를 발생시키도록 유도
- webSocketClient.execute(upbitTickerHandler, PUBLIC_URL).get(5, TimeUnit.SECONDS);
+ future = webSocketClient.execute(upbitTickerHandler, PUBLIC_URL);
+ future.get(5, TimeUnit.SECONDS);
} catch (Exception e) {
log.error("[ Websocket ]: 웹소켓 연결 실패: {}", e.getMessage());
+ if (future != null) {
+ future.cancel(true);
+ }
throw new WebsocketException(WebsocketErrorCode.UPBIT_WEBSOCKET_ERROR);
}
}📝 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.
Suggested change
| 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); | |
| } | |
| } | |
| public void connectWithRetry() { | |
| log.info("[ Websocket ]: 업비트 웹소켓 연결 시도 중... URL: {}", PUBLIC_URL); | |
| java.util.concurrent.Future<WebSocketSession> future = null; | |
| try { | |
| future = webSocketClient.execute(upbitTickerHandler, PUBLIC_URL); | |
| future.get(5, TimeUnit.SECONDS); | |
| } catch (Exception e) { | |
| log.error("[ Websocket ]: 웹소켓 연결 실패: {}", e.getMessage()); | |
| if (future != null) { | |
| future.cancel(true); | |
| } | |
| // 예외를 던져서 `@Retryable이` AOP 기반 백오프 및 재시도를 수행하도록 트리거 | |
| throw new WebsocketException(WebsocketErrorCode.UPBIT_WEBSOCKET_ERROR); | |
| } | |
| } |
🤖 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 53 - 63, connectWithRetry에서 webSocketClient.execute(...)가 반환하는 Future를 변수로
보관하고, get 타임아웃 발생 시 해당 Future를 명시적으로 cancel(true)하여 진행 중인 연결 작업을 중단하세요. 취소 후 기존
WebsocketException을 발생시켜 `@Retryable` 재시도를 유지하고, 이미 완료된 경우에는 불필요한 취소가 발생하지 않도록
처리하세요.
- SmsSendResponse.expiredAt 주석 오류 정정 (3분 -> 5분) - 데드코드 generateSmsToken 제거 (호출처 없음, 인증 우회 소지) - resetPassword 트랜잭션 애노테이션 스프링으로 통일 - resetPassword에서 sms_required 플래그 삭제로 잠금 해제 일관성 확보 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
♻️ refactor: Auth 도메인 디테일/일관성 보완 (#142)
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/com/example/scoi/domain/auth/service/AuthService.java (2)
90-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value실패 카운터 초기화 로직 적절함.
새 인증번호 발급 시 이전 실패 카운터를 삭제해 새 시도 예산을 부여하는 흐름이 명확합니다. 다만 verifySms(Line 126)에서는
failKey를 지역 변수로 추출해 사용하는 반면, 이 지점에서는 키 문자열을 인라인으로 조합하고 있어 스타일이 다릅니다. 일관성을 위해 지역 변수로 추출하는 것도 고려해볼 만합니다.♻️ 참고용 리팩터링
+ String failKey = SMS_VERIFY_FAIL_PREFIX + request.phoneNumber(); redisUtil.set(redisKey, verificationCode, SMS_EXPIRATION_MINUTES, TimeUnit.MINUTES); - redisUtil.delete(SMS_VERIFY_FAIL_PREFIX + request.phoneNumber()); + redisUtil.delete(failKey);🤖 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/auth/service/AuthService.java` around lines 90 - 93, In the SMS issuance flow around the Redis save logic, extract SMS_VERIFY_FAIL_PREFIX + request.phoneNumber() into a local failKey variable and pass that variable to redisUtil.delete, matching the existing verifySms style while preserving behavior.
105-105: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win로그 인증코드 노출 제거는 적절하나, DEV/QA 로그와 일관성 없음.
성공 로그에서 인증코드를 제거한 것은 보안 관점에서 좋은 개선입니다. 다만 바로 아래 else 분기(Line 111, DEV/QA 모드)에서는 여전히
code={}로 인증코드를 로그에 남기고 있어, 이 PR의 취지(인증코드 로그 노출 최소화)와 어긋납니다.exposeCode플래그로 응답에는 이미 노출 여부를 제어하고 있으므로, DEV 로그도 같은 정책을 따르는 것이 좋습니다.🤖 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/auth/service/AuthService.java` at line 105, Update the DEV/QA logging branch in AuthService so it no longer logs the authentication code via the code placeholder. Align that log with the production success log by retaining only the phone number, while leaving exposeCode response behavior unchanged.
🤖 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.
Nitpick comments:
In `@src/main/java/com/example/scoi/domain/auth/service/AuthService.java`:
- Around line 90-93: In the SMS issuance flow around the Redis save logic,
extract SMS_VERIFY_FAIL_PREFIX + request.phoneNumber() into a local failKey
variable and pass that variable to redisUtil.delete, matching the existing
verifySms style while preserving behavior.
- Line 105: Update the DEV/QA logging branch in AuthService so it no longer logs
the authentication code via the code placeholder. Align that log with the
production success log by retaining only the phone number, while leaving
exposeCode response behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 33d22518-3b55-46dd-8729-ddcdafa6eaed
📒 Files selected for processing (2)
src/main/java/com/example/scoi/domain/auth/dto/AuthResDTO.javasrc/main/java/com/example/scoi/domain/auth/service/AuthService.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/example/scoi/domain/auth/dto/AuthResDTO.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📍 PR 타입 (하나 이상 선택)
❗️ 관련 이슈 링크
Closed #
📌 개요
🔁 변경 사항
📸 스크린샷
👀 기타 더 이야기해볼 점
✅ 체크 리스트
Summary by CodeRabbit
scoi.store로 변경.