Skip to content

🥅(summary) catch unexpected file-related exceptions when handling recording objects#944

Merged
lebaudantoine merged 1 commit intomainfrom
not-found-recording
Feb 5, 2026
Merged

🥅(summary) catch unexpected file-related exceptions when handling recording objects#944
lebaudantoine merged 1 commit intomainfrom
not-found-recording

Conversation

@lebaudantoine
Copy link
Collaborator

Previously, if a recording file was not found in the bucket, the code would crash. This adds proper error handling to avoid unhandled failures.

@lebaudantoine
Copy link
Collaborator Author

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

Walkthrough

A new FileServiceException class was added to summary.core.file_service and _download_from_minio now catches MinioException and S3Error, re-raising them as FileServiceException. In summary.core.celery_worker, process_audio_transcribe_summarize_v2 wraps audio preparation and transcription calls in a try/except that catches FileServiceException, logs and returns early on error, and records transcription start/end times in metadata. An import for FileServiceException was added to the worker module.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding exception handling for file-related errors when processing recording objects, which aligns with the changeset that introduces FileServiceException and wraps file operations in try-catch blocks.
Description check ✅ Passed The description is directly related to the changeset, explaining the motivation for the changes (preventing crashes when recording files are not found) and describing the solution (adding proper error handling).
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.


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

@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

🤖 Fix all issues with AI agents
In `@src/summary/summary/core/celery_worker.py`:
- Line 178: The logged full transcription (the logger.debug call that prints the
variable transcription) exposes potential PII; remove the direct transcription
output and replace it with a safe alternative such as logging only metadata
(e.g., transcription length, language, and a truncated/redacted snippet like
transcription[:100] + '...[REDACTED]') or a non-reversible hash/checksum of
transcription. Locate the logger.debug("Transcription: \n %s", transcription)
call in celery_worker.py and update it to log only redacted/derived info (or
remove the log entirely) while ensuring the variable name transcription is not
interpolated into logs.

@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

✅ Actions performed

Full review triggered.

Previously, if a recording file was not found in the bucket, the code would
crash. This adds proper error handling to avoid unhandled failures.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 5, 2026

@lebaudantoine lebaudantoine merged commit 9f58efb into main Feb 5, 2026
24 of 25 checks passed
@lebaudantoine lebaudantoine deleted the not-found-recording branch February 5, 2026 16:50
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