Skip to content

fix: raise PermanentUploadError instead of bare raise in get_uploader - #6956

Open
richboyneedcash wants to merge 1 commit into
crewAIInc:mainfrom
richboyneedcash:fix/get-uploader-bare-raise
Open

fix: raise PermanentUploadError instead of bare raise in get_uploader#6956
richboyneedcash wants to merge 1 commit into
crewAIInc:mainfrom
richboyneedcash:fix/get-uploader-bare-raise

Conversation

@richboyneedcash

Copy link
Copy Markdown

Fixes #6568

Bug

get_uploader() (lib/crewai-files/src/crewai_files/uploaders/factory.py) has two bare raise statements outside any exception handler:

  1. When Bedrock is requested without CREWAI_BEDROCK_S3_BUCKET configured
  2. When an unsupported provider string is passed

A bare raise re-raises the exception currently being handled, but neither path is inside an except block, so instead of a meaningful error Python raises:

RuntimeError: No active exception to re-raise
get_uploader("bedrock")               # without CREWAI_BEDROCK_S3_BUCKET set
get_uploader("not-a-real-provider")
# both -> RuntimeError: No active exception to re-raise

Fix

Raise PermanentUploadError (already used across this package for non-retryable config/input errors, and defined in crewai_files/processing/exceptions.py) with a descriptive message in both cases.

Tests

Added tests/uploaders/test_factory.py covering the unsupported-provider and missing-bucket paths (both now assert PermanentUploadError). ruff check --select PLE0704 also flagged these bare raises.

get_uploader() had two bare `raise` statements outside any except block:
one when Bedrock is requested without CREWAI_BEDROCK_S3_BUCKET configured,
and one for an unsupported provider. A bare raise re-raises the exception
currently being handled, but neither path is inside an except block, so
Python raised `RuntimeError: No active exception to re-raise` instead of a
meaningful error.

Raise PermanentUploadError (already used across this package for
non-retryable config/input errors) with a descriptive message in both
cases. Add regression tests for the unsupported-provider and
missing-bucket paths.

Fixes crewAIInc#6568

Co-authored-by: TRAE CLI <noreply@bytedance.com>
@coderabbitai

coderabbitai Bot commented Aug 11, 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: 2fb7ad7d-5a00-40c3-8911-407b358697a7

📥 Commits

Reviewing files that changed from the base of the PR and between 094b94e and dfff222.

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

📝 Walkthrough

Walkthrough

The uploader factory now raises PermanentUploadError for missing Bedrock configuration and unsupported providers. Tests verify the exception type and message for both cases.

Changes

Uploader error handling

Layer / File(s) Summary
Factory errors and regression tests
lib/crewai-files/src/crewai_files/uploaders/factory.py, lib/crewai-files/tests/uploaders/test_factory.py
The factory replaces bare raises with PermanentUploadError for missing Bedrock bucket configuration and unsupported providers. Tests verify both exception types and provider-specific messages.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states that get_uploader now raises PermanentUploadError instead of a bare raise.
Description check ✅ Passed The description accurately explains the bug, the fix, the affected paths, and the added regression tests.
Linked Issues check ✅ Passed The changes satisfy issue #6568 by replacing both bare raises with PermanentUploadError and adding tests for both paths.
Out of Scope Changes check ✅ Passed The changes are limited to the requested uploader error handling and directly related regression tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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