fix: prevent infinite retry loop on PermissionError in downloader

[allenhutchison]

Summary

• Fixes infinite retry loop when os.makedirs() fails with PermissionError during downloads
• Episode is now properly marked as failed instead of remaining in "pending" state
• Enhanced error message in constructor to help users identify configuration issues

Problem

When the download directory (e.g., /opt/podcasts) couldn't be created due to permission issues, the PermissionError was raised before the episode was marked as started or failed. This left the episode in download_status="pending", causing:

1. The pipeline to retry the same episode
2. Same failure repeating indefinitely
3. Log spam with "Download buffer low (0), downloading 10 more" every ~11 seconds

Test plan

• Added 3 new tests in TestEpisodeDownloaderPermissionErrors
• All 1082 tests pass
• Verified PermissionError marks episode as failed
• Verified mark_download_started is not called on PermissionError

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Enhanced error handling for download directory permission issues, providing clearer messages about which directory requires write access.
  • Improved tracking of downloads that fail due to permission restrictions, preventing unnecessary retry attempts.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The pull request adds PermissionError handling for directory creation across the EpisodeDownloader class. During initialization, permission errors raise an exception with a descriptive message. During episode downloads (both sync and async paths), permission errors are logged, the download is marked as failed in the repository, and a failed result is returned. Comprehensive tests validate these error scenarios.

Changes

Cohort / File(s)	Summary
Error Handling Implementation src/podcast/downloader.py	Wraps directory creation in three locations (__init__, download_episode, _download_episode_async) with try/except blocks. PermissionErrors during initialization raise with a clear message about directory writability. During downloads, PermissionErrors log errors, mark episodes as failed in the repository, and return failed DownloadResults.
Error Handling Tests tests/test_downloader.py	Adds three new test cases: one validating initialization raises PermissionError with correct message, and two validating download_episode properly handles PermissionError by marking download failed and not marking it started. Uses mocking for filesystem and repository interactions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Three try-excepts I did weave,
When permissions won't us leave,
Graceful errors, paths we test,
Dir creation handled best! ✨

🚥 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 fix: handling PermissionError to prevent infinite retry loops in the downloader module.
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 docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/downloader-permission-error-retry-loop

---

🧹 Recent nitpick comments

src/podcast/downloader.py (2)

162-172: Include traceback when logging PermissionError.

Capturing the traceback makes these failures much easier to debug in production logs.

♻️ Suggested change

-            logger.error(error_msg)
+            logger.exception(error_msg)

As per coding guidelines, ...

---

427-437: Mirror traceback logging in the async path.

Same rationale as the sync path—keep full context in logs.

♻️ Suggested change

-            logger.error(error_msg)
+            logger.exception(error_msg)

As per coding guidelines, ...

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ffff4f4 and c2a9e60.

📒 Files selected for processing (2)

• src/podcast/downloader.py
• tests/test_downloader.py

🧰 Additional context used

📓 Path-based instructions (3)

**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use PascalCase for class names (e.g., TranscriptionManager, GeminiFileSearchManager)
Use snake_case for function and method names (e.g., handle_transcription, search_vector_db)
Use UPPER_SNAKE_CASE for constants (e.g., TRANSCRIPTION_OUTPUT_SUFFIX)
Use leading underscore for private methods (e.g., _parse_response)
Use try/except with specific exception types instead of bare exceptions
Log errors with context using logger.error with traceback
Implement retry logic with exponential backoff for network operations
Validate inputs with Pydantic schemas

Files:

• tests/test_downloader.py
• src/podcast/downloader.py

tests/test_*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/test_*.py: Test files should mirror source structure using test_*.py naming convention
Use pytest fixtures for test setup and teardown

Files:

• tests/test_downloader.py

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use Factory pattern for model loading (Whisper, AI clients)
Use Service Layer pattern for separation of business logic from routes
Include docstrings in all modules, classes, and functions to describe purpose and behavior
Maintain type hints on function signatures and class attributes for better IDE support and type checking

Files:

• src/podcast/downloader.py

🧬 Code graph analysis (2)

tests/test_downloader.py (1)

src/podcast/downloader.py (1)

• download_episode (135-232)

src/podcast/downloader.py (2)

tests/test_repository.py (1)

• repository (10-19)

src/db/repository.py (2)

• mark_download_failed (526-536)
• mark_download_failed (2165-2177)

🪛 Ruff (0.14.13)

src/podcast/downloader.py

102-105: Avoid specifying long messages outside the exception class

(TRY003)

---

166-166: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

431-431: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

🔇 Additional comments (2)

src/podcast/downloader.py (1)

98-105: Clear PermissionError message improves diagnosability.

Nice improvement to point users to PODCAST_DOWNLOAD_DIRECTORY when creation fails.

tests/test_downloader.py (1)

665-736: PermissionError coverage looks solid.

These tests nicely exercise init and download failure paths, including the “do not mark started” behavior.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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