Removed Exception and logger added #91
Conversation
WalkthroughThe method Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🛑 Comments failed to post (3)
src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java (3)
186-209:
⚠️ Potential issueInconsistent exception handling logic.
The inner try-catch catches and logs exceptions but allows processing to continue, which may lead to unexpected behavior since subsequent operations depend on successful demographic fetching.
The current nested structure creates inconsistent behavior:
- Inner catch: Logs error and continues processing
- Outer catch: Logs error and moves to next patient
This could result in processing bundles without proper demographic data. Consider removing the nested try-catch as suggested in the previous comment to ensure consistent error handling.
🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java between lines 186 and 209, the inner try-catch block catches exceptions and logs them but allows processing to continue, which can cause issues since later code depends on successful demographic fetching. To fix this, remove the inner try-catch block entirely so that any exceptions propagate to the outer catch block, ensuring consistent error handling and preventing processing of incomplete data.
237-239: 🛠️ Refactor suggestion
Enhance error logging with patient-specific context.
The error message should include patient-specific information to aid in debugging and monitoring.
Apply this diff to improve error logging:
-logger.error("Fhir Schedule run failed " +e.getMessage()); +logger.error("FHIR bundle creation failed for BenID: {} | BenRegID: {} | VisitCode: {} - {}", + p.getBeneficiaryId(), p.getBeneficiaryRegID(), p.getVisitCode(), e.getMessage(), e);This provides better context for debugging and includes the full stack trace.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.} catch (Exception e) { - logger.error("Fhir Schedule run failed " + e.getMessage()); + logger.error("FHIR bundle creation failed for BenID: {} | BenRegID: {} | VisitCode: {} - {}", + p.getBeneficiaryId(), + p.getBeneficiaryRegID(), + p.getVisitCode(), + e.getMessage(), + e); }🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java around lines 237 to 239, the error logging in the catch block only logs the exception message without patient-specific context or the full stack trace. Modify the logger.error call to include relevant patient information (such as patient ID or name) along with the exception message, and pass the exception object itself to log the full stack trace for better debugging and monitoring.
176-240: 🛠️ Refactor suggestion
Good improvement to error handling, but consider simplifying the nested structure.
The addition of the outer try-catch block is a solid improvement that prevents exceptions from one patient's processing from affecting others. However, the nested try-catch structure could be simplified for better maintainability.
Consider refactoring to eliminate the nested try-catch blocks:
for (PatientEligibleForResourceCreation p : pList) { try { logger.info("Bundle creation is started for BenID : " + p.getBeneficiaryId() + " | BenRegID : " + p.getBeneficiaryRegID() + " | VisitCode : " + p.getVisitCode()); resourceRequestHandler = new ResourceRequestHandler(); resourceRequestHandler.setBeneficiaryID(p.getBeneficiaryId()); resourceRequestHandler.setVisitCode(p.getVisitCode()); resourceRequestHandler.setBeneficiaryRegID(p.getBeneficiaryRegID()); - try { - logger.info("*****Fetch beneficiary Id: " + resourceRequestHandler.getBeneficiaryRegID()); - List<Object[]> rsObjList = patientEligibleForResourceCreationRepo - .callPatientDemographicSP(resourceRequestHandler.getBeneficiaryRegID()); - logger.info("*****Fetch beneficiary Id response recevied :", rsObjList); - - PatientDemographic patientDemographicOBJ = patientDemographic.getPatientDemographic(rsObjList); - logger.info("*****Fetch patient after fetching demographics"); - if (patientDemographicOBJ != null) { - addCareContextToMongo(patientDemographicOBJ, p); - logger.info("*****Add to mongo success done"); - if (patientDemographicOBJ.getPreferredPhoneNo() != null) - sendAbdmAdvSMS(patientDemographicOBJ.getPreferredPhoneNo()); - else - logger.error( - "Advertisement sms could not be sent as beneficiary phone no not found"); - } else - throw new FHIRException( - "Beneficiary not found, benRegId = " + resourceRequestHandler.getBeneficiaryRegID()); - - } catch (Exception e) { - logger.error(e.getMessage()); - } + logger.info("*****Fetch beneficiary Id: " + resourceRequestHandler.getBeneficiaryRegID()); + List<Object[]> rsObjList = patientEligibleForResourceCreationRepo + .callPatientDemographicSP(resourceRequestHandler.getBeneficiaryRegID()); + logger.info("*****Fetch beneficiary Id response recevied :", rsObjList); + + PatientDemographic patientDemographicOBJ = patientDemographic.getPatientDemographic(rsObjList); + logger.info("*****Fetch patient after fetching demographics"); + if (patientDemographicOBJ == null) { + throw new FHIRException( + "Beneficiary not found, benRegId = " + resourceRequestHandler.getBeneficiaryRegID()); + } + + addCareContextToMongo(patientDemographicOBJ, p); + logger.info("*****Add to mongo success done"); + if (patientDemographicOBJ.getPreferredPhoneNo() != null) { + sendAbdmAdvSMS(patientDemographicOBJ.getPreferredPhoneNo()); + } else { + logger.error("Advertisement sms could not be sent as beneficiary phone no not found"); + } // Bundle processing logic... } catch (Exception e) { - logger.error("Fhir Schedule run failed " +e.getMessage()); + logger.error("FHIR bundle creation failed for BenID: {} | BenRegID: {} | VisitCode: {} - {}", + p.getBeneficiaryId(), p.getBeneficiaryRegID(), p.getVisitCode(), e.getMessage()); } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.try { logger.info("Bundle creation is started for BenID : " + p.getBeneficiaryId() + " | BenRegID : " + p.getBeneficiaryRegID() + " | VisitCode : " + p.getVisitCode()); resourceRequestHandler = new ResourceRequestHandler(); resourceRequestHandler.setBeneficiaryID(p.getBeneficiaryId()); resourceRequestHandler.setVisitCode(p.getVisitCode()); resourceRequestHandler.setBeneficiaryRegID(p.getBeneficiaryRegID()); // fetch demographics (no more nested try/catch) logger.info("*****Fetch beneficiary Id: " + resourceRequestHandler.getBeneficiaryRegID()); List<Object[]> rsObjList = patientEligibleForResourceCreationRepo .callPatientDemographicSP(resourceRequestHandler.getBeneficiaryRegID()); logger.info("*****Fetch beneficiary Id response received: {}", rsObjList); PatientDemographic patientDemographicOBJ = patientDemographic.getPatientDemographic(rsObjList); logger.info("*****Fetch patient after fetching demographics"); if (patientDemographicOBJ == null) { throw new FHIRException( "Beneficiary not found, benRegId = " + resourceRequestHandler.getBeneficiaryRegID()); } addCareContextToMongo(patientDemographicOBJ, p); logger.info("*****Add to mongo success done"); if (patientDemographicOBJ.getPreferredPhoneNo() != null) { sendAbdmAdvSMS(patientDemographicOBJ.getPreferredPhoneNo()); } else { logger.error("Advertisement sms could not be sent as beneficiary phone no not found"); } // 1. OP consult resource bundle int i = oPConsultRecordBundle.processOPConsultRecordBundle(resourceRequestHandler, p); // 2. diagnostic report record bundle int j = diagnosticReportRecordBundle.processDiagnosticReportRecordBundle(resourceRequestHandler, p); // 3. prescription Bundle int k = prescriptionBundle.processPrescriptionRecordBundle(resourceRequestHandler, p); logger.info("The value of i: {} The value of j: {} The value of k: {}", i, j, k); if (i > 0 && j > 0 && k > 0) { // update the processed flag in trigger table p.setProcessed(true); PatientEligibleForResourceCreation resultSet = patientEligibleForResourceCreationRepo.save(p); if (resultSet != null && resultSet.getId().compareTo(BigInteger.ZERO) > 0) { logger.info("processed flag updated successfully after FHIR resource creation"); } response = "Bundle creation is success for BenID : " + p.getBeneficiaryId() + " | BenRegID : " + p.getBeneficiaryRegID() + " | VisitCode : " + p.getVisitCode(); logger.info("Bundle creation is success for BenID : {} | BenRegID : {} | VisitCode : {}", p.getBeneficiaryId(), p.getBeneficiaryRegID(), p.getVisitCode()); } // adv SMS - ABDM notify2 API } catch (Exception e) { logger.error("FHIR bundle creation failed for BenID: {} | BenRegID: {} | VisitCode: {} - {}", p.getBeneficiaryId(), p.getBeneficiaryRegID(), p.getVisitCode(), e.getMessage()); }🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java around lines 176 to 240, the nested try-catch blocks inside the main try block make the code harder to maintain. Refactor by merging the inner try-catch into the outer one, handling all exceptions in a single catch block. This will simplify the control flow and improve readability while preserving the error handling behavior.
2 participants
📋 Description
JIRA ID:
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit