Skip to content

ES-2909 | added logger for debugging#1783

Open
nandhu-kumar wants to merge 2 commits into
mosip:developfrom
nandhu-kumar:develop
Open

ES-2909 | added logger for debugging#1783
nandhu-kumar wants to merge 2 commits into
mosip:developfrom
nandhu-kumar:develop

Conversation

@nandhu-kumar
Copy link
Copy Markdown
Contributor

@nandhu-kumar nandhu-kumar commented Mar 4, 2026

  • added logger for debugging

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling and logging for SMS notifications to enhance reliability and observability; internal failures are now logged and surfaced without changing the public SMS API or response behavior.

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6844d13a-6768-49e0-badb-ea0ac66ac93d

📥 Commits

Reviewing files that changed from the base of the PR and between 1c6bce7 and ca0430d.

📒 Files selected for processing (1)
  • kernel/kernel-notification-service/src/main/java/io/mosip/kernel/emailnotification/controller/SmsNotificationController.java

Walkthrough

The SMS notification controller now includes structured error handling and logging: a static SLF4J logger was added and the SMS send call is wrapped in a try-catch that logs exceptions and rethrows them.

Changes

Cohort / File(s) Summary
SMS Notification Error Handling
kernel/kernel-notification-service/src/main/java/io/mosip/kernel/emailnotification/controller/SmsNotificationController.java
Added imports for EmailNotificationServiceImpl and SLF4J LoggerFactory; introduced a private static LOGGER; wrapped the SMS send call in a try-catch that logs errors and rethrows the exception.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A tiny rabbit found a log,
It caught the hiccups of each jog,
When SMS trips and stumbles loose,
I whisper back — "I'll note the use!"
🥕📜

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change in the PR: adding a logger to the SmsNotificationController for debugging purposes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@kernel/kernel-notification-service/src/main/java/io/mosip/kernel/emailnotification/controller/SmsNotificationController.java`:
- Around line 64-69: The controller currently swallows errors when calling
smsNotifierService.sendSmsNotification in SmsNotificationController by catching
Throwable and only logging, which returns a 200 with an empty responseWrapper;
change this so exceptions propagate to EmailNotificationApiExceptionHandler:
either remove the try/catch completely or replace it with a catch(Exception e)
that logs via LOGGER.error(...) and then rethrows the same exception (throw e)
so the global handler can produce the proper 500 response; ensure
responseWrapper.setResponse(...) is only executed on successful send and do not
suppress the original exception.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e8dec843-8247-4cd5-8c60-224c8ea2a41a

📥 Commits

Reviewing files that changed from the base of the PR and between bd276a7 and 1c6bce7.

📒 Files selected for processing (1)
  • kernel/kernel-notification-service/src/main/java/io/mosip/kernel/emailnotification/controller/SmsNotificationController.java

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant