Skip to content

fix(files): raise PermanentUploadError instead of bare raise in get_uploader#6569

Open
satyamshivam13 wants to merge 1 commit into
crewAIInc:mainfrom
satyamshivam13:fix/file-uploader-factory-bare-raise
Open

fix(files): raise PermanentUploadError instead of bare raise in get_uploader#6569
satyamshivam13 wants to merge 1 commit into
crewAIInc:mainfrom
satyamshivam13:fix/file-uploader-factory-bare-raise

Conversation

@satyamshivam13

Copy link
Copy Markdown

Closes #6568

Summary

Two bare raise statements in get_uploader() (lib/crewai-files/src/crewai_files/uploaders/factory.py)
were outside any exception handler, causing RuntimeError: No active exception to re-raise instead of
a meaningful error:

  1. Line 199 — Bedrock requested without CREWAI_BEDROCK_S3_BUCKET configured
  2. Line 216 — unsupported provider string passed

Fix

Both now raise PermanentUploadError (already defined in crewai_files/processing/exceptions.py
for exactly this kind of non-retryable config/input error), using the same message that was already
being logged via logger.debug().

Testing

Added lib/crewai-files/tests/test_factory.py with two regression tests covering both paths.
Verified locally:

  • uv run pytest lib/crewai-files/tests/test_factory.py -v — 2 passed
  • uv run ruff check lib/crewai-files/ --select PLE0704 — no issues (confirmed it flagged both
    lines on the pre-fix file, and passes after)
  • uv run mypy lib/crewai-files/src/crewai_files/ — no issues found in 33 source files

Disclosure

Found via AI-assisted static analysis (ruff's PLE0704 rule run across the codebase), same
defect pattern as #6430 in a different file. Per CONTRIBUTING.md this needs the llm-generated
label — I can't apply it myself, could a maintainer add it?

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 54d764ac-c337-4851-8ce9-922268096c7d

📥 Commits

Reviewing files that changed from the base of the PR and between c5ac2d9 and 7dae7d6.

📒 Files selected for processing (2)
  • lib/crewai-files/src/crewai_files/uploaders/factory.py
  • lib/crewai-files/tests/test_factory.py

📝 Walkthrough

Walkthrough

Changes

Uploader error handling

Layer / File(s) Summary
Factory permanent exceptions
lib/crewai-files/src/crewai_files/uploaders/factory.py
get_uploader now raises PermanentUploadError for missing Bedrock S3 configuration and unsupported providers.
Regression coverage
lib/crewai-files/tests/test_factory.py
Tests verify the exception type and messages for both error paths.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: replacing bare raises in get_uploader with PermanentUploadError.
Description check ✅ Passed The description is directly related to the code changes and regression tests in this PR.
Linked Issues check ✅ Passed The changes address #6568 by replacing both bare raises with PermanentUploadError and adding matching tests.
Out of Scope Changes check ✅ Passed The PR stays within scope; the added tests and error handling directly support the linked bug fix.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

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.

[BUG] Bare raise in get_uploader() causes RuntimeError instead of PermanentUploadError

1 participant