-
Notifications
You must be signed in to change notification settings - Fork 0
[ADE-66] fix: Enhance error handling and add debug message support in report processing #101
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the report processing workflow by improving error handling and incorporating debug message support.
- Frontend changes: Added an optional debugMessage field to the medical report model.
- Backend changes: Introduced an optional debugMessage field in the report model along with proper API documentation.
- Document Processor updates: Wrapped file retrieval and document processing in try-catch blocks, and refactored error handling by introducing an updateReportStatus method.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/src/common/models/medicalReport.ts | Added an optional debugMessage field to support debug information. |
| backend/src/reports/models/report.model.ts | Added a debugMessage field with accompanying @ApiProperty documentation. |
| backend/src/document-processor/controllers/document-processor.controller.ts | Enhanced error handling for file retrieval and document processing; added updateReportStatus function. |
Comments suppressed due to low confidence (2)
backend/src/document-processor/controllers/document-processor.controller.ts:193
- Consider passing the error message (or a relevant debug message) as the fourth parameter to updateReportStatus to capture additional error context.
await this.updateReportStatus(reportId, userId, ProcessingStatus.FAILED);
backend/src/document-processor/controllers/document-processor.controller.ts:206
- Consider including the error details in the updateReportStatus call by passing the error message as debugMessage to improve troubleshooting.
await this.updateReportStatus(reportId, userId, ProcessingStatus.FAILED);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances error handling and adds support for attaching a debug message in report processing. Key changes include:
- Introducing an optional debugMessage field into the frontend and backend report models.
- Wrapping S3 file retrieval and document processing in try/catch blocks with improved logging.
- Updating the failReport method to log the error details and update the report accordingly.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/src/common/models/medicalReport.ts | Added optional debugMessage field to the MedicalReport interface. |
| backend/src/reports/models/report.model.ts | Added optional debugMessage property to the Report model with appropriate API documentation. |
| backend/src/document-processor/controllers/document-processor.controller.ts | Enhanced error handling with try/catch blocks and modified error handling logic including a new failReport method. |
Files not reviewed (1)
- frontend/src/pages/Reports/ReportDetailPage.scss: Language not supported
Comments suppressed due to low confidence (1)
backend/src/document-processor/controllers/document-processor.controller.ts:216
- [nitpick] Removing the fallback values for report.title and report.category means that if result.analysis.title or result.analysis.category are undefined, the report may receive an undefined value. Please ensure that this change is intentional and that the service always provides valid values.
report.title = result.analysis.title;
backend/src/document-processor/controllers/document-processor.controller.ts
Outdated
Show resolved
Hide resolved
…controller.ts Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances error handling and adds support for debug messages in report processing.
- Adds an optional debugMessage property to the MedicalReport and Report models.
- Improves error handling in the DocumentProcessorController by wrapping external calls in try-catch blocks.
- Introduces a failReport method that updates the report status to FAILED and logs a debug message.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/src/common/models/medicalReport.ts | Added optional debugMessage property to MedicalReport. |
| backend/src/reports/models/report.model.ts | Added optional debugMessage property to Report model. |
| backend/src/document-processor/controllers/document-processor.controller.ts | Enhanced error handling for file retrieval and document processing; added failReport method. |
Files not reviewed (1)
- frontend/src/pages/Reports/ReportDetailPage.scss: Language not supported
Comments suppressed due to low confidence (1)
backend/src/document-processor/controllers/document-processor.controller.ts:211
- The silent return when a report is not found may mask underlying issues during async processing; consider rethrowing an error or handling this case more explicitly to ensure failures are not overlooked.
if (!report) { this.logger.error(`Report ${reportId} not found during async processing`); return; }
Change
fix: Enhance error handling and add debug message support in report processing
Does this PR introduce a breaking change?
{...}
What needs to be documented once your changes are merged?
{...}
Additional Comments
{...}