Skip to content

fix(android): Improve push notification loading reliability and add diagnostic logging #6711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 12 commits into
base: develop
Choose a base branch
from
Draft

fix(android): Improve push notification loading reliability and add diagnostic logging #6711

wants to merge 12 commits into from

Conversation

@diegolmello
Copy link
Member

@diegolmello diegolmello commented Oct 9, 2025 •
edited
Loading

Proposed changes

This PR fixes critical bugs in Android push notification handling that caused "message-id-only" notifications to display placeholder text ("You have a new message") instead of loading the actual message content.

The issue was difficult to reproduce and only affected certain users. Investigation revealed multiple bugs:

  1. Missing callback calls when validation failed, causing the notification flow to hang
  2. Array index out of bounds in retry logic
  3. No HTTP timeouts allowing requests to hang indefinitely
  4. Poor error handling making debugging impossible

This PR also adds comprehensive production-safe diagnostic logging to help identify and resolve similar issues in the future.

Issue(s)

https://rocketchat.atlassian.net/browse/SUP-882

How to test or reproduce

Testing the fix:

  1. Configure a Rocket.Chat server to send message-id-only push notifications
  2. Receive a notification while app is in background
  3. Notification should display actual message content (not placeholder)

Verifying logs (debug build):

  1. Build a debug APK
  2. Run adb logcat | grep RocketChat
  3. Receive a notification
  4. Logs should show the full diagnostic flow without exposing sensitive data

Affected screens:

  • Push notifications (system tray)
  • All notification-related background processing

Screenshots

N/A - This is a backend notification handling fix with no UI changes

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Technical Details:

Bug Fixes (LoadNotification.java):

  • Fixed early return to always call callback.call(null) ensuring proper flow continuation
  • Fixed array bounds check from RETRY_COUNT <= TIMEOUT.length to RETRY_COUNT < TIMEOUT.length - 1
  • Added HTTP timeouts (10s connect, 30s read/write) via OkHttpClient.Builder
  • Added null safety for response body, serverURL, and messageId
  • Separated exception handling for IOException, JsonSyntaxException, and generic exceptions

Logging (Production-Safe):

  • Added structured logging with appropriate levels (DEBUG/INFO/WARN/ERROR)
  • Verbose logs gated behind BuildConfig.DEBUG (debug builds only)
  • Production logs never expose: user IDs, auth tokens, or full URLs
  • URLs sanitized to show only protocol and host
  • Follows Android logging best practices

Files Changed:

  • LoadNotification.java - Core bug fixes and error handling
  • CustomPushNotification.java - Added flow logging
  • Ejson.java - Added debug-only verbose logging with BuildConfig guards

The logging addition makes future issues like this much easier to diagnose while maintaining security and following industry standards for production logging.

@diegolmello
fix react context
ca313e1
@diegolmello
Fix ejson mmkv
9b45bce
@diegolmello
Enhance notification handling with improved logging and error management
ebf1e4d 
- Added detailed logging for notification loading process in LoadNotification.java.
- Implemented error handling for null or empty userId and userToken.
- Enhanced retry logic for network requests with clear logging of attempts and failures.
- Updated Ejson.java to include logging for token and userId retrieval.
- Improved fallback notification handling in CustomPushNotification.java with additional logging for failures.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025 •
edited
Loading

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix.android-push-secured

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@diegolmello diegolmello temporarily deployed to experimental_android_build October 9, 2025 14:29 — with GitHub Actions Inactive
@diegolmello diegolmello had a problem deploying to experimental_ios_build October 9, 2025 14:29 — with GitHub Actions Error
@diegolmello diegolmello had a problem deploying to official_android_build October 9, 2025 14:29 — with GitHub Actions Error
@diegolmello diegolmello had a problem deploying to upload_experimental_android October 9, 2025 15:01 — with GitHub Actions Error
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Android Build Available

Rocket.Chat Experimental 4.65.0.107532

Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNT_WZhhUi5TNV4tmGX1GR2QFqcHIvQXdxflAusqcJtIUJIV51K9uLFChkhR_6W5bnYKp535DUtnt0lmIRY_

OtavioStasiak
OtavioStasiak approved these changes Oct 10, 2025
View reviewed changes
Copy link
Contributor

@OtavioStasiak OtavioStasiak left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@diegolmello diegolmello force-pushed the fix.android-push-secured branch from ebf1e4d to 1063087 Compare October 28, 2025 14:14
@diegolmello diegolmello had a problem deploying to experimental_android_build October 28, 2025 14:17 — with GitHub Actions Error
@diegolmello
Enhance CustomPushNotification and Ejson with improved logging and er…
733c21f 
…ror handling

- Added verbose logging for notification processing and avatar URI generation in CustomPushNotification.java.
- Implemented checks for React initialization before processing notifications.
- Updated Ejson.java to ensure avatar URI generation handles null or empty values gracefully.
- Enhanced LoadNotification.java to log encryption fields in notifications.
- Improved overall error management and notification handling logic.
@diegolmello diegolmello force-pushed the fix.android-push-secured branch from 1063087 to 733c21f Compare October 31, 2025 18:12
@diegolmello diegolmello force-pushed the fix.android-push-secured branch from 733c21f to f2ef6db Compare October 31, 2025 18:14
@diegolmello diegolmello temporarily deployed to approve_e2e_testing October 31, 2025 18:14 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to experimental_android_build October 31, 2025 18:19 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to experimental_ios_build October 31, 2025 18:19 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Oct 31, 2025

iOS Build Available

Rocket.Chat Experimental 4.66.0.107615

@diegolmello
Fix import
c11deec
@diegolmello
Enhance notification ID validation in CustomPushNotification and Repl…
60a317d 
…yBroadcast

- Added validation for notification ID in CustomPushNotification to ensure it is present and correctly formatted before processing.
- Improved error handling in ReplyBroadcast for invalid notification IDs during message replies.
- Enhanced logging for invalid notification ID scenarios to aid in debugging.
@diegolmello
Refactor error handling in CustomPushNotification
16f0b56 
- Added try-catch blocks around notification handling to log errors during processing.
- Removed redundant validation comments and ensured notification ID is parsed safely.
- Improved logging for failures in notification processing to enhance debugging capabilities.
@diegolmello
Improve synchronization and error handling in CustomPushNotification …
24282ae 
…and Ejson

- Added synchronization to ensure thread safety when creating notification properties in CustomPushNotification.
- Enhanced error handling for null NotificationManager during channel creation and added logging for better debugging.
- Made ensureMMKVInitialized method synchronized in Ejson to prevent concurrent access issues.
@diegolmello diegolmello force-pushed the fix.android-push-secured branch from f2ef6db to 24282ae Compare November 3, 2025 14:22
@diegolmello diegolmello temporarily deployed to approve_e2e_testing November 3, 2025 14:23 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to experimental_ios_build November 3, 2025 14:30 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to experimental_android_build November 3, 2025 14:30 — with GitHub Actions Inactive
@diegolmello diegolmello had a problem deploying to upload_experimental_android November 3, 2025 15:10 — with GitHub Actions Error
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

iOS Build Available

Rocket.Chat Experimental 4.66.0.107619

@github-actions
Copy link

github-actions bot commented Nov 3, 2025

Android Build Available

Rocket.Chat Experimental 4.66.0.107618

Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTceCDXLl0n5KzB7McYH_xPCE7vnmVY3fr0bo0WwW_fYcqILHUCkhPXNwvGE45prdAr8Xz_W_RnJOhUFfvO

@diegolmello diegolmello temporarily deployed to approve_e2e_testing November 3, 2025 18:28 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to experimental_android_build November 3, 2025 18:31 — with GitHub Actions Inactive
@diegolmello diegolmello requested a deployment to official_android_build November 3, 2025 18:31 — with GitHub Actions Waiting
@diegolmello diegolmello temporarily deployed to experimental_ios_build November 3, 2025 18:31 — with GitHub Actions Inactive
@diegolmello diegolmello requested a deployment to upload_experimental_android November 3, 2025 19:02 — with GitHub Actions Waiting
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

Android Build Available

Rocket.Chat Experimental 4.66.0.107620

Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNRfE-G-Ytsb5alGX8qbpjuuuaxJG396MHERdATYJodrY1VE3_9TkUs8OJpVe81zR3dSyCFHZ0SjxCHf0zhs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@OtavioStasiak OtavioStasiak OtavioStasiak approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@diegolmello @OtavioStasiak