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
36 changes: 36 additions & 0 deletions src/main/java/in/koreatech/koin/infrastructure/fcm/FcmClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static com.google.firebase.messaging.AndroidConfig.Priority.HIGH;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.springframework.scheduling.annotation.Async;
Expand All @@ -24,6 +25,8 @@
@Component
public class FcmClient {

private static final int FCM_MESSAGE_BATCH_SIZE = 500;

@Async
public void sendMessage(
String targetDeviceToken,
Expand Down Expand Up @@ -70,6 +73,39 @@ public boolean sendMessageWithResult(
}
}

public void sendMessages(List<FcmSendRequest> requests) {
try {
for (int start = 0; start < requests.size(); start += FCM_MESSAGE_BATCH_SIZE) {
int end = Math.min(start + FCM_MESSAGE_BATCH_SIZE, requests.size());
List<Message> messages = requests.subList(start, end).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);
}
Comment on lines +76 to +106
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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:


🏁 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"
done

Repository: 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 50

Repository: 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]}")
PY

Repository: 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 . || true

Repository: 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 a BatchResponse that must be inspected, so current code hides partial failures.
  • Unlike sendMessageWithResult, sendMessages doesn’t guard against blank/invalid request.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.

Comment on lines +104 to +106
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

개별 실패를 체크하면 좋긴할텐데 로그 노이즈가 많이 발생할거 같으니,
한번의 시퀀스에 실패한 것들 리스트화 해두면 좋을 것 같아요

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

제가 PR에 설명이 부족했던 거 같습니다 😢

배치 전송을 하게 되면 BatchResponse 응답을 받게 되고 내부에는 SendResponse이라는 클래스가 존재합니다. 내부에는 사진과 같은 필드가 존재합니다.
image
여기서 각 단건에 대해서 에러를 받을 수 있습니다 ! 해당 코드의 에러 캐칭은 배치 전송 과정 자체에서 발생하는 에러를 캐치하는 것으로 이해하시면 될 거 같습니다.

말씀해주신 작업은 향후 작업으로 진행할 예정입니다 !

}

private ApnsConfig generateAppleConfig(
String title,
String content,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package in.koreatech.koin.infrastructure.fcm;

import in.koreatech.koin.common.model.MobileAppPath;

public record FcmSendRequest(
String targetDeviceToken,
String title,
String content,
String imageUrl,
MobileAppPath path,
String schemeUri,
String type
) {
public static FcmSendRequest of(
String targetDeviceToken,
String title,
String content,
String imageUrl,
MobileAppPath path,
String schemeUri,
String type
) {
return new FcmSendRequest(
targetDeviceToken,
title,
content,
imageUrl,
path,
schemeUri,
type
);
}
}
Loading