Improved logging on fcm token nullability #3355
Conversation
📝 WalkthroughWalkthroughThe change updates Firebase Cloud Messaging token retrieval handling in FirebaseMessagingUtil.java. It imports org.apache.commons.lang3.StringUtils and modifies the onComplete callback: on success, it retrieves the token as a String and, if non-empty, calls updateFCMToken(token); if empty, it logs an exception indicating an empty token. On failure, it ensures a non-null Throwable by creating a default one when task.getException() is null and logs it. This adds explicit guards against null/empty tokens and standardizes error logging. Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Firebase as FirebaseMessaging
participant Util as FirebaseMessagingUtil
participant Logger
App->>Firebase: requestToken()
Firebase-->>Util: onComplete(Task<String>)
alt Success
Util->>Util: token = task.getResult()
alt token is non-empty
Util->>Util: updateFCMToken(token)
else token is null/empty
Util->>Logger: logException("Empty FCM token", Throwable)
end
else Failure
Util->>Util: ex = task.getException() or new Throwable("FCM token retrieval failed")
Util->>Logger: logException("FCM token retrieval error", ex)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/org/commcare/utils/FirebaseMessagingUtil.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/org/commcare/utils/FirebaseMessagingUtil.java (1)
app/src/org/commcare/utils/StringUtils.java (1)
StringUtils(15-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (2)
app/src/org/commcare/utils/FirebaseMessagingUtil.java (2)
104-110: LGTM! Empty token guard directly addresses PR objective.The change adds explicit logging when an empty FCM token is retrieved, which aligns perfectly with the PR's stated objective to diagnose empty FCM token occurrences. The implementation correctly uses
StringUtils.isEmpty()to handle both null and empty string cases, preventing invalid tokens from being stored while capturing diagnostic information.
112-114: LGTM! Defensive error handling improvement.The change ensures that
Logger.exception()always receives a non-nullThrowable, creating a fallback with a descriptive message whentask.getException()returns null. This defensive approach improves the robustness of error logging.
| import com.google.firebase.messaging.FirebaseMessaging; | ||
| import com.google.firebase.messaging.RemoteMessage; | ||
|
|
||
| import org.apache.commons.lang3.StringUtils; |
There was a problem hiding this comment.
❓ Verification inconclusive
Verify Apache Commons Lang dependency availability.
The code now uses org.apache.commons.lang3.StringUtils.isEmpty() for null/empty token checks. Ensure that Apache Commons Lang is declared as a project dependency (e.g., in build.gradle or similar). Note that a local org.commcare.utils.StringUtils class exists but doesn't provide isEmpty() functionality, so using the external library is appropriate.
Run the following script to verify the dependency is declared:
🏁 Script executed:
#!/bin/bash
# Description: Check if Apache Commons Lang is declared as a dependency
# Check for commons-lang3 in Gradle build files
fd -e gradle -x grep -l "commons-lang3"
# Also check Maven POM files if present
fd -e xml -x grep -l "commons-lang3"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Refined search for Apache Commons Lang dependency declarations
rg -n "commons-lang3" -g "*.gradle" -g "*.xml"
rg -n "lang3" -g "*.gradle" -g "*.xml"Length of output: 0
Add Apache Commons Lang3 dependency
The code uses org.apache.commons.lang3.StringUtils but no commons-lang3 dependency was found in any Gradle or Maven file. Add an appropriate entry (e.g. implementation "org.apache.commons:commons-lang3:3.x.x") to your build configuration.
🤖 Prompt for AI Agents
In app/src/org/commcare/utils/FirebaseMessagingUtil.java around line 22 the code
imports org.apache.commons.lang3.StringUtils but the project has no
commons-lang3 dependency; add the appropriate dependency to the project's build
configuration (for Gradle add implementation
"org.apache.commons:commons-lang3:3.12.0" to the module/app build.gradle
dependencies block or, for Maven, add the corresponding <dependency> entry for
org.apache.commons:commons-lang3 with a 3.x.x version) and then refresh/rebuild
so the import resolves.
| String token = task.getResult(); | ||
| if (!StringUtils.isEmpty(token)) { | ||
| updateFCMToken(token); | ||
| } else { | ||
| Logger.exception("Fetching FCM registration token failed", | ||
| new Throwable("FCM registration token is empty")); | ||
| } |
There was a problem hiding this comment.
I know you are trying to put the only logs for null fcm token but we should try to put extra code to make sure that fcm token is never null from the mobile at least.
FCM token cannot be null or empty unless task has failed. I think we should have FCM token retrieval mechanism if it's null / empty here or here. This will make sure that fcm token is always present.
Also, there can be issue on server side also which is making the token inactive if failed to send push notification. I have tried to understand that here but seems requires more discussion on it.
There was a problem hiding this comment.
Think the first step here is to understand what is happening and this PR is aimed to get more insights into question of where it's going wrong (mobile or server).
FCM token cannot be null or empty unless task has failed. I think we should have FCM token retrieval mechanism if it's null / empty here or here. This will make sure that fcm token is always present.
We already do FCM token retreival every time app startup so don't really think adding more code there will help here.
There was a problem hiding this comment.
Yeah correct. I have feeling that sometimes heartbeat is called before this retrieval take place at app start up but yeah we don't have concrete data for it.
Product Description
Additional logs to investigate if fcm tokens are getting set to empty.
Labels and Review