Skip to content
Merged
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
Expand Up @@ -2,11 +2,13 @@

import com.example.cs25batch.adapter.RedisStreamsClient;
import com.example.cs25batch.batch.dto.MailDto;
import com.example.cs25batch.batch.service.MailLogBatchService;
import com.example.cs25batch.batch.service.TodayQuizService;
import com.example.cs25entity.domain.quiz.entity.Quiz;
import com.example.cs25entity.domain.quiz.exception.QuizException;
import com.example.cs25entity.domain.subscription.entity.Subscription;
import com.example.cs25entity.domain.subscription.repository.SubscriptionRepository;
import java.time.LocalDateTime;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -21,6 +23,7 @@ public class MailConsumerAsyncProcessor implements ItemProcessor<Map<String, Str
private final SubscriptionRepository subscriptionRepository;
private final TodayQuizService todayQuizService;
private final RedisStreamsClient redisClient;
private final MailLogBatchService mailLogBatchService;

@Override
public MailDto process(Map<String, String> message) throws Exception {
Expand Down Expand Up @@ -49,6 +52,7 @@ public MailDto process(Map<String, String> message) throws Exception {
.build();
} catch(QuizException e){
//문제 출제 실패로 인한 예외 발생 시, 기존 Queue에 있는 데이터 삭제
mailLogBatchService.saveFailLog(subscription, null, LocalDateTime.now(), "No quizzes available");
redisClient.ackAndDel(recordId);
return null;
}
Comment on lines 54 to 58
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

ACK 보장: 로그 저장 실패 시에도 메시지를 반드시 ack/del 하도록 finally로 이동

현재는 saveFailLog에서 예외가 나면 ack가 호출되지 않아 동일 메시지가 재처리될 수 있습니다. ack를 finally로 옮기고, recordId null 가드 및 로깅을 추가해 주세요.

다음 패치를 제안합니다:

-        } catch(QuizException e){
-            //문제 출제 실패로 인한 예외 발생 시, 기존 Queue에 있는 데이터 삭제
-            mailLogBatchService.saveFailLog(subscription, null, LocalDateTime.now(), "No quizzes available");
-            redisClient.ackAndDel(recordId);
-            return null;
-        }
+        } catch (QuizException e) {
+            // 문제 출제 실패 시: 실패 로그 저장 후, 메시지는 항상 정리
+            try {
+                mailLogBatchService.saveFailLog(subscription, null, LocalDateTime.now(), "No quizzes available");
+            } catch (Exception logEx) {
+                log.error("Fail log persist failed. subscriptionId={}, recordId={}", subscriptionId, recordId, logEx);
+            } finally {
+                if (recordId != null) {
+                    redisClient.ackAndDel(recordId);
+                } else {
+                    log.warn("Missing recordId. subscriptionId={}", subscriptionId);
+                }
+            }
+            return null;
+        }
📝 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
//문제 출제 실패로 인한 예외 발생 시, 기존 Queue에 있는 데이터 삭제
mailLogBatchService.saveFailLog(subscription, null, LocalDateTime.now(), "No quizzes available");
redisClient.ackAndDel(recordId);
return null;
}
} catch (QuizException e) {
// 문제 출제 실패 시: 실패 로그 저장 후, 메시지는 항상 정리
try {
mailLogBatchService.saveFailLog(subscription, null, LocalDateTime.now(), "No quizzes available");
} catch (Exception logEx) {
log.error("Fail log persist failed. subscriptionId={}, recordId={}", subscriptionId, recordId, logEx);
} finally {
if (recordId != null) {
redisClient.ackAndDel(recordId);
} else {
log.warn("Missing recordId. subscriptionId={}", subscriptionId);
}
}
return null;
}
🤖 Prompt for AI Agents
In
cs25-batch/src/main/java/com/example/cs25batch/batch/component/processor/MailConsumerAsyncProcessor.java
around lines 54 to 58, the call to mailLogBatchService.saveFailLog can throw and
prevent redisClient.ackAndDel(recordId) from running, causing duplicate
reprocessing; move the ack/del into a finally block so it always runs, surround
saveFailLog with its own try/catch to log failures (including exception details)
but not rethrow, and in the finally check recordId for null before calling
redisClient.ackAndDel(recordId) and log a warning if recordId is null or ack/del
fails.

Expand Down