feat: FCM 병렬 동기 전송 메소드 추가#2263
Conversation
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 28 minutes and 54 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces batch Firebase Cloud Messaging support by adding a ChangesBatch FCM notification sending
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🤖 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/in/koreatech/koin/infrastructure/fcm/FcmClient.java`:
- Around line 74-101: The sendMessages method currently calls
FirebaseMessaging.getInstance().sendEach(messages) which hides per-message
failures and also builds messages from requests without filtering blank/invalid
targetDeviceToken(); update sendMessages to first filter out requests with
blank/empty request.targetDeviceToken(), then build Message objects (keeping a
parallel list mapping back to original request/token), call
FirebaseMessaging.getInstance().sendAll(messages) to obtain a BatchResponse,
iterate BatchResponse.getResponses() to detect failures and log each failed
index/token/request.id with the specific error, and optionally retry or collect
failed tokens for further handling; ensure the catch still logs unexpected
exceptions but per-message failures are handled via the BatchResponse rather
than swallowed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 457e575a-cfcf-4157-8e47-0e32d901acea
📒 Files selected for processing (2)
src/main/java/in/koreatech/koin/infrastructure/fcm/FcmClient.javasrc/main/java/in/koreatech/koin/infrastructure/fcm/FcmSendRequest.java
| public void sendMessages(List<FcmSendRequest> requests) { | ||
| try { | ||
| List<Message> messages = requests.stream() | ||
| .map(request -> Message.builder() | ||
| .setToken(request.targetDeviceToken()) | ||
| .setApnsConfig(generateAppleConfig( | ||
| request.title(), | ||
| request.content(), | ||
| request.imageUrl(), | ||
| request.path(), | ||
| request.type(), | ||
| request.schemeUri() | ||
| )) | ||
| .setAndroidConfig(generateAndroidConfig( | ||
| request.title(), | ||
| request.content(), | ||
| request.imageUrl(), | ||
| request.schemeUri(), | ||
| request.type() | ||
| )) | ||
| .build() | ||
| ) | ||
| .toList(); | ||
|
|
||
| FirebaseMessaging.getInstance().sendEach(messages); | ||
| } catch (Exception e) { | ||
| log.warn("FCM 알림 전송 실패", e); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Firebase Admin Java SDK, for FirebaseMessaging.sendEach(List), do individual message failures throw an exception or only appear in BatchResponse? Also confirm recommended failure-handling patterns.
💡 Result:
In the Firebase Admin Java SDK, the FirebaseMessaging.sendEach() method does not throw an exception for individual message failures [1]. Instead, it processes the entire batch and returns a BatchResponse object, which contains the results for each individual message [1][2]. If a specific message fails, that failure is encapsulated within the BatchResponse [1]. You must inspect the BatchResponse to determine the outcome of each message in the batch [2]. Recommended failure-handling patterns include: 1. Inspect the BatchResponse: Always check BatchResponse.getFailureCount() to see if any messages failed [2]. 2. Iterate through Responses: Use BatchResponse.getResponses() to iterate through the list of SendResponse objects [2]. Note that the order of these responses corresponds exactly to the order of the messages provided in the input list [2]. 3. Identify and Handle Failures: For each SendResponse, call isSuccessful(). If it returns false, use getException() to retrieve the specific error details [1][3]. 4. Token Management: Common errors, such as messaging/registration-token-not-registered or messaging/invalid-argument, often indicate that a token is stale or invalid [4][5][6]. When these errors are identified in the SendResponse, you should remove the associated token from your database to prevent future failed attempts [3][4][6]. 5. Retry Strategy: For errors indicating temporary issues (e.g., messaging/server-unavailable or messaging/internal-error), implement an exponential backoff retry strategy [5]. Do not retry messages that failed due to permanent client-side errors [3][7].
Citations:
- 1: feat(fcm): Implement
sendEach,sendEachAsync,sendEachForMulticastandsendEachForMulticastAsync(#785) firebase/firebase-admin-java#815 - 2: https://firebase.google.com/docs/cloud-messaging/send/admin-sdk
- 3: 9.0.0 to 9.3.0 sendMulticastAsync vs sendEachForMulticastAsync. Server crashes when sending too many pushes firebase/firebase-admin-java#1001
- 4: https://firebase.google.com/docs/reference/admin/error-handling
- 5: https://firebase.google.com/docs/cloud-messaging/error-codes
- 6: https://stackoverflow.com/questions/56216887/requested-entity-was-not-found-error-when-trying-to-send-a-push-notification-u
- 7: https://stackoverflow.com/questions/64160384/retrying-failed-fcm-messages-using-firebase-admin-sdk-for-go
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/main/java/in/koreatech/koin/infrastructure/fcm/FcmClient.java"
echo "=== File exists? ==="
ls -l "$FILE" || exit 1
echo "=== Method: sendMessages (around lines 60-130) ==="
nl -ba "$FILE" | sed -n '50,140p'
echo "=== Search: sendMessageWithResult ==="
rg -n "sendMessageWithResult" "$FILE"
echo "=== Context around sendMessageWithResult and any hasText guards ==="
# show around all hasText occurrences
rg -n "hasText" "$FILE" || true
for m in $(rg -n "sendMessageWithResult" "$FILE" | cut -d: -f1); do
start=$((m-40))
end=$((m+80))
echo "--- Around sendMessageWithResult at line $m ---"
nl -ba "$FILE" | sed -n "${start},${end}p"
doneRepository: BCSDLab/KOIN_API_V2
Length of output: 312
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/main/java/in/koreatech/koin/infrastructure/fcm/FcmClient.java"
echo "=== Method: sendMessages (around lines 60-130) ==="
sed -n '60,140p' "$FILE" | cat -n
echo "=== Search: sendMessageWithResult ==="
rg -n "sendMessageWithResult" "$FILE" || true
echo "=== Search: hasText usage ==="
rg -n "hasText" "$FILE" || true
echo "=== Search: token usage in sendMessages ==="
rg -n "targetDeviceToken|registration-token|token" "$FILE" | head -n 50Repository: BCSDLab/KOIN_API_V2
Length of output: 3970
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/main/java/in/koreatech/koin/infrastructure/fcm/FcmClient.java"
echo "=== File imports (top) ==="
sed -n '1,120p' "$FILE" | cat -n
echo "=== sendMessageWithResult block (full) ==="
# find start/end by searching for method signature and next method
python3 - <<'PY'
import re, itertools, pathlib
p = pathlib.Path("src/main/java/in/koreatech/koin/infrastructure/fcm/FcmClient.java")
lines = p.read_text().splitlines()
start = next(i for i,l in enumerate(lines) if "public boolean sendMessageWithResult" in l)
# end at next "private " or "public void sendMessages" or EOF
end = next((j for j in range(start+1,len(lines)) if re.search(r'public void sendMessages|private ApnsConfig|public void|private ', lines[j]) ), len(lines))
for k in range(start, min(end, start+140)):
print(f"{k+1:4d} {lines[k]}")
PY
echo "=== sendMessages exact block ==="
python3 - <<'PY'
import pathlib, re
p = pathlib.Path("src/main/java/in/koreatech/koin/infrastructure/fcm/FcmClient.java")
lines = p.read_text().splitlines()
start = next(i for i,l in enumerate(lines) if "public void sendMessages(List<FcmSendRequest> requests)" in l)
end = next((j for j in range(start+1,len(lines)) if re.search(r'\n\s*(private|public)\s', "\n"+ "\n".join(lines[j:])),), len(lines))
# fallback simple: print 60 lines
for k in range(start, min(start+60, len(lines))):
print(f"{k+1:4d} {lines[k]}")
PYRepository: BCSDLab/KOIN_API_V2
Length of output: 6534
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/main/java/in/koreatech/koin/infrastructure/fcm/FcmClient.java"
echo "=== Search for existing var usage in this file ==="
rg -n "\bvar\b" "$FILE" || true
echo "=== Search for BatchResponse usage in repo ==="
rg -n "BatchResponse|SendResponse" . || true
echo "=== Search for Java version / sourceCompatibility hints ==="
rg -n "sourceCompatibility|maven-compiler-plugin|toolchain|java\.version" -S . || trueRepository: BCSDLab/KOIN_API_V2
Length of output: 270
Handle per-message failures in FcmClient.sendMessages and filter blank tokens
FirebaseMessaging.getInstance().sendEach(messages)does not throw per-message errors; it returns aBatchResponsethat must be inspected, so current code hides partial failures.- Unlike
sendMessageWithResult,sendMessagesdoesn’t guard against blank/invalidrequest.targetDeviceToken(), so invalid tokens can degrade the whole batch.
Proposed fix
public void sendMessages(List<FcmSendRequest> requests) {
+ if (requests == null || requests.isEmpty()) {
+ return;
+ }
try {
- List<Message> messages = requests.stream()
+ List<FcmSendRequest> validRequests = requests.stream()
+ .filter(request -> request != null && StringUtils.hasText(request.targetDeviceToken()))
+ .toList();
+
+ if (validRequests.isEmpty()) {
+ return;
+ }
+
+ List<Message> messages = validRequests.stream()
.map(request -> Message.builder()
.setToken(request.targetDeviceToken())
.setApnsConfig(generateAppleConfig(
request.title(),
request.content(),
@@
.build()
)
.toList();
- FirebaseMessaging.getInstance().sendEach(messages);
+ var batchResponse = FirebaseMessaging.getInstance().sendEach(messages);
+ if (batchResponse.getFailureCount() > 0) {
+ for (int i = 0; i < batchResponse.getResponses().size(); i++) {
+ var response = batchResponse.getResponses().get(i);
+ if (!response.isSuccessful()) {
+ log.warn("FCM batch send failed. token: {}, error: {}",
+ validRequests.get(i).targetDeviceToken(),
+ response.getException() != null ? response.getException().getMessage() : "unknown");
+ }
+ }
+ }
} catch (Exception e) {
log.warn("FCM 알림 전송 실패", e);
}
}🤖 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/in/koreatech/koin/infrastructure/fcm/FcmClient.java` around
lines 74 - 101, The sendMessages method currently calls
FirebaseMessaging.getInstance().sendEach(messages) which hides per-message
failures and also builds messages from requests without filtering blank/invalid
targetDeviceToken(); update sendMessages to first filter out requests with
blank/empty request.targetDeviceToken(), then build Message objects (keeping a
parallel list mapping back to original request/token), call
FirebaseMessaging.getInstance().sendAll(messages) to obtain a BatchResponse,
iterate BatchResponse.getResponses() to detect failures and log each failed
index/token/request.id with the specific error, and optionally retry or collect
failed tokens for further handling; ensure the catch still logs unexpected
exceptions but per-message failures are handled via the BatchResponse rather
than swallowed.
|
|
||
| List<Message> messages = requests.stream() | ||
| .map(request -> Message.builder() | ||
| .setToken(request.targetDeviceToken()) |
There was a problem hiding this comment.
device 토큰을 여기서 검사해서 공통화 시키면 되겠네요
| if (requests.size() > FCM_MESSAGE_BATCH_SIZE) { | ||
| log.warn("FCM 전송 최대 개수를 초과했습니다."); | ||
| return ; | ||
| } |
There was a problem hiding this comment.
바로 return하는것보다 500개씩 끊어보내는 것도 방법일 것 같아요
| } catch (Exception e) { | ||
| log.warn("FCM 알림 전송 실패", e); | ||
| } |
There was a problem hiding this comment.
개별 실패를 체크하면 좋긴할텐데 로그 노이즈가 많이 발생할거 같으니,
한번의 시퀀스에 실패한 것들 리스트화 해두면 좋을 것 같아요
There was a problem hiding this comment.
제가 PR에 설명이 부족했던 거 같습니다 😢
배치 전송을 하게 되면 BatchResponse 응답을 받게 되고 내부에는 SendResponse이라는 클래스가 존재합니다. 내부에는 사진과 같은 필드가 존재합니다.
여기서 각 단건에 대해서 에러를 받을 수 있습니다 ! 해당 코드의 에러 캐칭은 배치 전송 과정 자체에서 발생하는 에러를 캐치하는 것으로 이해하시면 될 거 같습니다.
말씀해주신 작업은 향후 작업으로 진행할 예정입니다 !
2 participants
🔍 개요
키워드 알림 이벤트 처리는
@Async를 통해 별도 스레드에서 병렬로 동작하고 있으나, APM 확인 결과 각 이벤트 처리에 수 초가 소요되는 문제가 있었습니다.두 작업이 순차 및 동기적으로 진행되어 병목이 발생함을 확인했습니다.
게시물 키워드 알림 기능의 경우 속도가 핵심인 기능이라고 생각했습니다.
해당 상황을 예방하고자, FCM 전송 로직을 개선하는 작업을 진행합니다.
close [공통] FCM 병렬 동기 전송 메소드 추가 #2262
🚀 주요 변경 내용
FCM 병렬 동기 전송 메소드 추가
sendEach메소드를 활용하는 메소드를 추가했습니다.💬 참고 사항
✅ Checklist (완료 조건)